[PATCH 11/23] P2P: Cleanup handling of unknown peer in PD request processing

[Jouni Malinen]

On Thu, Sep 24, 2015 at 08:38:01PM +0300, Ilan Peer wrote:
> If a P2P provision discovery request is received for an unknown peer,
> a new device entry is being added, but the flow continues without
> updating the local p2p_device pointer, requiring to check the pointer
> value before every access.
> 
> Change this, so once a device is added, the flow updates the local
> p2p_device pointer and avoids the checks later in the flow.

> diff --git a/src/p2p/p2p_pd.c b/src/p2p/p2p_pd.c
> @@ -330,8 +330,7 @@ static struct wpabuf * p2p_build_prov_disc_resp(struct p2p_data *p2p,
> -			if (dev)
> -				p2p_go_select_channel(p2p, dev, &tmp);
> +			p2p_go_select_channel(p2p, dev, &tmp);

Could you please clarify why this change is fine?
p2p_go_select_channel() dereferences the dev argument unconditionally..

> @@ -575,6 +574,19 @@ void p2p_process_prov_disc_req(struct p2p_data *p2p, const u8 *sa,
> +			p2p_parse_free(&msg);
> +			goto out;

And this "goto out" case has dev == NULL.

> +		if (!dev) {
> +			dev = p2p_get_device(p2p, sa);
> +			if (!dev) {
> +				p2p_dbg(p2p,
> +					"Provision Discovery device not found "
> +					MACSTR, MAC2STR(sa));
> +				p2p_parse_free(&msg);
> +				goto out;
> +			}

Just like this one..

Wouldn't this result in NULL pointer dereference in
p2p_go_select_channel() due to that p2p_build_prov_disc_resp() change?

-- 
Jouni Malinen                                            PGP id EFC895FA