NLnetLabs / unbound

Unbound is a validating, recursive, and caching DNS resolver.
https://nlnetlabs.nl/unbound
BSD 3-Clause "New" or "Revised" License
3.15k stars 360 forks source link

Fix heap corruption when calling ub_ctx_delete in Windows #1157

Closed lnzhu closed 3 weeks ago

lnzhu commented 1 month ago

ub_ctx_delete in libunbound causes heap corruption exception in Windows OS. This issue is only for Windows and discovered when using the lastest libunbound Windows library published here.

gthess commented 1 month ago

Thanks, this looks good! But I don't think the #if defined is required. I see why you have it there, but always setting tube->ev_listen = NULL seems fine regardless if USE_MINI_EVENT is defined or not. Unless I miss something.

lnzhu commented 1 month ago

Thanks for reviewing! Yes, #if defined(USE_MINI_EVENT) is not strictly needed in current code. But tube->ev_listen is deleted in ub_winsock_unregister_wsaevent only if USE_MINI_EVENT is defined. I think it might be safer and future-proof to have the check before calling ub_winsock_unregister_wsaevent.

gthess commented 1 month ago

It is indeed only freed when USE_MINI_EVENT is defined. For the other cases (when not defined, or when ub_event_pluggable.c is used) setting tube->ev_listen = NULL always after the ub_winsock_unregister_wsaevent() call conveys the correct intent that this listener event is no more.

I also prefer that all the USE_MINI_EVENT #ifdefs are handled in ub_event(_pluggable).c and not exposed in tube.c.

lnzhu commented 4 weeks ago

removed #if defined in the last commit