From d22b174a7318401a3845ee6954d96b4376d0a4b9 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Wed, 26 Jan 2022 09:41:00 -0800 Subject: [PATCH] handshake: use _hs directly in handshake_event Fixes the following crash: #0 0x000211c4 in netdev_connect_event (msg=, netdev=0x2016940) at src/netdev.c:2915 #1 0x76f11220 in process_multicast (nlmsg=0x7e8acafc, group=, genl=) at ell/genl.c:1029 #2 received_data (io=, user_data=) at ell/genl.c:1096 #3 0x76f0da08 in io_callback (fd=, events=1, user_data=0x200a560) at ell/io.c:120 #4 0x76f0ca78 in l_main_iterate (timeout=) at ell/main.c:478 #5 0x76f0cb74 in l_main_run () at ell/main.c:525 #6 l_main_run () at ell/main.c:507 #7 0x76f0cdd4 in l_main_run_with_signal (callback=callback@entry=0x18c94 , user_data=user_data@entry=0x0) at ell/main.c:647 #8 0x00018178 in main (argc=, argv=) at src/main.c:532 This crash was introduced in commit: 4d2176df2985 ("handshake: Allow event handler to free handshake") The culprit seems to be that 'hs' is being used both in the caller and in the macro. Since the macro defines a variable 'hs' in local block scope, it overrides 'hs' from function scope. Yet (_hs) still evaluates to 'hs' leading the local variable to be initialized with itself. Only the 'handshake_event(hs, HANDSHAKE_EVENT_SETTING_KEYS))' is affected since it is the only macro invocation that uses 'hs' from function scope. Thus, the crash would only happen on hardware supporting handshake offload (brcmfmac). Fix this by removing the local scope variable declaration and evaluate (_hs) instead. Fixes: 4d2176df2985 ("handshake: Allow event handler to free handshake") --- src/handshake.h | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/handshake.h b/src/handshake.h index 34d4829d..7136087a 100644 --- a/src/handshake.h +++ b/src/handshake.h @@ -166,19 +166,18 @@ struct handshake_state { #define handshake_event(_hs, event, ...) \ (__extension__ ({ \ - struct handshake_state *hs = (_hs); \ bool freed = false; \ \ - if (hs->event_func && !hs->in_event) { \ - hs->in_event = true; \ - hs->event_func(hs, event, hs->user_data, \ + if ((_hs)->event_func && !(_hs)->in_event) { \ + (_hs)->in_event = true; \ + (_hs)->event_func((_hs), event, (_hs)->user_data, \ ##__VA_ARGS__); \ \ - if (!hs->in_event) { \ - handshake_state_free(hs); \ + if (!(_hs)->in_event) { \ + handshake_state_free((_hs)); \ freed = true; \ } else \ - hs->in_event = false; \ + (_hs)->in_event = false; \ } \ freed; \ }))