Re: [PATCH] wifi: p54: fix incorrect frame length check in p54_find_ie()
From: Christian Lamparter
Date: Sun Sep 06 2026 - 10:32:37 EST
On 9/6/26 1:49 PM, Johannes Berg wrote:
On Sun, 2026-09-06 at 13:06 +0200, Christian Lamparter wrote:
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;
The check in the while loop and the checks within the while loop make sure that
no "pos" is returned unless the IE is still within skb->len.
But true, it should have been *mgmt and not mgmt.
FWIW, I dropped it because it really shouldn't have been there this way
since 'mgmt' can be far bigger than needed since it contains the union
for all kinds of action frames etc.
I'm not even sure it's needed regardless of the next check since the
beacon is built by mac80211.
Just blindly patching one mistake for another doesn't help anyone.
Ok, alright? I just looked in both:
https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/
https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless.git/
Was there a patch already? Or did I missread the first sentence that hinted this was "dropped"?
Or do you want that Mr/Ms Yan (Sorry, but from what I was told, the name can be used for
both male and female) just post a new patch that removes the superfluous check.
About *mgmt vs mgmt: Yes, you are right. That said, that check came from sometime between
~2006-2008 ;). I don't think the struct back then already contained action frames with
sounding/beamforming/timing feedback. Still, I'm totally fine with it being "dropped" too.
If there was such a patch posted, please feel free to add a
"Acked-by: Christian Lamparter <chunkeey@xxxxxxxxx>" if you merge it.
Cheers,
Christian