james91b / ida_ipython

An IDA Pro Plugin for embedding an IPython Kernel
MIT License
252 stars 46 forks source link

IDA crashing on termination. #8

Closed tmr232 closed 9 years ago

tmr232 commented 9 years ago

It seems that every time I try and close IDA, it crashes.

It is not critical, as it crashes after saving the database, but it is still a bug. As it does no noticeable harm, I only noticed it after setting Windbg to be the postmortem debugger. But once I set it, I now see IDA crashing due to an access violation on every termination.

This has been reproduced on multiple computers.

Will add more information as soon as my debugging yields any.

james91b commented 9 years ago

What version of IDA are you using? I am not currently getting it on IDA 6.6. I have seen this happen due to the hook not getting removed correctly on termination.

tmr232 commented 9 years ago

I'm using IDA 6.8. My guess would be the hook removal as well.

williballenthin commented 9 years ago

I receive the same exception on IDA 6.8.

Unhandled exception at 0x70DE3DD8 in idaq.exe: 0xC00001A5: An invalid exception handler routine has been detected (parameters: 0x00000003).
>k
 Index  Function
--------------------------------------------------------------------------------
*1      70de26bb()
 2      [Frames below may be incorrect and/or missing]
 3      QtCore4.dll!65cc305d()
 4      QtCore4.dll!65cc88e6()
 5      idaq.exe!005707a3()
 6      [External Code]
tmr232 commented 9 years ago

Reproduced on IDA 6.6.

tmr232 commented 9 years ago

I think I found the issue.

Debugging shows that the hook removal happens as expected, and the plugin is unloaded from memory. After that, we encounter the crash, with the crashing address in the memory of our previously loaded plugin. The crashing address is the return from the original function, seen here in the code.

Apparently, the plugin memory is freed before the hooked function returns. So when it does return, it has no code to return to.

@williballenthin can you verify this before we go on to solving this?

james91b commented 9 years ago

Ahh so are you only seeing it with a debugger attached? I attached one and i think i see what you mean now. Maybe the term function is being called in the QEventDispatcherWin32 loop. Not sure what a fix might be but hmmmm...i'll have to do some more investigation.

tmr232 commented 9 years ago

I think the simplest solution will be to separate the plugin into a plugin and a DLL. The plugin will load the DLL, and the DLL does the rest. So when the plugin terminates, we remove the hook, but keep the DLL in memory.

I think that anything else will probably require messy code to get things done (assembly to change the return address / IAT hooking / copying a part of the code to newly allocated memory). I do want to avoid the extra file, though.

james91b commented 9 years ago

The easiest messy solution would be to prevent the error by just avoiding the call.

__declspec(naked) void DetourQEventDispatcherWin32()
{
    __asm {
        pushad
        call ipython_embed_iteration
        popad
        jmp pQEventDispatcherWin32
    };
}

It works, but it's not exactly the nicest or most portable solution. Depends on how much we care about supporting non VC++ compilers or non x86.

tmr232 commented 9 years ago

@james91b If can still fail on return from ipython_embed_iteration.

I found a different solution. If we call LoadLibrary on the plugin again (LoadLibrary("ida_python.plw")), it will not be released on plugin termination, so everything will continue to work. (We can use GetModuleHandleEx to get the handle to the plugin, and GetModuleFilename to get the plugin's filename, then LoadLibrary to load it.)

james91b commented 9 years ago

I don't think it would fail unless the plugin was terminated in the ipython_embed_iteration call. But regardless, the LoadLibrary solution is much nicer, let's go with that.