* [PATCH] wifi: mac80211: use frame link_id when adjusting link_sta on reorder release
@ 2026-11-08 21:55 Noah Weinberger
2026-11-09 22:41 ` netdev-bot+sashiko
0 siblings, 1 reply; 3+ messages in thread
From: Noah Weinberger @ 2026-11-08 21:55 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, netdev, linux-kernel, security
Hello,
In ieee80211_rx_handlers(), when frames are released from the BA reorder
buffer on timeout, rx->link_sta is re-fetched using the *previous*
rx->link_id instead of the frame's own status->link_id. For a legacy
(non-MLO) client on an MLO access point,
ieee80211_release_reorder_timeout() passes link_id = -1 and
ieee80211_rx_data_set_sta() never updates rx->link_id, so the adjust
block evaluates rcu_dereference(rx->sta->link[-1]).
link[-1] aliases deflink.pub, the last member of the struct embedded
directly before link[] in struct sta_info, which is always non-NULL - so
the existing NULL fallback cannot fire and rx->link_sta becomes a
struct ieee80211_link_sta * that the RX handlers then treat as a
struct link_sta_info *.
ieee80211_rx_h_sta_process() subsequently writes statistics through that
type-confused pointer (a fragments counter, a u64 byte counter fed with
the attacker-controlled frame length, signal EWMAs and chain-signal
values) at fixed offsets inside struct ieee80211_sta around deflink. The
corruption is intra-object and therefore invisible to KASAN; depending
on the exact layout it can reach the link[] pointer array, nmi,
nan_sched and the driver-private area that follow deflink. At minimum
this is a remotely triggered crash on MLO APs serving legacy clients;
worse outcomes are layout-dependent.
The one-word fix below uses the frame's own status->link_id, which also
restores the intended behaviour for MLD peers interleaving frames from
two links inside one reorder window (the current code fetches the link
STA of the *previous* frame's link).
Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), net/mac80211/rx.c:4243-4255 (adjust block), rx.c:4375-4404 (ieee80211_rx_data_set_sta), rx.c:4410-4426 (ieee80211_release_reorder_timeout).
Introduced in the v7.4 merge window (regression vs v7.3).
This was found during an assisted (AI-supported) static source review of
mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07); every code path cited below was re-verified by the
reporter against that tree. The patch below has NOT been compiled and has
NOT been runtime tested, and no reproducer was produced - the triggering
conditions are described above instead. Please treat this as a report for
verification rather than a battle-tested fix.
Fixes: [fill in before sending - on a full clone run:
git log -S 'rcu_dereference(rx->sta->link[rx->link_id])' --oneline -- net/mac80211/rx.c]
Reported-by: [your name and email]
---
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index ecee934..836d571 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -4244,7 +4244,7 @@ static void ieee80211_rx_handlers(struct ieee80211_rx_data *rx,
unlikely(rx->link_id != status->link_id)) {
if (rx->sta) {
rx->link_sta =
- rcu_dereference(rx->sta->link[rx->link_id]);
+ rcu_dereference(rx->sta->link[status->link_id]);
/* Link got disabled, just use deflink */
if (!rx->link_sta)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] wifi: mac80211: use frame link_id when adjusting link_sta on reorder release
2026-11-08 21:55 [PATCH] wifi: mac80211: use frame link_id when adjusting link_sta on reorder release Noah Weinberger
@ 2026-11-09 22:41 ` netdev-bot+sashiko
0 siblings, 0 replies; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-11-09 22:41 UTC (permalink / raw)
To: NoahWeinberger2003
Cc: johannes, linux-wireless, netdev, linux-kernel, security, kuba
Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 3
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] The commit message names the wrong trigger for the link[-1]
access.
- [Low] The commit has no Signed-off-by: line.
- [Low] The Fixes: tag is a literal placeholder ('Fixes: [fill in before
sending - on a full clone run: git log -S ...]').
- [Low] The commit message is a bug report or cover email, not a
changelog.
--
wifi: mac80211: use frame link_id when adjusting link_sta on reorder release
This patch changes the link-adjust block in ieee80211_rx_handlers(). When
a released frame arrived on a different link, rx->link_sta is now looked
up with the frame's own status->link_id instead of the previous
rx->link_id. This avoids indexing rx->sta->link[] with -1.
> In ieee80211_rx_handlers(), when frames are released from the BA reorder
> buffer on timeout, rx->link_sta is re-fetched using the *previous*
> rx->link_id instead of the frame's own status->link_id. For a legacy
> (non-MLO) client on an MLO access point,
> ieee80211_release_reorder_timeout() passes link_id = -1 and
> ieee80211_rx_data_set_sta() never updates rx->link_id, so the adjust
> block evaluates rcu_dereference(rx->sta->link[-1]).
[ ... ]
> nan_sched and the driver-private area that follow deflink. At minimum
> this is a remotely triggered crash on MLO APs serving legacy clients;
> worse outcomes are layout-dependent.
[Severity: Medium]
Is this the right trigger for the link[-1] access? For stations added
through nl80211 on an MLD interface, nl80211_new_station() requires a
link ID:
net/wireless/nl80211.c:nl80211_new_station() {
...
if (wdev->valid_links) {
if (params.link_sta_params.link_id < 0)
return -EINVAL;
...
}
ieee80211_add_station() then goes through sta_info_alloc_with_link().
__sta_info_alloc() sets valid_links for non-MLO stations as well:
if (link_id >= 0) {
sta_info_add_link(sta, link_id, &sta->deflink,
&sta->sta.deflink);
sta->sta.valid_links = BIT(link_id);
So ieee80211_release_reorder_timeout() picks a real link:
if (sta->sta.valid_links)
link_id = ffs(sta->sta.valid_links) - 1;
ieee80211_rx_data_set_sta()->ieee80211_rx_data_set_link() then sets
rx->link_id to the station's own link. That matches status->link_id, so on
this path a legacy client never enters the adjust block.
Before this patch, the caller that does reach rx->sta->link[-1] seems to
be ieee80211_mark_rx_ba_filtered_frames(), which always passes -1:
if (!ieee80211_rx_data_set_sta(&rx, sta, -1))
return;
On an MLD vif with a station that has valid_links set (MLD or legacy),
this leaves rx->link_id at -1. status->link_id is an unsigned 4-bit field,
so the first released frame always takes the mismatch branch.
In-tree, only iwlwifi mvm calls ieee80211_mark_rx_ba_filtered_frames().
MLO-capable mvm hardware sets SUPPORTS_REORDERING_BUFFER, so
___ieee80211_start_rx_ba_session() never assigns tid_rx and the function
returns early.
The reorder timeout path could only hit link[-1] if an MLD vif held a
station with valid_links == 0. nl80211 doesn't appear to create such a
station.
The code change looks correct for all of these paths. It also fixes the
MLD peer case, where stats go to the previous frame's link.
Could the changelog describe the actual trigger and its driver dependence,
instead of "a remotely triggered crash on MLO APs serving legacy clients"?
As written, it may mislead severity triage and stable backport decisions.
> Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), net/mac80211/rx.c:4243-4255 (adjust block), rx.c:4375-4404 (ieee80211_rx_data_set_sta), rx.c:4410-4426 (ieee80211_release_reorder_timeout).
> Introduced in the v7.4 merge window (regression vs v7.3).
>
> This was found during an assisted (AI-supported) static source review of
> mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07); every code path cited below was re-verified by the
> reporter against that tree. The patch below has NOT been compiled and has
> NOT been runtime tested, and no reproducer was produced - the triggering
> conditions are described above instead. Please treat this as a report for
> verification rather than a battle-tested fix.
[Severity: Low]
This isn't a bug, but the changelog reads like a cover email rather than
a commit message. It opens with "Hello,", refers to "The one-word fix
below" and "every code path cited below", and includes tree-specific rx.c
line ranges that will go stale once the patch is applied.
Could the greeting, the rx.c line references and the review and testing
notes move below the --- separator? The note that the patch is untested
is still needed, but it doesn't belong in the permanent changelog.
> Fixes: [fill in before sending - on a full clone run:
> git log -S 'rcu_dereference(rx->sta->link[rx->link_id])' --oneline -- net/mac80211/rx.c]
[Severity: Low]
The Fixes: tag is still a placeholder. git blame on the adjust block in
ieee80211_rx_handlers(), including the rx->sta->link[rx->link_id] line,
points to:
Fixes: 4db13371e217 ("wifi: mac80211: refactor RX link_id and station handling")
That commit also removed status->link_valid, started deriving
status->link_id from rx->link_id in ieee80211_prepare_and_rx_handle() and
ieee80211_invoke_fast_rx(), and left callers such as
ieee80211_mark_rx_ba_filtered_frames() passing link_id = -1.
> Reported-by: [your name and email]
[Severity: Low]
The trailers have no Signed-off-by:, and the Reported-by: is a
placeholder. Documentation/process/coding-assistants.rst says that when
an AI assistant was used, the human submitter must add their own
Signed-off-by to certify the DCO. submitting-patches.rst also requires
one.
Could a Signed-off-by be added, and the placeholder Reported-by: either
filled in or dropped?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/Sq80dyNRnQWE5H_yZYIwxaQiwgvIrIlET6kND66_K0UpnKQZruHroWDq6hlYKDM0y9e2vXW2UBlBmnjw3bCLKk1Acrp7F49_jk883tbCqxk%3D%40proton.me
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] wifi: mac80211: use frame link_id when adjusting link_sta on reorder release
[not found] <dYfRhWjvPLvnRlGAXf1svcifESfUexTeJbDV20GUaTUimkPONN3lwAZPHogH48gXZluH9txGQ4eJaswHEsWGshWlJxYvRlUPh0172G3W64Q=@proton.me>
@ 2026-11-09 4:41 ` Willy Tarreau
0 siblings, 0 replies; 3+ messages in thread
From: Willy Tarreau @ 2026-11-09 4:41 UTC (permalink / raw)
To: Noah Weinberger; +Cc: johannes, linux-wireless, netdev, linux-kernel, security
Hello,
On Sun, Nov 08, 2026 at 09:35:44PM +0000, Noah Weinberger wrote:
> Hello,
>
> In ieee80211_rx_handlers(), when frames are released from the BA reorder
> buffer on timeout, rx->link_sta is re-fetched using the *previous*
> rx->link_id instead of the frame's own status->link_id. For a legacy
> (non-MLO) client on an MLO access point,
> ieee80211_release_reorder_timeout() passes link_id = -1 and
> ieee80211_rx_data_set_sta() never updates rx->link_id, so the adjust
> block evaluates rcu_dereference(rx->sta->link[-1]).
(...)
Note, in the future please do not Cc security@ once public lists are
already in Cc, as there is no need for private coordination anymore.
thanks,
Willy
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-09 22:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-08 21:55 [PATCH] wifi: mac80211: use frame link_id when adjusting link_sta on reorder release Noah Weinberger
2026-11-09 22:41 ` netdev-bot+sashiko
[not found] <dYfRhWjvPLvnRlGAXf1svcifESfUexTeJbDV20GUaTUimkPONN3lwAZPHogH48gXZluH9txGQ4eJaswHEsWGshWlJxYvRlUPh0172G3W64Q=@proton.me>
2026-11-09 4:41 ` Willy Tarreau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®