[RESEND PATCH] net: Fix UAF in ieee80211_scan_rx()

[RESEND PATCH] net: Fix UAF in ieee80211_scan_rx()

[Siddh Raman Pant]

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@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>
---
Resending because didn't get any reply from maintainers for more
than 2 weeks.

 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

[RESEND PATCH] net: Fix UAF in ieee80211_scan_rx()

[Siddh Raman Pant via Linux-kernel-mentees]

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@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>
---
Resending because didn't get any reply from maintainers for more
than 2 weeks.

 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@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [RESEND PATCH] net: Fix UAF in ieee80211_scan_rx()

[Lukas Bulwahn]

On Thu, Jul 21, 2022 at 12:11 PM Siddh Raman Pant via
Linux-kernel-mentees <linux-kernel-mentees@lists.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@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>
> ---
> 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@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [RESEND PATCH] net: Fix UAF in ieee80211_scan_rx()

[Lukas Bulwahn]

On Thu, Jul 21, 2022 at 12:11 PM Siddh Raman Pant via
Linux-kernel-mentees <linux-kernel-mentees@lists.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@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>
> ---
> 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@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [RESEND PATCH] net: Fix UAF in ieee80211_scan_rx()

[Siddh Raman Pant]

Hello Lukas,

Sorry for the late reply.

On Thu, 21 Jul 2022 16:05:01 +0530  Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote:
> 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 have been intermittently looking about this for the past few days. Since
such test creation is new to me, I am stuck at how to go about calling the
requisite function.

What I have gathered is that I need to use the netlink API or related tool
and issue the scan and recieve commands. Since qemu by default doesn't
have a WiFi interface setup by default, I was looking at simulation and
came across the mac80211_hwsim module. After building the kernel it, I
tried using `iw` command for scanning with the two phy simulated devices,
but I seem to hit a deadend due to not being able to properly use them
for the task at hand.

Do you have any resources or/and examples on such "stress tests"?

> I hope this helps, otherwise you will just need to have some patience.
> 
> Best regards,
> 
> Lukas

Eric had replied to me on the original email soon after, and I have sent
a v2. Though, I still want to see how people go about making the tests,
so any resources for further exploring will be useful.

Thanks,
Siddh

Re: [RESEND PATCH] net: Fix UAF in ieee80211_scan_rx()

[Siddh Raman Pant via Linux-kernel-mentees]

Hello Lukas,

Sorry for the late reply.

On Thu, 21 Jul 2022 16:05:01 +0530  Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote:
> 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 have been intermittently looking about this for the past few days. Since
such test creation is new to me, I am stuck at how to go about calling the
requisite function.

What I have gathered is that I need to use the netlink API or related tool
and issue the scan and recieve commands. Since qemu by default doesn't
have a WiFi interface setup by default, I was looking at simulation and
came across the mac80211_hwsim module. After building the kernel it, I
tried using `iw` command for scanning with the two phy simulated devices,
but I seem to hit a deadend due to not being able to properly use them
for the task at hand.

Do you have any resources or/and examples on such "stress tests"?

> I hope this helps, otherwise you will just need to have some patience.
> 
> Best regards,
> 
> Lukas

Eric had replied to me on the original email soon after, and I have sent
a v2. Though, I still want to see how people go about making the tests,
so any resources for further exploring will be useful.

Thanks,
Siddh
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees