netdev - Re: [PATCH v2] wifi: cfg80211: Fix UAF in ieee80211_scan

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <182a5dabcc5.36efd4946461.1649818863091410857@siddh.me>
Date:   Tue, 16 Aug 2022 14:22:32 +0530
From:   Siddh Raman Pant <code@...dh.me>
To:     "Johannes Berg" <johannes@...solutions.net>
Cc:     "jakub kicinski" <kuba@...nel.org>,
        "greg kh" <gregkh@...uxfoundation.org>,
        "david s. miller" <davem@...emloft.net>,
        "eric dumazet" <edumazet@...gle.com>,
        "paolo abeni" <pabeni@...hat.com>,
        "netdev" <netdev@...r.kernel.org>,
        "linux-kernel-mentees" 
        <linux-kernel-mentees@...ts.linuxfoundation.org>
Subject: Re: [PATCH v2] wifi: cfg80211: Fix UAF in ieee80211_scan_rx()

On Tue, 16 Aug 2022 01:28:24 +0530  Johannes Berg  wrote:
> Sorry everyone, I always thought "this looks odd" and then never got
> around to taking a closer look.
> 
> So yeah, I still think this looks odd - cfg80211 doesn't really know
> anything about how mac80211 might be doing something with RCU to protect
> the pointer.
> 
> The patch also leaves the NULL-ing in mac80211 (that is how we reach it)
> broken wrt. the kfree_rcu() since it doesn't happen _before_, and the
> pointer in rdev isn't how this is reached through RCU (it's not even
> __rcu annotated).
> 

Thanks for the critical review.

> I think it might be conceptually better, though not faster, to do
> something like https://p.sipsolutions.net/1d23837f455dc4c2.txt which
> ensures that from mac80211's POV it can no longer be reached before we
> call cfg80211_scan_done().
> 
> Yeah, that's slower, but scanning is still a relatively infrequent (and
> slow anyway) operation, and this way we can stick to "this is not used
> once you call cfg80211_scan_done()" which just makes much more sense?
> 
> johannes
> 

Agreed, that looks like a good way to go about.

Thanks,
Siddh