Crash in NetworkAdapter::RemoveRef

[simoncn]

I've just seen a crash in the control point stack. It happened in NetworkAdapter::RemoveRef within the delete operator call near the end of the method. This was on Windows, just after my laptop had been resumed after being suspended.

You might be able to get a better idea of what was happening from the following disassembly code. The start of this extract corresponds to the Stack::Mutex().Signal() call on line 84 of OhNet.cpp.

5D79A261 call OpenHome::Mutex::Signal (5D7944D0h)
5D79A266 cmp byte ptr [esp+17h],0
5D79A26B je OpenHome::NetworkAdapter::RemoveRef+0FCh (5D79A2BCh)
5D79A26D mov dword ptr [esp+28h],esi
5D79A271 mov dword ptr [esi],offset OpenHome::NetworkAdapter::`vftable' (5D7C3550h)
5D79A277 push esi
5D79A278 mov dword ptr [esp+24h],1
5D79A280 call OpenHome::Net::Stack::RemoveObject (5D791DB0h)
5D79A285 mov eax,dword ptr [esi+1Ch]
5D79A288 xor edi,edi
5D79A28A add esp,4
5D79A28D cmp eax,edi
5D79A28F je OpenHome::NetworkAdapter::RemoveRef+0DAh (5D79A29Ah)
5D79A291 push eax
5D79A292 call operator delete (5D79D197h)
5D79A297 add esp,4
5D79A29A lea ecx,[esi+10h]
5D79A29D mov dword ptr [esi+1Ch],edi
5D79A2A0 mov dword ptr [esi+20h],edi
5D79A2A3 mov dword ptr [esi+24h],edi
5D79A2A6 mov dword ptr [esp+20h],0FFFFFFFFh
5D79A2AE call OpenHome::Brv::~Brv (5D733030h)
5D79A2B3 push esi
5D79A2B4 call operator delete (5D79D197h)
5D79A2B9 add esp,4

The crash happened during the call at address 5D79A2B4.

[simonc]

Thanks for the report.

I don't fully understand the disassembly but it appears to be showing that the memory for a NetworkAdapter's iName is corrupt (already freed or over-written).

The only way I can think of for it to have been freed already is if client code deleted the const pointer returned by Name(). I can't find any evidence of this; also, it'd be hard to get this wrong as you'd also have to cast away the constness of this pointer.

The memory having been over-written is possible but much harder to diagnose as the software that caused the over-write could be anywhere else in ohNet or client code.

I'll keep thinking about this but we might have to wait for further crashes to get more hints about what's going wrong.

[simonc]

As an aside, this report encouraged me to review some jni code and spot a memory leak in NetworkAdapter.FullName(). So, even if we don't fix the bug you saw, ohNet is slightly more robust as a result of your report .

[simoncn]

(03-10-2012 12:47 PM)simonc Wrote:  Thanks for the report.

I don't fully understand the disassembly but it appears to be showing that the memory for a NetworkAdapter's iName is corrupt (already freed or over-written).

The only way I can think of for it to have been freed already is if client code deleted the const pointer returned by Name(). I can't find any evidence of this; also, it'd be hard to get this wrong as you'd also have to cast away the constness of this pointer.

The memory having been over-written is possible but much harder to diagnose as the software that caused the over-write could be anywhere else in ohNet or client code.

I'll keep thinking about this but we might have to wait for further crashes to get more hints about what's going wrong.

Thanks for looking into this. If it happens again, I'll try to look at the memory in question. (Finding the right address will stretch my disassembly/C++ skills to the limit!)

[simonc]

(03-10-2012 01:23 PM)simoncn Wrote:  Thanks for looking into this. If it happens again, I'll try to look at the memory in question. (Finding the right address will stretch my disassembly/C++ skills to the limit!)

Thanks. Note that its actually the value of NetworkAdapter.iName.iPtr we'd want. And possibly the value of some of the preceding heap cells (on the assumption that heap corruption is only being detected if guard words just before our pointer have been over-written.)

[simoncn]

(03-10-2012 01:26 PM)simonc Wrote:  Thanks. Note that its actually the value of NetworkAdapter.iName.iPtr we'd want. And possibly the value of some of the preceding heap cells (on the assumption that heap corruption is only being detected if guard words just before our pointer have been over-written.)

I've been able to provoke this crash repeatably on Linux by disabling the current network adapter while the control point stack is running. This was using the release build. I'll try this with the debug build and see if I can work out how to get the necessary information from gdb. Is there by any chance a nice graphical debugger for Ubuntu that will make this easier?

[simonc]

(03-10-2012 02:01 PM)simoncn Wrote:  I've been able to provoke this crash repeatably on Linux by disabling the current network adapter while the control point stack is running. This was using the release build. I'll try this with the debug build and see if I can work out how to get the necessary information from gdb. Is there by any chance a nice graphical debugger for Ubuntu that will make this easier?

Sorry, I can't help here. My Linux debugging is all gdb command line.

[simonc]

I can reproduce a crash now. I can take it from here if you like.

[simonc]

I've committed a fix now which should make it onto github this evening. When you get a chance, could you let me know if this fixes your crash too please?

[simoncn]

(03-10-2012 03:28 PM)simonc Wrote:  I've committed a fix now which should make it onto github this evening. When you get a chance, could you let me know if this fixes your crash too please?

I've downloaded this and tried the same scenario on Linux that was crashing previously. It seems to be working OK now. Many thanks for the quick fix!