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] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKXUXMzeSLuH31zQDe3Q_1YAvfmFR16ZsfFGmxEMiMQSKcp_Nw@mail.gmail.com>
Date:   Thu, 21 Jul 2022 12:35:01 +0200
From:   Lukas Bulwahn <lukas.bulwahn@...il.com>
To:     Siddh Raman Pant <code@...dh.me>
Cc:     Johannes Berg <johannes@...solutions.net>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        netdev <netdev@...r.kernel.org>,
        syzbot+6cb476b7c69916a0caca@...kaller.appspotmail.com,
        linux-wireless <linux-wireless@...r.kernel.org>,
        syzbot+f9acff9bf08a845f225d@...kaller.appspotmail.com,
        syzbot+9250865a55539d384347@...kaller.appspotmail.com,
        linux-kernel-mentees 
        <linux-kernel-mentees@...ts.linuxfoundation.org>
Subject: Re: [RESEND PATCH] net: Fix UAF in ieee80211_scan_rx()

On Thu, Jul 21, 2022 at 12:11 PM Siddh Raman Pant via
Linux-kernel-mentees <linux-kernel-mentees@...ts.linuxfoundation.org>
wrote:
>
> 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 5043 of mac80211/rx.c).
>
> Thus, add an rcu_head to the scan_req struct so as to use kfree_rcu()
> instead of kfree() so that we don't free during the critical section.
>
> Bug report (3): https://syzkaller.appspot.com/bug?extid=f9acff9bf08a845f225d
> Reported-by: syzbot+f9acff9bf08a845f225d@...kaller.appspotmail.com
> Reported-by: syzbot+6cb476b7c69916a0caca@...kaller.appspotmail.com
> Reported-by: syzbot+9250865a55539d384347@...kaller.appspotmail.com
>
> Signed-off-by: Siddh Raman Pant <code@...dh.me>
> ---
> Resending because didn't get any reply from maintainers for more
> than 2 weeks.
>

Siddh,

I had a look at the Bug report above. Currently, we do not have any
syzkaller or C reproducer to confirm that the bug was actually fixed.

Now, that you have a supposed fix for the issue:
Can you write a 'stress test' and (qemu) setup script that eventually
makes that bug trigger (e.g., if we run the stress test for two or
three days it will eventually trigger)? Then, we can also use that to
confirm that your patch fixes the issue (beyond the normal sanity code
review).

This is certainly something you can do on your side to move this patch
forward, and other developers with testing infrastructure can pick up
and confirm your tests and results independently.

I hope this helps, otherwise you will just need to have some patience.

Best regards,

Lukas

>  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 6d02e12e4702..ba4a49884de8 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 {
>         struct cfg80211_ssid *ssids;
> @@ -2397,6 +2398,7 @@ struct cfg80211_scan_request {
>         bool scan_6ghz;
>         u32 n_6ghz_params;
>         struct cfg80211_scan_6ghz_params *scan_6ghz_params;
> +       struct rcu_head rcu_head;
>
>         /* keep last */
>         struct ieee80211_channel *channels[];
> diff --git a/net/wireless/scan.c b/net/wireless/scan.c
> index 6d82bd9eaf8c..638b2805222c 100644
> --- a/net/wireless/scan.c
> +++ b/net/wireless/scan.c
> @@ -988,7 +988,7 @@ void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev,
>         kfree(rdev->int_scan_req);
>         rdev->int_scan_req = NULL;
>
> -       kfree(rdev->scan_req);
> +       kfree_rcu(rdev->scan_req, rcu_head);
>         rdev->scan_req = NULL;
>
>         if (!send_message)
> --
> 2.35.1
>
>
> _______________________________________________
> Linux-kernel-mentees mailing list
> Linux-kernel-mentees@...ts.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