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

[Jouni Malinen]

On Wed, May 09, 2012 at 12:09:23PM -0700, greearb at candelatech.com wrote:
> This is a hack to fix use-after-free when the
> first-enabled station is deleted.

Does the "station" in that refer to the wpa_supplicant interface?

> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
> +#include "p2p/p2p_i.h"

This won't fly (internal header file cannot be included here).

> +	/* 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).

Well, whether it is bad or not, there is an assumption on the first
wpa_supplicant interface being used for P2P management.

> +	 * 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.

I'm not sure I want to go as far as re-writing this part any time soon..

> +	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);
> +		}
> +	}

It's probable fine to disable P2P in case the first wpa_s instance goes
away (i.e., the interface that called wpas_p2p_init()).. Though, this
needs to be done bit more cleanly so that p2p_i.h does not need to get
included here (may need to maintain a pointer to the initializing wpa_s
within struct wpa_global)..

-- 
Jouni Malinen                                            PGP id EFC895FA