* [PATCH 1/1] wifi: mac80211: ignore PM bit in non-bufferable MMPDUs for PS start
@ 2026-09-28 10:05 Lee Jones
2026-09-28 10:15 ` Johannes Berg
0 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2026-09-28 10:05 UTC (permalink / raw)
To: lee, Johannes Berg, Emmanuel Grumbach, Luca Coelho,
linux-wireless, linux-kernel
Per IEEE 802.11, the Power Management subfield in the Frame Control
field is reserved in non-bufferable management frames (such as
Authentication, Association Request, and Reassociation Request) and a
station remains in Active mode during authentication and association.
When commit 9fef65443388 ("mac80211: always update the PM state of a
peer on MGMT / DATA frames") allowed non-bufferable management frames
to update peer power-save state so that re-authenticating stations
could transition from doze to awake (sta_ps_end()), it also allowed
non-bufferable management frames with the Power Management bit set to
transition an awake station into power-save mode (sta_ps_start()).
Restrict wake-to-doze transitions (sta_ps_start()) in
ieee80211_rx_h_sta_process() to data, action, disassociation, and
deauthentication frames using hdr->frame_control (avoiding inspecting
encrypted action frame payloads prior to ieee80211_rx_h_decrypt()) while
preserving doze-to-wake transitions (sta_ps_end()).
Fixes: 9fef65443388 ("mac80211: always update the PM state of a peer on MGMT / DATA frames")
Signed-off-by: Lee Jones <lee@kernel.org>
---
net/mac80211/rx.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index f5893f9c07c6..8e90d0d7b87f 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1924,7 +1924,11 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
if (!ieee80211_has_pm(hdr->frame_control))
sta_ps_end(sta);
} else {
- if (ieee80211_has_pm(hdr->frame_control))
+ if (ieee80211_has_pm(hdr->frame_control) &&
+ (ieee80211_is_data(hdr->frame_control) ||
+ ieee80211_is_action(hdr->frame_control) ||
+ ieee80211_is_disassoc(hdr->frame_control) ||
+ ieee80211_is_deauth(hdr->frame_control)))
sta_ps_start(sta);
}
}
--
2.56.0.rc1.315.gc6ed9934b7-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] wifi: mac80211: ignore PM bit in non-bufferable MMPDUs for PS start
2026-09-28 10:05 [PATCH 1/1] wifi: mac80211: ignore PM bit in non-bufferable MMPDUs for PS start Lee Jones
@ 2026-09-28 10:15 ` Johannes Berg
2026-09-28 14:24 ` Lee Jones
0 siblings, 1 reply; 8+ messages in thread
From: Johannes Berg @ 2026-09-28 10:15 UTC (permalink / raw)
To: Lee Jones, Emmanuel Grumbach, Luca Coelho, linux-wireless,
linux-kernel
On Mon, 2026-09-28 at 10:05 +0000, Lee Jones wrote:
> Per IEEE 802.11, the Power Management subfield in the Frame Control
> field is reserved in non-bufferable management frames (such as
> Authentication, Association Request, and Reassociation Request) and a
> station remains in Active mode during authentication and association.
>
> When commit 9fef65443388 ("mac80211: always update the PM state of a
> peer on MGMT / DATA frames") allowed non-bufferable management frames
> to update peer power-save state so that re-authenticating stations
> could transition from doze to awake (sta_ps_end()), it also allowed
> non-bufferable management frames with the Power Management bit set to
> transition an awake station into power-save mode (sta_ps_start()).
>
> Restrict wake-to-doze transitions (sta_ps_start()) in
> ieee80211_rx_h_sta_process() to data, action, disassociation, and
> deauthentication frames using hdr->frame_control (avoiding inspecting
> encrypted action frame payloads prior to ieee80211_rx_h_decrypt()) while
> preserving doze-to-wake transitions (sta_ps_end()).
>
This doesn't make sense to me - you don't really say why you're making
this change other than saying what the spec says, but then you're
explicitly not spec compliant both ways - without any explanation either
way?
johannes
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] wifi: mac80211: ignore PM bit in non-bufferable MMPDUs for PS start
2026-09-28 10:15 ` Johannes Berg
@ 2026-09-28 14:24 ` Lee Jones
2026-09-29 16:32 ` Johannes Berg
0 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2026-09-28 14:24 UTC (permalink / raw)
To: Johannes Berg
Cc: Emmanuel Grumbach, Luca Coelho, linux-wireless, linux-kernel
On Mon, 28 Sep 2026, Johannes Berg wrote:
> On Mon, 2026-09-28 at 10:05 +0000, Lee Jones wrote:
> > Per IEEE 802.11, the Power Management subfield in the Frame Control
> > field is reserved in non-bufferable management frames (such as
> > Authentication, Association Request, and Reassociation Request) and a
> > station remains in Active mode during authentication and association.
> >
> > When commit 9fef65443388 ("mac80211: always update the PM state of a
> > peer on MGMT / DATA frames") allowed non-bufferable management frames
> > to update peer power-save state so that re-authenticating stations
> > could transition from doze to awake (sta_ps_end()), it also allowed
> > non-bufferable management frames with the Power Management bit set to
> > transition an awake station into power-save mode (sta_ps_start()).
> >
> > Restrict wake-to-doze transitions (sta_ps_start()) in
> > ieee80211_rx_h_sta_process() to data, action, disassociation, and
> > deauthentication frames using hdr->frame_control (avoiding inspecting
> > encrypted action frame payloads prior to ieee80211_rx_h_decrypt()) while
> > preserving doze-to-wake transitions (sta_ps_end()).
> >
>
> This doesn't make sense to me - you don't really say why you're making
> this change other than saying what the spec says, but then you're
> explicitly not spec compliant both ways - without any explanation either
> way?
Fair point, sorry for that.
The commit message was purposely vague due to the fact that the change
fixes a security issue and I didn't want to publish a HOWTO guide for
exploiting it. However, I obviously overdid it a little and left out
the reasoning you'd need to review the patch. The short version is that
an unauthenticated peer can currently change an associated station's
power save state with frames that shouldn't be able to have that
capability which can be used against the station.
Happy to send you the details off-list.
Plans for v2 with your blessing:
- Rewrite the commit message to explain the problem and the reasoning
properly, rather than just quoting the spec.
- Make the PM handling consistent in both directions, so it doesn't
honour the bit in one case and ignore it in the other, while keeping
the re-authenticating station case from 9fef65443388 working.
Does that work for you?
--
Lee Jones
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] wifi: mac80211: ignore PM bit in non-bufferable MMPDUs for PS start
2026-09-28 14:24 ` Lee Jones
@ 2026-09-29 16:32 ` Johannes Berg
2026-10-20 6:58 ` Lee Jones
0 siblings, 1 reply; 8+ messages in thread
From: Johannes Berg @ 2026-09-29 16:32 UTC (permalink / raw)
To: Lee Jones; +Cc: Emmanuel Grumbach, Luca Coelho, linux-wireless, linux-kernel
On Mon, 2026-09-28 at 15:24 +0100, Lee Jones wrote:
>
> The commit message was purposely vague due to the fact that the change
> fixes a security issue and I didn't want to publish a HOWTO guide for
> exploiting it. However, I obviously overdid it a little and left out
> the reasoning you'd need to review the patch. The short version is that
> an unauthenticated peer can currently change an associated station's
> power save state with frames that shouldn't be able to have that
> capability which can be used against the station.
>
> Happy to send you the details off-list.
I think I've (conceptually) known about this issue for ... years?
First, arguably wake-to-doze are _less_ interesting to attack, since
then the TIM bit will end up getting set and if the STA isn't sleeping
it'll see the beacon and recover fairly easily (though not entirely sure
we actually implement that in mac80211), and if it keeps sending traffic
then the state corruption is short-lived.
OTOH, corrupting doze-to-wake could lead to traffic getting dropped if
TX filtering status isn't implemented well on the AP side/driver since
the STA is actually sleeping and won't see any frames.
Second, I don't think in general this issue is fixable, and while it's
good to implement this closer to the spec, security wise it doesn't
really do anything as is?
If, for example, there's any bufferable MMPDU the attacker can just
replay it - we've been doing this before MIC/replay check forever
(unless that's offloaded). It can also - although this is harder -
intercept and modify the PM bit, since it's not protected by the AAD.
It's also wrong that we're doing this *after* the reorder buffer, since
that potentially holds the frames for a long time, and the spec says to
look at the PM bit after the frame exchange sequence completes (REVmf
11.2.3.2). We should fix that too, but it doesn't really matter for this
discussion:
Any unprotected complete frame exchange sequence can modify the state
anyway, QoS NDP for example.
> - Rewrite the commit message to explain the problem and the reasoning
> properly, rather than just quoting the spec.
>
> - Make the PM handling consistent in both directions, so it doesn't
> honour the bit in one case and ignore it in the other, while keeping
> the re-authenticating station case from 9fef65443388 working.
>
> Does that work for you?
Well, I think it's really only a spec compliance cleanup, but then we
might as well try to fix some of the other issues?
I also think we might be better off making sure that as a client we
handle the TIM indicating traffic while awake and send a wake frame, and
perhaps checking AP side handles filtered/un-acked traffic well.
johannes
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] wifi: mac80211: ignore PM bit in non-bufferable MMPDUs for PS start
2026-09-29 16:32 ` Johannes Berg
@ 2026-10-20 6:58 ` Lee Jones
2026-10-20 7:42 ` Johannes Berg
0 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2026-10-20 6:58 UTC (permalink / raw)
To: Johannes Berg
Cc: Emmanuel Grumbach, Luca Coelho, linux-wireless, linux-kernel
Sorry for the delay. It's been a busy few weeks.
> On Mon, 2026-09-28 at 15:24 +0100, Lee Jones wrote:
> >
> > The commit message was purposely vague due to the fact that the change
> > fixes a security issue and I didn't want to publish a HOWTO guide for
> > exploiting it. However, I obviously overdid it a little and left out
> > the reasoning you'd need to review the patch. The short version is that
> > an unauthenticated peer can currently change an associated station's
> > power save state with frames that shouldn't be able to have that
> > capability which can be used against the station.
> >
> > Happy to send you the details off-list.
>
> I think I've (conceptually) known about this issue for ... years?
>
> First, arguably wake-to-doze are _less_ interesting to attack, since
> then the TIM bit will end up getting set and if the STA isn't sleeping
> it'll see the beacon and recover fairly easily (though not entirely sure
> we actually implement that in mac80211), and if it keeps sending traffic
> then the state corruption is short-lived.
> OTOH, corrupting doze-to-wake could lead to traffic getting dropped if
> TX filtering status isn't implemented well on the AP side/driver since
> the STA is actually sleeping and won't see any frames.
>
> Second, I don't think in general this issue is fixable, and while it's
> good to implement this closer to the spec, security wise it doesn't
> really do anything as is?
>
> If, for example, there's any bufferable MMPDU the attacker can just
> replay it - we've been doing this before MIC/replay check forever
> (unless that's offloaded). It can also - although this is harder -
> intercept and modify the PM bit, since it's not protected by the AAD.
>
> It's also wrong that we're doing this *after* the reorder buffer, since
> that potentially holds the frames for a long time, and the spec says to
> look at the PM bit after the frame exchange sequence completes (REVmf
> 11.2.3.2). We should fix that too, but it doesn't really matter for this
> discussion:
>
> Any unprotected complete frame exchange sequence can modify the state
> anyway, QoS NDP for example.
>
> > - Rewrite the commit message to explain the problem and the reasoning
> > properly, rather than just quoting the spec.
> >
> > - Make the PM handling consistent in both directions, so it doesn't
> > honour the bit in one case and ignore it in the other, while keeping
> > the re-authenticating station case from 9fef65443388 working.
> >
> > Does that work for you?
>
> Well, I think it's really only a spec compliance cleanup, but then we
> might as well try to fix some of the other issues?
>
> I also think we might be better off making sure that as a client we
> handle the TIM indicating traffic while awake and send a wake frame, and
> perhaps checking AP side handles filtered/un-acked traffic well.
Agreed on both counts.
Since the PM bit in the Frame Control header is unauthenticated and
processed prior to decryption, an unauthenticated QoS Null frame can
still flip the AP's PS state anyway, so the AP-side change is best
treated as a spec-compliance cleanup.
You were also right to suspect the client-side TIM path in
ieee80211_rx_mgmt_beacon(), when (dynamic_ps_timeout == 0) (for instance,
when power-save is disabled on the client interface) and IEEE80211_CONF_PS
is not set, mac80211 currently ignores its AID bit in the TIM and never
sends a wake nullfunc frame to flush the AP's buffered frames.
If you're happy for me to do so, I'll send the following 2-patch v3 series:
1. Symmetric PM-bit handling in ieee80211_rx_h_sta_process() (only
consulting the PM bit on data and bufferable management frames, while
waking sleeping stations on Auth / (Re)AssocReq as-per 9fef65443388).
2. Sending a wake nullfunc in ieee80211_rx_mgmt_beacon() when the TIM
indicates buffered unicast traffic while IEEE80211_CONF_PS is not set.
Does this help or hinder? Your call. If you decline, I'll go ahead and
close out our internal (Android) report as `Won't Fix` and move on to
something else.
Thanks.
--
Lee Jones
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] wifi: mac80211: ignore PM bit in non-bufferable MMPDUs for PS start
2026-10-20 6:58 ` Lee Jones
@ 2026-10-20 7:42 ` Johannes Berg
2026-10-20 7:47 ` Grumbach, Emmanuel
0 siblings, 1 reply; 8+ messages in thread
From: Johannes Berg @ 2026-10-20 7:42 UTC (permalink / raw)
To: Lee Jones; +Cc: Emmanuel Grumbach, Luca Coelho, linux-wireless, linux-kernel
On Tue, 2026-10-20 at 07:58 +0100, Lee Jones wrote:
>
> You were also right to suspect the client-side TIM path in
> ieee80211_rx_mgmt_beacon(), when (dynamic_ps_timeout == 0) (for instance,
> when power-save is disabled on the client interface) and IEEE80211_CONF_PS
> is not set, mac80211 currently ignores its AID bit in the TIM and never
> sends a wake nullfunc frame to flush the AP's buffered frames.
>
> If you're happy for me to do so, I'll send the following 2-patch v3 series:
>
> 1. Symmetric PM-bit handling in ieee80211_rx_h_sta_process() (only
> consulting the PM bit on data and bufferable management frames, while
> waking sleeping stations on Auth / (Re)AssocReq as-per 9fef65443388).
>
> 2. Sending a wake nullfunc in ieee80211_rx_mgmt_beacon() when the TIM
> indicates buffered unicast traffic while IEEE80211_CONF_PS is not set.
>
> Does this help or hinder? Your call. If you decline, I'll go ahead and
> close out our internal (Android) report as `Won't Fix` and move on to
> something else.
I think what you do with your report is probably not my problem ;-)
IMHO adding the client side defence definitely makes sense, though in
many/most cases it'll be up to the firmware anyway.
I don't think I have a problem with the AP-side spec alignment, even if
it's a touchy area, but depends how complex the code gets perhaps? I
don't think any client would rely on us not handling the PM bit in those
frames, and they're supposed to set it to 0 (reserved) anyway, so I'm
not very worried about it. The patch was fine and I guess moving that
around to apply to both sides wouldn't really change it much? Want to
try?
johannes
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] wifi: mac80211: ignore PM bit in non-bufferable MMPDUs for PS start
2026-10-20 7:42 ` Johannes Berg
@ 2026-10-20 7:47 ` Grumbach, Emmanuel
2026-10-20 9:54 ` Lee Jones
0 siblings, 1 reply; 8+ messages in thread
From: Grumbach, Emmanuel @ 2026-10-20 7:47 UTC (permalink / raw)
To: lee@kernel.org, johannes@sipsolutions.net
Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
Coelho, Luciano
On Tue, 2026-10-20 at 09:42 +0200, Johannes Berg wrote:
> On Tue, 2026-10-20 at 07:58 +0100, Lee Jones wrote:
> >
> > You were also right to suspect the client-side TIM path in
> > ieee80211_rx_mgmt_beacon(), when (dynamic_ps_timeout == 0) (for
> > instance,
> > when power-save is disabled on the client interface) and
> > IEEE80211_CONF_PS
> > is not set, mac80211 currently ignores its AID bit in the TIM and
> > never
> > sends a wake nullfunc frame to flush the AP's buffered frames.
> >
> > If you're happy for me to do so, I'll send the following 2-patch v3
> > series:
> >
> > 1. Symmetric PM-bit handling in ieee80211_rx_h_sta_process() (only
> > consulting the PM bit on data and bufferable management frames,
> > while
> > waking sleeping stations on Auth / (Re)AssocReq as-per
> > 9fef65443388).
> >
> > 2. Sending a wake nullfunc in ieee80211_rx_mgmt_beacon() when the
> > TIM
> > indicates buffered unicast traffic while IEEE80211_CONF_PS is
> > not set.
> >
> > Does this help or hinder? Your call. If you decline, I'll go
> > ahead and
> > close out our internal (Android) report as `Won't Fix` and move on
> > to
> > something else.
>
> I think what you do with your report is probably not my problem ;-)
>
> IMHO adding the client side defence definitely makes sense, though in
> many/most cases it'll be up to the firmware anyway.
>
> I don't think I have a problem with the AP-side spec alignment, even
> if
> it's a touchy area, but depends how complex the code gets perhaps? I
> don't think any client would rely on us not handling the PM bit in
> those
> frames, and they're supposed to set it to 0 (reserved) anyway, so I'm
> not very worried about it. The patch was fine and I guess moving that
> around to apply to both sides wouldn't really change it much? Want to
> try?
>
FWIW: the firmware on Intel devices will send an NDP with PM=0 when we
get a TIM bit set even if we are already awake. This is meant to re-
sync the AP in case we are out of sync. So 2. makes sense to me.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] wifi: mac80211: ignore PM bit in non-bufferable MMPDUs for PS start
2026-10-20 7:47 ` Grumbach, Emmanuel
@ 2026-10-20 9:54 ` Lee Jones
0 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2026-10-20 9:54 UTC (permalink / raw)
To: Grumbach, Emmanuel
Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org, Coelho, Luciano
On Tue, 20 Oct 2026, Grumbach, Emmanuel wrote:
> On Tue, 2026-10-20 at 09:42 +0200, Johannes Berg wrote:
> > On Tue, 2026-10-20 at 07:58 +0100, Lee Jones wrote:
> > >
> > > You were also right to suspect the client-side TIM path in
> > > ieee80211_rx_mgmt_beacon(), when (dynamic_ps_timeout == 0) (for
> > > instance,
> > > when power-save is disabled on the client interface) and
> > > IEEE80211_CONF_PS
> > > is not set, mac80211 currently ignores its AID bit in the TIM and
> > > never
> > > sends a wake nullfunc frame to flush the AP's buffered frames.
> > >
> > > If you're happy for me to do so, I'll send the following 2-patch v3
> > > series:
> > >
> > > 1. Symmetric PM-bit handling in ieee80211_rx_h_sta_process() (only
> > > consulting the PM bit on data and bufferable management frames,
> > > while
> > > waking sleeping stations on Auth / (Re)AssocReq as-per
> > > 9fef65443388).
> > >
> > > 2. Sending a wake nullfunc in ieee80211_rx_mgmt_beacon() when the
> > > TIM
> > > indicates buffered unicast traffic while IEEE80211_CONF_PS is
> > > not set.
> > >
> > > Does this help or hinder? Your call. If you decline, I'll go
> > > ahead and
> > > close out our internal (Android) report as `Won't Fix` and move on
> > > to
> > > something else.
> >
> > I think what you do with your report is probably not my problem ;-)
Yeah, TMI, sorry! =:-)
> > IMHO adding the client side defence definitely makes sense, though in
> > many/most cases it'll be up to the firmware anyway.
> >
> > I don't think I have a problem with the AP-side spec alignment, even
> > if
> > it's a touchy area, but depends how complex the code gets perhaps? I
> > don't think any client would rely on us not handling the PM bit in
> > those
> > frames, and they're supposed to set it to 0 (reserved) anyway, so I'm
> > not very worried about it. The patch was fine and I guess moving that
> > around to apply to both sides wouldn't really change it much? Want to
> > try?
> >
>
> FWIW: the firmware on Intel devices will send an NDP with PM=0 when we
> get a TIM bit set even if we are already awake. This is meant to re-
> sync the AP in case we are out of sync. So 2. makes sense to me.
Okay, let me follow-up. Bear with.
--
Lee Jones
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-20 9:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-28 10:05 [PATCH 1/1] wifi: mac80211: ignore PM bit in non-bufferable MMPDUs for PS start Lee Jones
2026-09-28 10:15 ` Johannes Berg
2026-09-28 14:24 ` Lee Jones
2026-09-29 16:32 ` Johannes Berg
2026-10-20 6:58 ` Lee Jones
2026-10-20 7:42 ` Johannes Berg
2026-10-20 7:47 ` Grumbach, Emmanuel
2026-10-20 9:54 ` Lee Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox