Thread Rating:
  • 0 Votes - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
DviServer crash
14-10-2014, 10:26 AM
Post: #1
DviServer crash
I've been debugging a problem with a SIGSEGV crash in DviServer::FindInterface. It happens when an active network adapter is disabled.

The crash is within the NetworkAdapter::Address method called by this line in DviServer::FindInterface:

if (aNifList[i]->Address() == aInterface) {

It looks that the code is accessing memory that has been freed and then reallocated and overwritten. As is normal in such cases, the crash doesn't happen every time because the memory isn't reallocated and overwritten every time.

By looking at a debug log, I have identified where this is happening. The call to NetworkAdapterList::List from DviServer::SubnetListChanged in this line:

const std::vector<NetworkAdapter*>& nifList = adapterList.List();

returns the iNetworkAdapters member. This member is deleted and reallocated when NetworkAdapterList::HandleInterfaceListChanged is called on another thread. Here is the code that does this:

Code:
DestroySubnetList(iNetworkAdapters);
iNetworkAdapters = list;

I added some extra debug log calls to confirm that the above code is running during the execution of the following loop in DviServer::SubnetListChanged:

Code:
for (i = (TInt)iServers.size() - 1; i >= 0; i--) {
    DviServer::Server* server = iServers[i];
    if (FindInterface(server->Interface(), nifList) == -1) {
        delete server;
        iServers.erase(iServers.begin() + i);
    }
}

This loop can take quite a long time to complete and the problem happens because the nifList reference becomes invalid while the loop is executing.

The simplest fix seems to be for DviServer::SubnetListChanged to create a copy of the iNetworkAdapters member by calling NetworkAdapterList::CreateNetworkAdapterList and NetworkAdapterList::DestroyNetworkAdapterList instead of NetworkAdapterList::List. I am attaching a patch that does this. There is similar code in DviProtocolUpnp::HandleInterfaceChange and I have made the same change here as well. I have built and tested these changes and they seem to be working OK.


Attached File(s)
.zip  adapterList.zip (Size: 2.05 KB / Downloads: 1)
Find all posts by this user
15-10-2014, 12:21 PM
Post: #2
RE: DviServer crash
Good spot!

That code was initially safe because adapter changes were all handled in the ohNet thread that received notification of changes from the OS port. When we changed to run the ohNet & client callbacks in a different thread from the OS notification, NetworkAdapterList::List() was no longer threadsafe.

I've committed a fix along the lines you suggested but have also removed NetworkAdapterList::List(). I didn't change the signature of the FindInterface() helper functions - our convention is that passing by pointer normally implies a transfer of ownership so the changes here were potentially confusing.
Find all posts by this user


Forum Jump: