On Mon, Sep 11, 2017 at 12:09 PM, Arend van Spriel <arend.vanspr...@broadcom.com> wrote: >>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c >>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c >>> index 5aabdc9ed7e0..4cad1f0d2a82 100644 >>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c >>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c >>> @@ -429,7 +429,8 @@ void brcmf_fweh_process_event(struct brcmf_pub *drvr, >>> if (code != BRCMF_E_IF && !fweh->evt_handler[code]) >>> return; >>> >>> - if (datalen > BRCMF_DCMD_MAXLEN) >>> + if (datalen > BRCMF_DCMD_MAXLEN || >>> + datalen + sizeof(*event_packet) < packet_len) >> >> >> Shouldn't this check be larger-than, i.e. we need the packet to be at >> least sizeof(*event_packet) + its payload size? > > That depends on how you formulate the requirement. packet_len here is the > length for the received skbuff. The event message (= sizeof(*event_packet)) > and its variable payload (= datalen) shall not exceed length of received > skbuff (= packet_len).
Or should it be an exact match, i.e. datalen + sizeof(*event_packet) != packet_len What did Franky's version of the check look like? If Broadcom has a test suite that tries different event types and notices if events are getting unexpectedly dropped, that would be helpful in validating the change. I would be wary of pushing this to -stable until we know the check is 100% correct.
