On 12-09-17 17:04, Kevin Cernekee wrote:
On Mon, Sep 11, 2017 at 12:09 PM, Arend van Spriel
<arend.vanspriel@xxxxxxxxxxxx> 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
Checking for exact match might not work, because the skbuff length could
differ because of host interface alignment requirements.
What did Franky's version of the check look like?
the check Franky had was:
datalen > packet_len - sizeof(*event_packet)
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.
Agree. I quickly browsed through our collection of tests in our test
framework, but found none covering this.
Regards,
Arend
