Re: [PATCH v2] wifi: cfg80211: Fix UAF in ieee80211_scan_rx()

From: Jakub Kicinski <kuba@kernel.org>
To: Siddh Raman Pant <code@siddh.me>
Cc: syzbot+6cb476b7c69916a0caca@lists.linuxfoundation.org,
	"         
	<syzbot+6cb476b7c69916a0caca@syzkaller.appspotmail.com>, ,
	eric  dumazet <edumazet@google.com>,
	netdev <netdev@vger.kernel.org>,
	 johannes berg <johannes@sipsolutions.net>,
	paolo abeni <pabeni@redhat.com>,
	 "@lists.linuxfoundation.org, david@lists.linuxfoundation.org,
	s.miller@lists.linuxfoundation.org,
	" <davem@davemloft.net>"@lists.linuxfoundation.org
Subject: Re: [PATCH v2] wifi: cfg80211: Fix UAF in ieee80211_scan_rx()
Date: Mon, 15 Aug 2022 09:47:22 -0700	[thread overview]
Message-ID: <20220815094722.3c275087@kernel.org> (raw)
In-Reply-To: <182980137c6.5665bf61226802.3084448395277966678@siddh.me>

On Sat, 13 Aug 2022 21:49:52 +0530 Siddh Raman Pant wrote:
> On Sat, 13 Aug 2022 00:55:09 +0530  Jakub Kicinski  wrote:
> > Similarly to Greg, I'm not very familiar with the code base but one
> > sure way to move things forward would be to point out a commit which
> > broke things and put it in a Fixes tag. Much easier to validate a fix
> > by looking at where things went wrong.  
> 
> Thanks, I now looked at some history.
> 
> The following commit on 28 Sep 2020 put the kfree call before NULLing:
> c8cb5b854b40 ("nl80211/cfg80211: support 6 GHz scanning")
> 
> The following commit on 19 Nov 2014 introduces RCU:
> 6ea0a69ca21b ("mac80211: rcu-ify scan and scheduled scan request pointers")
> 
> The kfree call wasn't "rcu-ified" in this commit, and neither were
> RCU heads added.
> 
> The following commit on 18 Dec 2014 added RCU head for sched_scan_req:
> 31a60ed1e95a ("nl80211: Convert sched_scan_req pointer to RCU pointer")
> 
> It seems a similar thing might not have been done for scan_req, but I
> could have also missed commits.
> 
> So what should go into the fixes tag, if any? Probably 6ea0a69ca21b?

That'd be my instinct, too. But do add the full history analysis 
to the commit message.

> Also, I probably should use RCU_INIT_POINTER in this patch. Or should
> I make a patch somewhat like 31a60ed1e95a?

Yeah, IDK, I'm confused on what the difference between rdev and local
is. The crash site reads the pointer from local, so other of clearing
the pointer on rdev should not matter there. Hopefully wireless folks
can chime in on v3.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees