[PATCH] wifi: cfg80211: fix leak if split 6 GHz scanning fails

Fedor Pchelkin posted 1 patch 2 weeks, 1 day ago
net/wireless/scan.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
[PATCH] wifi: cfg80211: fix leak if split 6 GHz scanning fails
Posted by Fedor Pchelkin 2 weeks, 1 day ago
rdev->int_scan_req is leaked if cfg80211_scan() fails [1].  It's supposed
to be released at ___cfg80211_scan_done() but this doesn't happen as
rdev->scan_req is NULL at that point, too, leading to the early return
from the freeing function.  Anyway scan_done shouldn't ever be expected to
be called if triggering the scan has already failed.

Note that the similar pattern at cfg80211_scan_6ghz() looks okay since
int_scan_req is freed there on error in case first_part is being
processed.  On the other side, when first_part is false - called directly
from ___cfg80211_scan_done() - the old int_scan_req is kept then and will
be freed afterwards.

[1]:
unreferenced object 0xffff8881161d0800 (size 512):
  comm "wpa_supplicant", pid 379, jiffies 4294749765
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 f0 81 13 16 81 88 ff ff  ................
  backtrace (crc c867fdb6):
    kmemleak_alloc+0x89/0x90
    __kmalloc_noprof+0x2fd/0x410
    cfg80211_scan+0x133/0x730
    nl80211_trigger_scan+0xc69/0x1cc0
    genl_family_rcv_msg_doit+0x204/0x2f0
    genl_rcv_msg+0x431/0x6b0
    netlink_rcv_skb+0x143/0x3f0
    genl_rcv+0x27/0x40
    netlink_unicast+0x4f6/0x820
    netlink_sendmsg+0x797/0xce0
    __sock_sendmsg+0xc4/0x160
    ____sys_sendmsg+0x5e4/0x890
    ___sys_sendmsg+0xf8/0x180
    __sys_sendmsg+0x136/0x1e0
    __x64_sys_sendmsg+0x76/0xc0
    x64_sys_call+0x13f0/0x17d0

Found by Linux Verification Center (linuxtesting.org).

Fixes: c8cb5b854b40 ("nl80211/cfg80211: support 6 GHz scanning")
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
 net/wireless/scan.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 358cbc9e43d8..fe7b47d046c7 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -1071,6 +1071,7 @@ int cfg80211_scan(struct cfg80211_registered_device *rdev)
 	struct cfg80211_scan_request_int *request;
 	struct cfg80211_scan_request_int *rdev_req = rdev->scan_req;
 	u32 n_channels = 0, idx, i;
+	int err;
 
 	if (!(rdev->wiphy.flags & WIPHY_FLAG_SPLIT_SCAN_6GHZ)) {
 		rdev_req->req.first_part = true;
@@ -1101,7 +1102,12 @@ int cfg80211_scan(struct cfg80211_registered_device *rdev)
 	rdev_req->req.scan_6ghz = false;
 	rdev_req->req.first_part = true;
 	rdev->int_scan_req = request;
-	return rdev_scan(rdev, request);
+	err = rdev_scan(rdev, request);
+	if (err) {
+		kfree(rdev->int_scan_req);
+		rdev->int_scan_req = NULL;
+	}
+	return err;
 }
 
 void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev,
-- 
2.53.0
Re: [PATCH] wifi: cfg80211: fix leak if split 6 GHz scanning fails
Posted by Johannes Berg 1 week, 4 days ago
On Sun, 2026-05-24 at 19:53 +0300, Fedor Pchelkin wrote:
> 
> @@ -1101,7 +1102,12 @@ int cfg80211_scan(struct cfg80211_registered_device *rdev)
>  	rdev_req->req.scan_6ghz = false;
>  	rdev_req->req.first_part = true;
>  	rdev->int_scan_req = request;
> -	return rdev_scan(rdev, request);
> +	err = rdev_scan(rdev, request);
> +	if (err) {
> +		kfree(rdev->int_scan_req);
> +		rdev->int_scan_req = NULL;
> +	}
> +	return err;

Given that rdev isn't accessible to the driver call in rdev_scan(), I
think it'd be nicer to do kfree(request) and defer the int_scan_req
assignment to after the rdev_scan() call?

johannes
Re: [PATCH] wifi: cfg80211: fix leak if split 6 GHz scanning fails
Posted by Fedor Pchelkin 1 week ago
On Thu, 28. May 10:42, Johannes Berg wrote:
> On Sun, 2026-05-24 at 19:53 +0300, Fedor Pchelkin wrote:
> > 
> > @@ -1101,7 +1102,12 @@ int cfg80211_scan(struct cfg80211_registered_device *rdev)
> >  	rdev_req->req.scan_6ghz = false;
> >  	rdev_req->req.first_part = true;
> >  	rdev->int_scan_req = request;
> > -	return rdev_scan(rdev, request);
> > +	err = rdev_scan(rdev, request);
> > +	if (err) {
> > +		kfree(rdev->int_scan_req);
> > +		rdev->int_scan_req = NULL;
> > +	}
> > +	return err;
> 
> Given that rdev isn't accessible to the driver call in rdev_scan(), I
> think it'd be nicer to do kfree(request) and defer the int_scan_req
> assignment to after the rdev_scan() call?

It would differ now (at least visually) from the similar scheme used at
the end of cfg80211_scan_6ghz().  I'd go for that all this request
initialization stuff should be done before calling rdev_scan(), taking
that rdev_scan() may be changed in future or whatever.  But I don't have a
strong opinion here - will send v2 so that you may choose the eventually
preferred one between these versions.