* [PATCH v2 wireless 3/3] wifi: p54: replace open-coded IE search with cfg80211_find_ie()
@ 2026-10-26 22:32 Tristan Madani
2026-10-28 19:03 ` Christian Lamparter
0 siblings, 1 reply; 2+ messages in thread
From: Tristan Madani @ 2026-10-26 22:32 UTC (permalink / raw)
To: Johannes Berg, Christian Lamparter, Stanislaw Gruszka
Cc: Ivo van Doorn, John W . Linville, Kees Cook, linux-wireless,
linux-kernel, stable, tristan
Replace the hand-rolled p54_find_ie() with cfg80211_find_ie() at both
call sites. The old function had a buggy short-frame guard
(sizeof(mgmt) compares against pointer size, not struct size) and
open-coded the IE iteration that cfg80211_find_ie() already provides
correctly.
Add a pre-subtraction length guard at both call sites to prevent an
unsigned underflow when computing the IE area length from a truncated
beacon frame.
In p54_beacon_format_ie_tim() the returned pointer is cast to non-const
u8 * because the caller modifies the buffer via memmove(); this matches
existing in-tree precedent (ath10k, wlcore, cw1200, nxpwifi).
Fixes: e5ea92a7528d ("p54: AP & Ad-hoc testing")
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
drivers/net/wireless/intersil/p54/lmac.h | 3 ---
drivers/net/wireless/intersil/p54/main.c | 30 ++++++------------------
drivers/net/wireless/intersil/p54/txrx.c | 12 +++++++---
3 files changed, 16 insertions(+), 29 deletions(-)
diff --git a/drivers/net/wireless/intersil/p54/lmac.h b/drivers/net/wireless/intersil/p54/lmac.h
index 8adde6ba35ab8..d9cbffb74a265 100644
--- a/drivers/net/wireless/intersil/p54/lmac.h
+++ b/drivers/net/wireless/intersil/p54/lmac.h
@@ -553,7 +553,4 @@ int p54_download_eeprom(struct p54_common *priv, void *buf,
u16 offset, u16 len);
struct p54_rssi_db_entry *p54_rssi_find(struct p54_common *p, const u16 freq);
-/* utility */
-u8 *p54_find_ie(struct sk_buff *skb, u8 ie);
-
#endif /* LMAC_H */
diff --git a/drivers/net/wireless/intersil/p54/main.c b/drivers/net/wireless/intersil/p54/main.c
index 57a62108cbc35..30f235d470bb4 100644
--- a/drivers/net/wireless/intersil/p54/main.c
+++ b/drivers/net/wireless/intersil/p54/main.c
@@ -71,28 +71,6 @@ static int p54_set_tim(struct ieee80211_hw *dev, struct ieee80211_sta *sta,
return p54_update_beacon_tim(priv, sta->aid, set);
}
-u8 *p54_find_ie(struct sk_buff *skb, u8 ie)
-{
- struct ieee80211_mgmt *mgmt = (void *)skb->data;
- u8 *pos, *end;
-
- if (skb->len <= sizeof(mgmt))
- return NULL;
-
- pos = (u8 *)mgmt->u.beacon.variable;
- end = skb->data + skb->len;
- while (pos < end) {
- if (pos + 2 + pos[1] > end)
- return NULL;
-
- if (pos[0] == ie)
- return pos;
-
- pos += 2 + pos[1];
- }
- return NULL;
-}
-
static int p54_beacon_format_ie_tim(struct sk_buff *skb)
{
/*
@@ -100,12 +78,18 @@ static int p54_beacon_format_ie_tim(struct sk_buff *skb)
* The dummy TIM MUST be at the end of the beacon frame,
* because it'll be overwritten!
*/
+ struct ieee80211_mgmt *mgmt = (void *)skb->data;
+ size_t ie_start = offsetof(struct ieee80211_mgmt, u.beacon.variable);
u8 *tim;
u8 dtim_len;
u8 dtim_period;
u8 *next;
- tim = p54_find_ie(skb, WLAN_EID_TIM);
+ if (skb->len < ie_start)
+ return 0;
+
+ tim = (u8 *)cfg80211_find_ie(WLAN_EID_TIM, mgmt->u.beacon.variable,
+ skb->len - ie_start);
if (!tim)
return 0;
diff --git a/drivers/net/wireless/intersil/p54/txrx.c b/drivers/net/wireless/intersil/p54/txrx.c
index 9f491334c8d04..7f952fcf16da7 100644
--- a/drivers/net/wireless/intersil/p54/txrx.c
+++ b/drivers/net/wireless/intersil/p54/txrx.c
@@ -293,9 +293,11 @@ static int p54_rssi_to_dbm(struct p54_common *priv, int rssi)
*/
static void p54_pspoll_workaround(struct p54_common *priv, struct sk_buff *skb)
{
- struct ieee80211_hdr *hdr = (void *) skb->data;
+ struct ieee80211_hdr *hdr = (void *)skb->data;
+ struct ieee80211_mgmt *mgmt = (void *)skb->data;
+ size_t ie_start = offsetof(struct ieee80211_mgmt, u.beacon.variable);
struct ieee80211_tim_ie *tim_ie;
- u8 *tim;
+ const u8 *tim;
u8 tim_len;
bool new_psm;
@@ -310,7 +312,11 @@ static void p54_pspoll_workaround(struct p54_common *priv, struct sk_buff *skb)
if (!ether_addr_equal_64bits(hdr->addr3, priv->bssid))
return;
- tim = p54_find_ie(skb, WLAN_EID_TIM);
+ if (skb->len < ie_start)
+ return;
+
+ tim = cfg80211_find_ie(WLAN_EID_TIM, mgmt->u.beacon.variable,
+ skb->len - ie_start);
if (!tim)
return;
--
2.47.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2 wireless 3/3] wifi: p54: replace open-coded IE search with cfg80211_find_ie()
2026-10-26 22:32 [PATCH v2 wireless 3/3] wifi: p54: replace open-coded IE search with cfg80211_find_ie() Tristan Madani
@ 2026-10-28 19:03 ` Christian Lamparter
0 siblings, 0 replies; 2+ messages in thread
From: Christian Lamparter @ 2026-10-28 19:03 UTC (permalink / raw)
To: Tristan Madani, Johannes Berg
Cc: Ivo van Doorn, Kees Cook, linux-wireless, linux-kernel, tristan
On 10/26/26 11:32 PM, Tristan Madani wrote:
> Replace the hand-rolled p54_find_ie() with cfg80211_find_ie() at both
> call sites. The old function had a buggy short-frame guard
> (sizeof(mgmt) compares against pointer size, not struct size) and
> open-coded the IE iteration that cfg80211_find_ie() already provides
> correctly.
>
> Add a pre-subtraction length guard at both call sites to prevent an
> unsigned underflow when computing the IE area length from a truncated
> beacon frame.
>
> In p54_beacon_format_ie_tim() the returned pointer is cast to non-const
> u8 * because the caller modifies the buffer via memmove(); this matches
> existing in-tree precedent (ath10k, wlcore, cw1200, nxpwifi).
Acked-by: Christian Lamparter <chunkeey@gmail.com>
>
> Fixes: e5ea92a7528d ("p54: AP & Ad-hoc testing")
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
> drivers/net/wireless/intersil/p54/lmac.h | 3 ---
> drivers/net/wireless/intersil/p54/main.c | 30 ++++++------------------
> drivers/net/wireless/intersil/p54/txrx.c | 12 +++++++---
> 3 files changed, 16 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/wireless/intersil/p54/lmac.h b/drivers/net/wireless/intersil/p54/lmac.h
> index 8adde6ba35ab8..d9cbffb74a265 100644
> --- a/drivers/net/wireless/intersil/p54/lmac.h
> +++ b/drivers/net/wireless/intersil/p54/lmac.h
> @@ -553,7 +553,4 @@ int p54_download_eeprom(struct p54_common *priv, void *buf,
> u16 offset, u16 len);
> struct p54_rssi_db_entry *p54_rssi_find(struct p54_common *p, const u16 freq);
>
> -/* utility */
> -u8 *p54_find_ie(struct sk_buff *skb, u8 ie);
> -
> #endif /* LMAC_H */
> diff --git a/drivers/net/wireless/intersil/p54/main.c b/drivers/net/wireless/intersil/p54/main.c
> index 57a62108cbc35..30f235d470bb4 100644
> --- a/drivers/net/wireless/intersil/p54/main.c
> +++ b/drivers/net/wireless/intersil/p54/main.c
> @@ -100,12 +78,18 @@ static int p54_beacon_format_ie_tim(struct sk_buff *skb)
> * The dummy TIM MUST be at the end of the beacon frame,
> * because it'll be overwritten!
> */
> + struct ieee80211_mgmt *mgmt = (void *)skb->data;
> + size_t ie_start = offsetof(struct ieee80211_mgmt, u.beacon.variable);
> u8 *tim;
> u8 dtim_len;
> u8 dtim_period;
> u8 *next;
>
> - tim = p54_find_ie(skb, WLAN_EID_TIM);
> + if (skb->len < ie_start)
> + return 0;
> +
> + tim = (u8 *)cfg80211_find_ie(WLAN_EID_TIM, mgmt->u.beacon.variable,
> + skb->len - ie_start);
casting (const u8*) to (u8 *). Oh well... If the checkers are fine with it.
Then sure.
> if (!tim)
> return 0;
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-28 19:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-26 22:32 [PATCH v2 wireless 3/3] wifi: p54: replace open-coded IE search with cfg80211_find_ie() Tristan Madani
2026-10-28 19:03 ` Christian Lamparter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox