Kernel/iwd
3
0
Fork 0
mirror of https://git.kernel.org/pub/scm/network/wireless/iwd.git synced 2025-10-31 13:17:25 +01:00
Code Releases Activity

handshake: Do not crash if handshake is destroyed

Browse Source 
Commit 4d2176df2985 ("handshake: Allow event handler to free handshake")
introduced a re-entrancy guard so that handshake_state objects that are
destroyed as a result of the event do not cause a crash.  It rightly
used a temporary object to store the passed in handshake.  Unfortunately
this caused variable shadowing which resulted in crashes fixed by commit
d22b174a7318 ("handshake: use _hs directly in handshake_event").
However, since the temporary was no longer used, this fix itself caused
a crash:

 #0  0x00005555f0ba8b3d in eapol_handle_ptk_1_of_4 (sm=sm@entry=0x5555f2b4a920, ek=0x5555f2b62588, ek@entry=0x16, unencrypted=unencrypted@entry=false) at src/eapol.c:1236
1236				handshake_event(sm->handshake,
(gdb) bt
 #0  0x00005555f0ba8b3d in eapol_handle_ptk_1_of_4 (sm=sm@entry=0x5555f2b4a920, ek=0x5555f2b62588, ek@entry=0x16, unencrypted=unencrypted@entry=false) at src/eapol.c:1236
 #1  0x00005555f0bab118 in eapol_key_handle (unencrypted=<optimized out>, frame=<optimized out>, sm=0x5555f2b4a920) at src/eapol.c:2343
 #2  eapol_rx_packet (proto=<optimized out>, from=<optimized out>, frame=<optimized out>, unencrypted=<optimized out>, user_data=0x5555f2b4a920) at src/eapol.c:2665
 #3  0x00005555f0bac497 in __eapol_rx_packet (ifindex=62, src=src@entry=0x5555f2b62574 "x\212 J\207\267", proto=proto@entry=34958, frame=frame@entry=0x5555f2b62588 "\002\003",
   len=len@entry=121, noencrypt=noencrypt@entry=false) at src/eapol.c:3017
 #4  0x00005555f0b8c617 in netdev_control_port_frame_event (netdev=0x5555f2b64450, msg=0x5555f2b62588) at src/netdev.c:5574
 #5  netdev_unicast_notify (msg=msg@entry=0x5555f2b619a0, user_data=<optimized out>) at src/netdev.c:5613
 #6  0x00007f60084c9a51 in dispatch_unicast_watches (msg=0x5555f2b619a0, id=<optimized out>, genl=0x5555f2b3fc80) at ell/genl.c:954
 #7  process_unicast (nlmsg=0x7fff61abeac0, genl=0x5555f2b3fc80) at ell/genl.c:973
 #8  received_data (io=<optimized out>, user_data=0x5555f2b3fc80) at ell/genl.c:1098
 #9  0x00007f60084c61bd in io_callback (fd=<optimized out>, events=1, user_data=0x5555f2b3fd20) at ell/io.c:120
 #10 0x00007f60084c536d in l_main_iterate (timeout=<optimized out>) at ell/main.c:478
 #11 0x00007f60084c543e in l_main_run () at ell/main.c:525
 #12 l_main_run () at ell/main.c:507
 #13 0x00007f60084c5670 in l_main_run_with_signal (callback=callback@entry=0x5555f0b89150 <signal_handler>, user_data=user_data@entry=0x0) at ell/main.c:647
 #14 0x00005555f0b886a4 in main (argc=<optimized out>, argv=<optimized out>) at src/main.c:532

This happens when the driver does not support rekeying, which causes iwd to
attempt a disconnect and re-connect.  The disconnect action is
taken during the event callback and destroys the underlying eapol state
machine.  Since a temporary isn't used, attempting to dereference
sm->handshake results in a crash.

Fix this by introducing a UNIQUE_ID macro which should prevent shadowing
and using a temporary variable as originally intended.

Fixes: d22b174a7318 ("handshake: use _hs directly in handshake_event")
Fixes: 4d2176df2985 ("handshake: Allow event handler to free handshake")
Reported-By: Toke Høiland-Jørgensen <toke@toke.dk>
Tested-by: Toke Høiland-Jørgensen <toke@toke.dk>
This commit is contained in:
Denis Kenzior 2022-02-02 10:54:52 -06:00
parent fe2272c112
commit 8a5b3f6880
2 changed files with 25 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
src/handshake.h
View File

@ -164,23 +164,27 @@ struct handshake_state {
handshake_event_func_t event_func;
};
#define handshake_event(_hs, event, ...) \
(__extension__ ({ \
bool freed = false; \
\
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)); \
freed = true; \
} else \
(_hs)->in_event = false; \
} \
freed; \
}))
#define HSID(x) UNIQUE_ID(handshake_, x)
#define handshake_event(_hs, event, ...) \
({ \
bool HSID(freed) = false; \
typeof(_hs) HSID(hs) = (_hs); \
\
if (HSID(hs)->event_func && !HSID(hs)->in_event) { \
HSID(hs)->in_event = true; \
HSID(hs)->event_func(HSID(hs), (event), \
HSID(hs)->user_data, \
##__VA_ARGS__); \
\
if (!HSID(hs)->in_event) { \
handshake_state_free(HSID(hs)); \
HSID(freed) = true; \
} else \
HSID(hs)->in_event = false; \
} \
HSID(freed); \
})
void handshake_state_free(struct handshake_state *s);

4
src/util.h
View File

@ -26,6 +26,10 @@
#include <stdint.h>
#include <unistd.h>
#define ___PASTE(a, b) a ## b
#define __PASTE(a, b) ___PASTE(a, b)
#define UNIQUE_ID(x, id) __PASTE(__unique_prefix_, __PASTE(x, id))
#define align_len(len, boundary) (((len)+(boundary)-1) & ~((boundary)-1))
#define MAC "%02x:%02x:%02x:%02x:%02x:%02x"