Re: brcmfmac: heap overflow in brcmf_notify_auth_frame_rx() on a short auth frame
Arend van Spriel <[email protected]> Sat, 27 Jun 2026 14:09:15 +0200
| Newsgroups | dev.linux.lists.brcm80211,org.kernel.vger.linux-kernel,org.kernel.vger.linux-wireless |
|---|---|
| Message-ID | <[email protected]> |
On 22/06/2026 18:02, Maoyi Xie wrote:
> Hi all,
>
> I think brcmf_notify_auth_frame_rx() in
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c can overflow the
> heap when the firmware reports a short external auth frame. I would
> appreciate it if you could take a look.
>
> The handler takes the frame length from the event, then allocates a buffer
> for it.
>
> u32 mgmt_frame_len = e->datalen - sizeof(struct brcmf_rx_mgmt_data);
> ...
> if (e->datalen < sizeof(*rxframe)) {
> ...
> return -EINVAL;
> }
> ...
> mgmt_frame = kzalloc(mgmt_frame_len, GFP_KERNEL);
>
> The only length check is e->datalen >= sizeof(*rxframe). So mgmt_frame_len
> can be anything from 0 up. The frame body is then copied with a length that
> subtracts the management header offset.
>
> memcpy(&mgmt_frame->u, frame,
> mgmt_frame_len - offsetof(struct ieee80211_mgmt, u));
>
> offsetof(struct ieee80211_mgmt, u) is 24. If mgmt_frame_len is less than 24,
> the subtraction wraps around as an unsigned value to a huge number. The
> memcpy then runs far past the small kzalloc buffer. That is a heap overflow
> driven by the frame the firmware passes up. A malicious or malfunctioning AP
> can make the frame short during the external SAE auth exchange.
>
> The p2p path in the same driver allocates with the header offset included,
> so it does not have this shape.
>
> I reproduced the overflow on 7.1-rc7. With mgmt_frame_len set below the 24
> byte header offset, the subtracted length wraps to a huge value and the copy
> faults.
>
> BUG: unable to handle page fault ... in memcpy_orig
>
> A check that mgmt_frame_len is at least offsetof(struct ieee80211_mgmt, u)
> before the copy would close it.
>
> Does this look like a real bug to you, and is that the right place to bound
> it? If so I am happy to send a proper patch with a Fixes tag and Cc stable.
>
> Kaixuan Li and I found this together.
Thanks for reaching out although it would have been fine to just send
the patch straight away. I do agree with the assessment given so this is
a real bug.
Regards,
Arend