[PATCH wireless 1/3] wifi: carl9170: fix IE parsing loop bounds check

[PATCH wireless 1/3] wifi: carl9170: fix IE parsing loop bounds check

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

carl9170_find_ie() iterates over information elements using a loop
condition of while (pos < end). When only one byte remains
(pos == end - 1), the loop body accesses pos[1] which reads one byte
beyond the validated buffer boundary. The read happens in the bounds
check expression pos + 2 + pos[1] > end before the result of that
check can prevent the access.

Fix by tightening the loop condition to while (pos + 2 <= end),
which ensures that at least the IE id (pos[0]) and length (pos[1])
bytes are within bounds before any access.

Fixes: a84fab3cbfdc4 ("carl9170: 802.11 rx/tx processing and usb backend")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 drivers/net/wireless/ath/carl9170/rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/carl9170/rx.c b/drivers/net/wireless/ath/carl9170/rx.c
index fbe46a5e4e415..de3ee0abd5be2 100644
--- a/drivers/net/wireless/ath/carl9170/rx.c
+++ b/drivers/net/wireless/ath/carl9170/rx.c
@@ -497,7 +497,7 @@ static u8 *carl9170_find_ie(u8 *data, unsigned int len, u8 ie)
 
 	pos = (u8 *)mgmt->u.beacon.variable;
 	end = data + len;
