diff options
| author | Johannes Berg <johannes.berg@intel.com> | 2026-09-04 16:55:03 +0200 |
|---|---|---|
| committer | Johannes Berg <johannes.berg@intel.com> | 2026-09-09 14:51:14 +0200 |
| commit | dab68a74e90b8e07f08ed9deaa5884857a3cfe89 (patch) | |
| tree | 4100ba5b449a6a020df73706ff01404fd5501668 | |
| parent | 48b2c5c628b09cf36cbeca53e0432fc2a7518be7 (diff) | |
| download | linux-next-dab68a74e90b8e07f08ed9deaa5884857a3cfe89.tar.gz linux-next-dab68a74e90b8e07f08ed9deaa5884857a3cfe89.zip | |
wifi: cfg80211: don't free driver-owned scan requests
When an interface goes down while a scan is running, cfg80211 completes
the scan towards userspace and frees the scan request. However, the
driver can be convinced that it owns the request, since the cancellation
is (intended to be) asynchronous.
The WARN_ON() in the netdev notifier was meant to catch this, but it's
not actually avoidable, so it triggers and we get a UAF in scan_done().
There doesn't seem to be a great way around it, so just track that the
driver is still convinced it owns the request, and then just free it on
completion if it was already cancelled. Also remove the warnings since
they can trigger in the intended architecture.
Assisted-by: LLM
Fixes: 4a58e7c38443 ("cfg80211: don't "leak" uncompleted scans")
Reported-by: syzbot+189dcafc06865d38178d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=189dcafc06865d38178d
Link: https://patch.msgid.link/20260904165614.375e543228b1.I03cbb5a54cb02d6bba5034286af1ed73aba134d1@changeid
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
| -rw-r--r-- | net/wireless/core.c | 11 | ||||
| -rw-r--r-- | net/wireless/core.h | 10 | ||||
| -rw-r--r-- | net/wireless/rdev-ops.h | 3 | ||||
| -rw-r--r-- | net/wireless/scan.c | 31 |
4 files changed, 47 insertions, 8 deletions
diff --git a/net/wireless/core.c b/net/wireless/core.c index d13310fef691..8bb2cbd66b48 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -244,9 +244,8 @@ void cfg80211_stop_p2p_device(struct cfg80211_registered_device *rdev, rdev->opencount--; if (rdev->scan_req && rdev->scan_req->req.wdev == wdev) { - if (WARN_ON(!rdev->scan_req->notified && - (!rdev->int_scan_req || - !rdev->int_scan_req->notified))) + if (!rdev->scan_req->notified && + (!rdev->int_scan_req || !rdev->int_scan_req->notified)) rdev->scan_req->info.aborted = true; ___cfg80211_scan_done(rdev, false); } @@ -1758,9 +1757,9 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb, wiphy_lock(&rdev->wiphy); cfg80211_update_iface_num(rdev, wdev->iftype, -1); if (rdev->scan_req && rdev->scan_req->req.wdev == wdev) { - if (WARN_ON(!rdev->scan_req->notified && - (!rdev->int_scan_req || - !rdev->int_scan_req->notified))) + if (!rdev->scan_req->notified && + (!rdev->int_scan_req || + !rdev->int_scan_req->notified)) rdev->scan_req->info.aborted = true; ___cfg80211_scan_done(rdev, false); } diff --git a/net/wireless/core.h b/net/wireless/core.h index b4610f6685dc..a0c2b6ebe31f 100644 --- a/net/wireless/core.h +++ b/net/wireless/core.h @@ -24,6 +24,16 @@ struct cfg80211_scan_request_int { struct cfg80211_scan_info info; bool notified; + /* + * set while the request is handed to the driver, i.e. between + * rdev_scan() and cfg80211_scan_done() + */ + bool driver_owns; + /* + * set when cfg80211 is done with the request but the driver still + * owns it, so that cfg80211_scan_done() knows to just free it + */ + bool stale; /* must be last - variable members */ struct cfg80211_scan_request req; }; diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h index 46849fe8d0b3..adcfd0278da3 100644 --- a/net/wireless/rdev-ops.h +++ b/net/wireless/rdev-ops.h @@ -464,7 +464,10 @@ static inline int rdev_scan(struct cfg80211_registered_device *rdev, return -EINVAL; trace_rdev_scan(&rdev->wiphy, request); + request->driver_owns = true; ret = rdev->ops->scan(&rdev->wiphy, &request->req); + if (ret) + request->driver_owns = false; trace_rdev_return_int(&rdev->wiphy, ret); return ret; } diff --git a/net/wireless/scan.c b/net/wireless/scan.c index 9e934b185e34..4fe114f6aee3 100644 --- a/net/wireless/scan.c +++ b/net/wireless/scan.c @@ -1114,6 +1114,21 @@ int cfg80211_scan(struct cfg80211_registered_device *rdev) return 0; } +/* + * Release the scan request, but free it only if the driver is also done, + * e.g. mac80211 may cancel it asynchronously and still use it. + */ +static void cfg80211_put_scan_req(struct cfg80211_scan_request_int *req) +{ + if (!req) + return; + + if (req->driver_owns) + req->stale = true; + else + kfree(req); +} + void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev, bool send_message) { @@ -1173,10 +1188,10 @@ void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev, dev_put(wdev->netdev); - kfree(rdev->int_scan_req); + cfg80211_put_scan_req(rdev->int_scan_req); rdev->int_scan_req = NULL; - kfree(rdev->scan_req); + cfg80211_put_scan_req(rdev->scan_req); rdev->scan_req = NULL; if (!send_message) @@ -1199,6 +1214,18 @@ void cfg80211_scan_done(struct cfg80211_scan_request *request, struct cfg80211_scan_info old_info = intreq->info; trace_cfg80211_scan_done(intreq, info); + + intreq->driver_owns = false; + + if (intreq->stale) { + /* + * The scan is already completed as far as we're concerned, + * it was just kept around for the driver - done now, free it. + */ + kfree(intreq); + return; + } + WARN_ON(intreq != rdev->scan_req && intreq != rdev->int_scan_req); |
