2024-03-28 00:58:01

by Richard Kinder

Subject: [PATCH] wifi: mac80211: ensure beacon is non-S1G prior to extracting the beacon timestamp field

Logic inside ieee80211_rx_mgmt_beacon accesses the
mgmt->u.beacon.timestamp field without first checking whether the beacon
received is non-S1G format.

Fix the problem by checking the beacon is non-S1G format to avoid access
of the mgmt->u.beacon.timestamp field.

Signed-off-by: Richard Kinder <[email protected]>
---
net/mac80211/mlme.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 47a2cba8313f..6c89e2e4886c 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -6185,7 +6185,8 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_link_data *link,
link->u.mgd.dtim_period = elems->dtim_period;
link->u.mgd.have_beacon = true;
ifmgd->assoc_data->need_beacon = false;
- if (ieee80211_hw_check(&local->hw, TIMING_BEACON_ONLY)) {
+ if (ieee80211_hw_check(&local->hw, TIMING_BEACON_ONLY) &&
+ !ieee80211_is_s1g_beacon(hdr->frame_control)) {
link->conf->sync_tsf =
le64_to_cpu(mgmt->u.beacon.timestamp);
link->conf->sync_device_ts =

base-commit: 4076fa161217fcd64a578ca04586c4be728cb004
--
2.39.3 (Apple Git-146)

2024-03-28 18:29:17

by Johannes Berg

Subject: Re: [PATCH] wifi: mac80211: ensure beacon is non-S1G prior to extracting the beacon timestamp field

On Thu, 2024-03-28 at 11:57 +1100, Richard Kinder wrote:
> Logic inside ieee80211_rx_mgmt_beacon accesses the
> mgmt->u.beacon.timestamp field without first checking whether the beacon
> received is non-S1G format.
>
> Fix the problem by checking the beacon is non-S1G format to avoid access
> of the mgmt->u.beacon.timestamp field.

Huh, how did that end up being a problem, since iwlmvm with older
devices is the only driver using that flag, and it doesn't support S1G?

It's still correct, but it shouldn't be a problem now?

johannes

2024-03-29 03:25:02

by Richard Kinder

Subject: Re: [PATCH] wifi: mac80211: ensure beacon is non-S1G prior to extracting the beacon timestamp field

> On 29 Mar 2024, at 5:28 AM, Johannes Berg <[email protected]> wrote:
>
> On Thu, 2024-03-28 at 11:57 +1100, Richard Kinder wrote:
>> Logic inside ieee80211_rx_mgmt_beacon accesses the
>> mgmt->u.beacon.timestamp field without first checking whether the beacon
>> received is non-S1G format.
>>
>> Fix the problem by checking the beacon is non-S1G format to avoid access
>> of the mgmt->u.beacon.timestamp field.
>
> Huh, how did that end up being a problem, since iwlmvm with older
> devices is the only driver using that flag, and it doesn't support S1G?
>
> It's still correct, but it shouldn't be a problem now?
>
> johannes

Hi Johannes,

Thanks for the quick reply, much appreciated.

The motivation behind the patch was that a similar pattern is shown in lines
6315-6316: the same flag is checked along with !ieee80211_is_s1g_beacon.

If it is guaranteed that an interface running at non-S1G frequencies cannot
receive an S1G formatted frame at this point in the receive path, then the check
for is_s1g_beacon can be removed. However, could a malicious actor form an
S1G formatted frame with appropriate MAC addresses and trigger this path on
the older iwlmvm devices?

Regards,
Richard