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

[Peer, Ilan]

Hi Jouni,

> -----Original Message-----
> From: Jouni Malinen [mailto:j at w1.fi]
> Sent: Monday, October 05, 2015 19:56
> To: Peer, Ilan
> Cc: hostap at lists.shmoo.com
> Subject: Re: [PATCH 11/23] P2P: Cleanup handling of unknown peer in PD
> request processing
> 
> 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..
>

I know see that it is not .. need to fix it. 

> > @@ -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?
> 

Thanks. I'll fix this. 

Ilan.