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

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

[Richard Kinder]

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 <richard.kinder@gmail.com>
---
 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)

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

[Johannes Berg]

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

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

[Richard Kinder]

> On 29 Mar 2024, at 5:28 AM, Johannes Berg <johannes@sipsolutions.net> 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