| Message ID | 20210307214002.29126-1-andrei.otcheretianski@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | wpa_supplicant: Don't process EAPOLs while disconnecting | expand |
On Sun, Mar 07, 2021 at 11:40:01PM +0200, Andrei Otcheretianski wrote: > An EAPOL frame may be pending when the supplicant requests to > deauthenticate. At this stage the EAP SM cache is already cleaned by > calling eapol_sm_invalidate_cached_session(). Since at this stage the > wpa_supplicant's state is still set to associated, the EAPOL is > processed and results in a crash due to NULL dereference. > This wasn't seen previously as nl80211 wouldn't process the > NL80211_CMD_CONTROL_PORT_FRAME, since wpa_driver_nl80211_mlme() would > set the valid_handler to NULL. This behavior was changed in ab8929192 > ("nl80211: use the process_bss_event for the nl_connect handler"), > exposing this race. > Fix it by ignoring EAPOL frames while the deauthentication is in > progress. Thanks, applied. However, I was unable to reproduce that NULL dereference by trying to add calls to eapol_sm_invalidate_cached_session() in inconvenient places. Can you please provide more details on that crash and which pointer is being dereferenced? I'd like to add more protection against unexpected cases, but cannot do that here since I could not figure out where this NULL dereferencing could have happened.
> Thanks, applied. However, I was unable to reproduce that NULL dereference > by trying to add calls to > eapol_sm_invalidate_cached_session() in inconvenient places. Can you > please provide more details on that crash and which pointer is being > dereferenced? I'd like to add more protection against unexpected cases, but > cannot do that here since I could not figure out where this NULL > dereferencing could have happened. Hi, I was able to reproduce it rerunning eap_tls_errors() several times.. Here's the stack trace: 1615712557.476359: WPA_TRACE: eloop SIGSEGV - START 1615712557.476676: [1]: /home/tester/devel/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(+0x708d0) [0x55ca0fdcd8d0] 1615712557.476699: eloop_sigsegv_handler() ../src/utils/eloop.c:123 1615712557.476709: [2]: /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7f2a23cfef20] 1615712557.476723: [3]: /home/tester/devel/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(+0x15c369) [0x55ca0feb9369] 1615712557.476734: sm_EAP_SUCCESS_Enter() ../src/eap_peer/eap.c:1072 1615712557.476746: [4]: /home/tester/devel/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(eap_peer_sm_step+0x346) [0x55ca0feba206] 1615712557.476757: eap_peer_sm_step_idle() ../src/eap_peer/eap.c:1152 1615712557.476765: eap_peer_sm_step_local() ../src/eap_peer/eap.c:1280 1615712557.476773: sm_EAP_Step() ../src/eap_peer/eap.c:1365 1615712557.476781: eap_peer_sm_step() ../src/eap_peer/eap.c:2237 1615712557.476791: [5]: /home/tester/devel/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(eapol_sm_step+0x13c) [0x55ca0feb69dc] 1615712557.476802: eapol_sm_step() ../src/eapol_supp/eapol_supp_sm.c:999 1615712557.476812: [6]: /home/tester/devel/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(eapol_sm_rx_eapol+0x138) [0x55ca0feb7428] 1615712557.476840: eapol_sm_rx_eapol() ../src/eapol_supp/eapol_supp_sm.c:1293 1615712557.476852: [7]: /home/tester/devel/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(wpa_supplicant_rx_eapol+0x3d5) [0x55ca0ff6f1a5] 1615712557.476862: wpa_supplicant_rx_eapol() wpa_supplicant.c:4894 1615712557.476873: [8]: /home/tester/devel/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(wpa_supplicant_event+0xbf3) [0x55ca0ff829e3] 1615712557.476884: wpa_supplicant_event() events.c:5223 1615712557.476895: [9]: /home/tester/devel/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(process_bss_event+0x358) [0x55ca0ffa9f88] 1615712557.476905: drv_event_eapol_rx() ../src/drivers/driver.h:6068 1615712557.476913: nl80211_control_port_frame() ../src/drivers/driver_nl80211_event.c:2792 1615712557.476920: process_bss_event() ../src/drivers/driver_nl80211_event.c:3164 1615712557.476930: [10]: /lib/x86_64-linux-gnu/libnl-3.so.200(nl_recvmsgs_report+0x3cc) [0x7f2a2500ac1c] 1615712557.476941: [11]: /lib/x86_64-linux-gnu/libnl-3.so.200(nl_recvmsgs+0x9) [0x7f2a2500b049] 1615712557.476954: [12]: /home/tester/devel/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(+0x232cab) [0x55ca0ff8fcab] 1615712557.476966: send_and_recv() ../src/drivers/driver_nl80211.c:450 1615712557.476978: [13]: /home/tester/devel/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(wpa_driver_nl80211_mlme+0xcf) [0x55ca0ff9c9af] 1615712557.476988: wpa_driver_nl80211_mlme() ../src/drivers/driver_nl80211.c:3577 1615712557.476999: [14]: /home/tester/devel/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(+0x240a40) [0x55ca0ff9da40] 1615712557.477010: wpa_driver_nl80211_deauthenticate() ../src/drivers/driver_nl80211.c:3635 1615712557.477021: [15]: /home/tester/devel/iwlwifi-hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(wpa_supplicant_deauthenticate+0x1e7) [0x55ca0ff71da7] 1615712557.477033: memset() usr/include/x86_64-linux-gnu/bits/string_fortified.h:71 1615712557.477043: wpa_supplicant_deauthenticate() wpa_supplicant.c:4022 1615712557.477051: WPA_TRACE: eloop SIGSEGV - END The full log is attached. Thanks, Andrei > > -- > Jouni Malinen PGP id EFC895FA
On Sun, Mar 14, 2021 at 09:25:28AM +0000, Otcheretianski, Andrei wrote: > I was able to reproduce it rerunning eap_tls_errors() several times.. > Here's the stack trace: > 1615712557.476734: sm_EAP_SUCCESS_Enter() ../src/eap_peer/eap.c:1072 Thanks. I was not really able to reproduce this with eap_tls_errors no matter what I tried, but I did manage to change the timing both in the test script and wpa_supplicant to be able to trigger this. The real issue here was caused by an earlier change where code was moved to the EAP SUCCESS state handler without including the same checks for the context state still being present before deferencing the pointers in that location. Your patch is fine as-is, but it is just hiding the real issue, so I'll add a fix for this regression as well so that this cannot be triggered again regardless of how the code that calls into the EAP state machine behaves with call and event order.
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c index 6a02ed7c56..c8dcef21bd 100644 --- a/wpa_supplicant/wpa_supplicant.c +++ b/wpa_supplicant/wpa_supplicant.c @@ -4773,6 +4773,11 @@ void wpa_supplicant_rx_eapol(void *ctx, const u8 *src_addr, wpa_dbg(wpa_s, MSG_DEBUG, "RX EAPOL from " MACSTR, MAC2STR(src_addr)); wpa_hexdump(MSG_MSGDUMP, "RX EAPOL", buf, len); + if (wpa_s->own_disconnect_req) { + wpa_printf(MSG_DEBUG, "RX EAPOL - drop EAPOL frame as we are disconnecting"); + return; + } + #ifdef CONFIG_TESTING_OPTIONS if (wpa_s->ignore_auth_resp) { wpa_printf(MSG_INFO, "RX EAPOL - ignore_auth_resp active!");
An EAPOL frame may be pending when the supplicant requests to deauthenticate. At this stage the EAP SM cache is already cleaned by calling eapol_sm_invalidate_cached_session(). Since at this stage the wpa_supplicant's state is still set to associated, the EAPOL is processed and results in a crash due to NULL dereference. This wasn't seen previously as nl80211 wouldn't process the NL80211_CMD_CONTROL_PORT_FRAME, since wpa_driver_nl80211_mlme() would set the valid_handler to NULL. This behavior was changed in ab8929192 ("nl80211: use the process_bss_event for the nl_connect handler"), exposing this race. Fix it by ignoring EAPOL frames while the deauthentication is in progress. Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com> --- wpa_supplicant/wpa_supplicant.c | 5 +++++ 1 file changed, 5 insertions(+)