[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