[PATCH] supplicant/p2p: Fix use-after free crash.

greearb at candelatech.com greearb
Wed May 9 12:09:23 PDT 2012


From: Ben Greear <greearb at candelatech.com>

This is a hack to fix use-after-free when the
first-enabled station is deleted.  This needs to
be fixed properly, but at least now we do not
crash.  Valgrind report from one such crash:

For counts of detected and suppressed errors, rerun with: -v
==31996== ERROR SUMMARY: 59 errors from 59 contexts (suppressed: 6 from 6)
==31996== Invalid read of size 1
==31996==    at 0x3108E46781: vfprintf (in /lib64/libc-2.13.so)
==31996==    by 0x3108E6EF31: vsnprintf (in /lib64/libc-2.13.so)
==31996==    by 0x3108E4FBF2: snprintf (in /lib64/libc-2.13.so)
==31996==    by 0x40F995: wpa_msg (wpa_debug.c:613)
==31996==    by 0x433D37: p2p_stop_find_for_freq (p2p.c:1027)
==31996==    by 0x433EFD: p2p_stop_find (p2p.c:1069)
==31996==    by 0x437924: p2p_flush (p2p.c:2305)
==31996==    by 0x437829: p2p_deinit (p2p.c:2286)
==31996==    by 0x42B38C: wpas_p2p_deinit_global (p2p_supplicant.c:2571)
==31996==    by 0x4CA169: wpa_supplicant_deinit (wpa_supplicant.c:3049)
==31996==    by 0x4D5E08: main (main.c:288)
==31996==  Address 0x504661e is 46 bytes inside a block of size 1,600 free'd
==31996==    at 0x4A05187: free (vg_replace_malloc.c:325)
==31996==    by 0x4C9C2E: wpa_supplicant_remove_iface (wpa_supplicant.c:2830)
==31996==    by 0x4BA40E: wpa_supplicant_global_iface_remove (ctrl_iface.c:4441)
==31996==    by 0x4BA859: wpa_supplicant_global_ctrl_iface_process (ctrl_iface.c:4555)
==31996==    by 0x4BBE4D: wpa_supplicant_global_ctrl_iface_receive (ctrl_iface_unix.c:631)
==31996==    by 0x4115B0: eloop_sock_table_dispatch_table (eloop.c:335)
==31996==    by 0x41161D: eloop_sock_table_dispatch (eloop.c:352)
==31996==    by 0x4120EC: eloop_run (eloop.c:766)
==31996==    by 0x4CA13F: wpa_supplicant_run (wpa_supplicant.c:3028)
==31996==    by 0x4D5DF9: main (main.c:286)
==31996==

Signed-hostap: Ben Greear <greearb at candelatech.com>
---
 wpa_supplicant/wpa_supplicant.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 0996c38..dbf06b4 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -32,6 +32,7 @@
 #include "common/wpa_ctrl.h"
 #include "common/ieee802_11_defs.h"
 #include "p2p/p2p.h"
+#include "p2p/p2p_i.h"
 #include "blacklist.h"
 #include "wpas_glue.h"
 #include "wps_supplicant.h"
@@ -2692,6 +2693,8 @@ next_driver:
 static void wpa_supplicant_deinit_iface(struct wpa_supplicant *wpa_s,
 					int notify, int terminate)
 {
+	struct wpa_global *global = wpa_s->global;
+
 	if (wpa_s->drv_priv) {
 		wpa_supplicant_deauthenticate(wpa_s,
 					      WLAN_REASON_DEAUTH_LEAVING);
@@ -2702,6 +2705,31 @@ static void wpa_supplicant_deinit_iface(struct wpa_supplicant *wpa_s,
 
 	wpa_supplicant_cleanup(wpa_s);
 
+	/* P2P logic is bad:  It saves a pointer to wpa_s in global->p2p
+	 * in the wpas_p2p_init logic.  And, only does this for the first
+	 * station.  When a station goes away, the p2p logic is not fixed
+	 * up properly, leading to use-after-free issues when global->p2p->msg_ctx
+	 * or cb_ctx is used (at least).
+	 * I think p2p logic is going to need a serious re-write to get rid of
+	 * that global pointer or at least manage it better.  In the meantime,
+	 * this code attempts to at least stop crashes due to stale memory access.
+	 */
+	if (global->p2p && global->p2p->cfg &&
+	    (global->p2p->cfg->msg_ctx == wpa_s ||
+	     global->p2p->cfg->cb_ctx == wpa_s)) {
+		wpa_dbg(wpa_s, MSG_ERROR, "ERROR: global->p2p still has reference in deinit_iface()");
+		wpas_p2p_deinit(wpa_s);
+		/* If that didn't do the trick, I think we just have to
+		 * de-init everything.
+		 */
+		if (global->p2p && global->p2p->cfg &&
+		    (global->p2p->cfg->msg_ctx == wpa_s ||
+		     global->p2p->cfg->cb_ctx == wpa_s)) {
+			wpa_dbg(wpa_s, MSG_ERROR, "ERROR: global->p2p still has reference after p2p_deinit(), disabling p2p globally.");
+			wpas_p2p_deinit_global(global);
+		}
+	}
+
 	if (wpa_s->drv_priv)
 		wpa_drv_deinit(wpa_s);
 
-- 
1.7.3.4




More information about the Hostap mailing list