Re: [PATCH v2 wireless 3/3] wifi: p54: replace open-coded IE search with cfg80211_find_ie()

[Christian Lamparter]

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@xxxxxxxxx>

> Fixes: e5ea92a7528d ("p54: AP & Ad-hoc testing")
> Signed-off-by: Tristan Madani <tristan@xxxxxxxxxxxxxxxxxxx>
> ---
>   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;
>