search

nanomsg / nng

nanomsg-next-generation -- light-weight brokerless messaging
https://nng.nanomsg.org
MIT License
3.63k stars 472 forks source link

IPC - Use After Free #1837

Open itayzafrir opened 1 month ago

itayzafrir commented 1 month ago

Describe the bug Use after free in nni_list_node_remove leads to process crash. This is a continuation of https://github.com/nanomsg/nng/issues/1827 and https://github.com/nanomsg/nng/pull/1831 which were closed. https://github.com/nanomsg/nng/pull/1834 did not fix the use after free issue - at least for the IPC.

Expected behavior No user after free, process does not crash.

Actual Behavior ipc_send_cb is called after the pipe has been already closed, causing use after free crash.

To Reproduce Use branch from https://github.com/itayzafrir/nng/tree/itay/new-use-after-free and run the demo/reqrep.c executable as server and another one as client.

Environment Details

Additional context Stack Trace of crash (same as before):

image image

Looks like aio has already been freed/deleted.

Another point, sometimes the client process crashes with a segfault. I wasn't able to debug this yet but it does happen from time to time.

gdamore commented 1 month ago

There is still indeed a use after free here. I spent pretty much most of last weekend trying to debug it. It seems like our attempt to unregister the method from the callbacks is not quite complete, or Windows is issuing a second completion callback.

We should not have had any outstanding requests when this callback was fired, as we try to ensure that there are no such requests pending or likely to be queued when we call close.

I'll have another look in the next day or two -- maybe something will occur.