| Message ID | ZEKkooQ5qbpFKAVS@ajax |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | don't try to access list members to free them unless already initialised | expand |
On Fri, Apr 21, 2023 at 10:58:42AM -0400, npiazza@disroot.org wrote: > taken from openbsd ports, and used by many linux distributions. > > https://github.com/openbsd/ports/blob/master/security/wpa_supplicant/patches/patch-src_utils_eloop_c > > does it make sense to include it in wpa_supplicant? Probably not unless someone can provide an explanation why that would be needed. > Don't try to access list members to free them unless already > initialised > > Index: src/utils/eloop.c > @@ -1254,6 +1254,9 @@ void eloop_destroy(void) > struct eloop_timeout *timeout, *prev; > struct os_reltime now; > > + if (eloop.timeout.prev == NULL) > + return; It is not valid to call eloop_destroy() without having first called eloop_init() successfully. As such, this condition should never be reached and should not be needed. If there are applications that do indeed have eloop.timeout.prev == NULL when eloop_destroy() is being called, those applications should be fixed instead.
On Fri, Apr 28, 2023 at 06:23:35PM +0300, Jouni Malinen wrote: > On Fri, Apr 21, 2023 at 10:58:42AM -0400, npiazza@disroot.org wrote: > > taken from openbsd ports, and used by many linux distributions. > > > > https://github.com/openbsd/ports/blob/master/security/wpa_supplicant/patches/patch-src_utils_eloop_c > > > > does it make sense to include it in wpa_supplicant? > > Probably not unless someone can provide an explanation why that would be > needed. > the only comment I could find is that it fixes a crash with wpa_priv usage();
Index: src/utils/eloop.c --- src/utils/eloop.c.orig +++ src/utils/eloop.c @@ -1254,6 +1254,9 @@ void eloop_destroy(void) struct eloop_timeout *timeout, *prev; struct os_reltime now; + if (eloop.timeout.prev == NULL) + return; + os_get_reltime(&now); dl_list_for_each_safe(timeout, prev, &eloop.timeout, struct eloop_timeout, list) {