-	while (pos < end) {
+	while (pos + 2 <= end) {
 		if (pos + 2 + pos[1] > end)
 			return NULL;
 
-- 
2.47.3

[PATCH wireless 2/3] wifi: rt2x00: fix IE parsing loop bounds check

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

rt2x00lib_find_ie() iterates over information elements using a loop
condition of while (pos < end). When only one byte remains
(pos == end - 1), the loop body accesses pos[1] which reads one byte
beyond the validated buffer boundary. The read happens in the bounds
check expression pos + 2 + pos[1] > end before the result of that
check can prevent the access.

Fix by tightening the loop condition to while (pos + 2 <= end),
which ensures that at least the IE id (pos[0]) and length (pos[1])
bytes are within bounds before any access.

Fixes: 1c0bcf89d85cc ("rt2x00: Add autowake support for USB hardware")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2x00dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
index 4d94b7062f440..5623ab11c6142 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
@@ -564,7 +564,7 @@ static u8 *rt2x00lib_find_ie(u8 *data, unsigned int len, u8 ie)
 
 	pos = (u8 *)mgmt->u.beacon.variable;
 	end = data + len;
-	while (pos < end) {
+	while (pos + 2 <= end) {
 		if (pos + 2 + pos[1] > end)
 			return NULL;
 
-- 
2.47.3

[PATCH wireless 3/3] wifi: p54: fix IE parsing loop bounds check

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

p54_find_ie() iterates over information elements using a loop condition
of while (pos < end). When only one byte remains (pos == end - 1),
the loop body accesses pos[1] which reads one byte beyond the validated
buffer boundary. The read happens in the bounds check expression
pos + 2 + pos[1] > end before the result of that check can prevent
the access.

Fix by tightening the loop condition to while (pos + 2 <= end),
which ensures that at least the IE id (pos[0]) and length (pos[1])
bytes are within bounds before any access.

Fixes: 0ac0d6cedf611 ("p54: Move mac80211 glue code")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 drivers/net/wireless/intersil/p54/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intersil/p54/main.c b/drivers/net/wireless/intersil/p54/main.c
index 57a62108cbc35..16614f65dfa5f 100644
--- a/drivers/net/wireless/intersil/p54/main.c
+++ b/drivers/net/wireless/intersil/p54/main.c
@@ -81,7 +81,7 @@ u8 *p54_find_ie(struct sk_buff *skb, u8 ie)
 
 	pos = (u8 *)mgmt->u.beacon.variable;
 	end = skb->data + skb->len;
-	while (pos < end) {
+	while (pos + 2 <= end) {
 		if (pos + 2 + pos[1] > end)
 			return NULL;
 
-- 
2.47.3

Re: [PATCH wireless 3/3] wifi: p54: fix IE parsing loop bounds check

[Johannes Berg]

On Mon, 2026-10-26 at 11:21 +0000, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
> 
> p54_find_ie() iterates over information elements using a loop condition
> of while (pos < end). When only one byte remains (pos == end - 1),
> the loop body accesses pos[1] which reads one byte beyond the validated
> buffer boundary. The read happens in the bounds check expression
> pos + 2 + pos[1] > end before the result of that check can prevent
> the access.
> 
> Fix by tightening the loop condition to while (pos + 2 <= end),
> which ensures that at least the IE id (pos[0]) and length (pos[1])
> bytes are within bounds before any access.
> 
> Fixes: 0ac0d6cedf611 ("p54: Move mac80211 glue code")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
>  drivers/net/wireless/intersil/p54/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/intersil/p54/main.c b/drivers/net/wireless/intersil/p54/main.c
> index 57a62108cbc35..16614f65dfa5f 100644
> --- a/drivers/net/wireless/intersil/p54/main.c
> +++ b/drivers/net/wireless/intersil/p54/main.c
> @@ -81,7 +81,7 @@ u8 *p54_find_ie(struct sk_buff *skb, u8 ie)

We'd probably be better off just replacing that whole function with one
of the real search functions or iteration macros?

johannes

Re: [PATCH wireless 3/3] wifi: p54: fix IE parsing loop bounds check

[Christian Lamparter]

On 10/26/26 12:45 PM, Johannes Berg wrote:
> On Mon, 2026-10-26 at 11:21 +0000, Tristan Madani wrote:
>> From: Tristan Madani <tristan@talencesecurity.com>
>>
>> p54_find_ie() iterates over information elements using a loop condition
>> of while (pos < end). When only one byte remains (pos == end - 1),
>> the loop body accesses pos[1] which reads one byte beyond the validated
>> buffer boundary. The read happens in the bounds check expression
>> pos + 2 + pos[1] > end before the result of that check can prevent
>> the access.
>>
>> Fix by tightening the loop condition to while (pos + 2 <= end),
>> which ensures that at least the IE id (pos[0]) and length (pos[1])
>> bytes are within bounds before any access.
>>
>> Fixes: 0ac0d6cedf611 ("p54: Move mac80211 glue code")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
>> ---
>>   drivers/net/wireless/intersil/p54/main.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/intersil/p54/main.c b/drivers/net/wireless/intersil/p54/main.c
>> index 57a62108cbc35..16614f65dfa5f 100644
>> --- a/drivers/net/wireless/intersil/p54/main.c
>> +++ b/drivers/net/wireless/intersil/p54/main.c
>> @@ -81,7 +81,7 @@ u8 *p54_find_ie(struct sk_buff *skb, u8 ie)
> 
> We'd probably be better off just replacing that whole function with one
> of the real search functions or iteration macros?
> yeah, I think carl9170 probably can just straight up use cfg80211_find_ie.

But p54 needs to edit the beacon and the memmove the (found) TIM element
to the end of the beacon and modify it a bit of tim in p54_beacon_format_ie_tim.
The problem here is that the helpers are all casting to const. So this will
look "funny" via to "uncast" const. Unless you meant something else?

As for what to expect if something read past the end? Well, p54 works on skbs. So past the
"end" in p54_beacon_format_ie_tim() would either be extra tailroom then padding or the start
of skb_shared_info (u8 flags) I think? In case of p54_pspoll_workaround(), after the "end",
there will be the FCS (32-bit checksum) - at least on real hardware) and then padding or
the start of skb_shared_info.

For rt2xx: not sure. But the existing use cfg80211_find_ie in rtw89_core_cancel_6ghz_probe_tx()
and rtw89_core_cancel_6ghz_probe_tx() looks suspecious.
Because from what I gleamed, both just pass "skb->len" as then len parameter. But shouldn't
that be subtracted by the offsetof mgmt->u.beacon.variable? Because cfg80211_find_ie thinks
len starts from the "ies"? I can't tell right now as I'm sick and need to go back to bed.
This all could either be a fever dream an be true/half-true or completely wrong. So I have to
ask in advance: Please don't take this all too seriously.

Cheers,
Christian

PS: Is there also a patch for rtl_find_ie (isn't that pretty much the same as all the others?)