Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Siddh Raman Pant <code@siddh.me>
To:     Johannes Berg <johannes@sipsolutions.net>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>
Cc:     linux-wireless <linux-wireless@vger.kernel.org>,
        netdev <netdev@vger.kernel.org>,
        linux-kernel-mentees 
        <linux-kernel-mentees@lists.linuxfoundation.org>,
        syzbot+f9acff9bf08a845f225d@syzkaller.appspotmail.com,
        syzbot+6cb476b7c69916a0caca@syzkaller.appspotmail.com,
        syzbot+9250865a55539d384347@syzkaller.appspotmail.com
Message-ID: <20220726123921.29664-1-code@siddh.me>
Subject: [PATCH v2] wifi: cfg80211: Fix UAF in ieee80211_scan_rx()
Date:   Tue, 26 Jul 2022 18:09:21 +0530
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=utf8
Precedence: bulk

ieee80211_scan_rx() tries to access scan_req->flags after a null check
(see line 303 of mac80211/scan.c), but ___cfg80211_scan_done() uses
kfree() on the scan_req (see line 991 of wireless/scan.c).

This results in a UAF.

ieee80211_scan_rx() is called inside a RCU read-critical section
initiated by ieee80211_rx_napi() (see line 5044 of mac80211/rx.c).

Thus, add an rcu_head to the scan_req struct, so that we can use
kfree_rcu() instead of kfree() and thus not free during the critical
section.

We can clear the pointer before freeing here, since scan_req is
accessed using rcu_dereference().

Bug report (3): https://syzkaller.appspot.com/bug?extid=3Df9acff9bf08a845f2=
25d
Reported-by: syzbot+f9acff9bf08a845f225d@syzkaller.appspotmail.com
Reported-by: syzbot+6cb476b7c69916a0caca@syzkaller.appspotmail.com
Reported-by: syzbot+9250865a55539d384347@syzkaller.appspotmail.com

Signed-off-by: Siddh Raman Pant <code@siddh.me>
---
Changes since v1 as requested:
- Fixed commit heading and better commit message.
- Clear pointer before freeing.

 include/net/cfg80211.h | 2 ++
 net/wireless/scan.c    | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 80f41446b1f0..7e0b448c4cdb 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2368,6 +2368,7 @@ struct cfg80211_scan_6ghz_params {
  * @n_6ghz_params: number of 6 GHz params
  * @scan_6ghz_params: 6 GHz params
  * @bssid: BSSID to scan for (most commonly, the wildcard BSSID)
+ * @rcu_head: (internal) RCU head to use for freeing
  */
 struct cfg80211_scan_request {
 =09struct cfg80211_ssid *ssids;
@@ -2397,6 +2398,7 @@ struct cfg80211_scan_request {
 =09bool scan_6ghz;
 =09u32 n_6ghz_params;
 =09struct cfg80211_scan_6ghz_params *scan_6ghz_params;
+=09struct rcu_head rcu_head;
=20
 =09/* keep last */
 =09struct ieee80211_channel *channels[];
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 6d82bd9eaf8c..6cf58fe6dea0 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -988,8 +988,8 @@ void ___cfg80211_scan_done(struct cfg80211_registered_d=
evice *rdev,
 =09kfree(rdev->int_scan_req);
 =09rdev->int_scan_req =3D NULL;
=20
-=09kfree(rdev->scan_req);
 =09rdev->scan_req =3D NULL;
+=09kfree_rcu(rdev_req, rcu_head);
=20
 =09if (!send_message)
 =09=09rdev->scan_msg =3D msg;
--=20
2.35.1