Re: [PATCH v3] brcmfmac: fix incorrect event channel deduction

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I think calling this a performance regression is a bit understated; my
download speed jumped from 1Mbit/s back up to 40MBit/s after applying
the patch due to the sheer amount of packets being incorrectly
processed.

In addition, processing arbitrary data frames as firmware events might
be a security vulnerability.

On Wed, Jan 18, 2017 at 4:38 AM, Arend Van Spriel
<arend.vanspriel@xxxxxxxxxxxx> wrote:
> On 18-1-2017 0:24, gavinli@xxxxxxxxxxxxxx wrote:
>> From: Gavin Li <git@xxxxxxxxxxxxxx>
>>
>> brcmf_sdio_fromevntchan() was being called on the the data frame
>> rather than the software header, causing some frames to be
>> mischaracterized as on the event channel rather than the data channel.
>>
>> This fixes a major performance regression (due to dropped packets).
>>
>> Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet")
>
> Actually would prefer Franky to chime in as well as he made the change
> and confirm correctness. It was introduced a couple of kernel versions
> back and only a performance regression so seems ok to let this go in
> 4.11 for now and backport as needed for stable later.
>
> Acked-by: Arend van Spriel <arend.vanspriel@xxxxxxxxxxxx>
>> Signed-off-by: Gavin Li <git@xxxxxxxxxxxxxx>
>> Cc: <stable@xxxxxxxxxxxxxxx> [4.7+]
>> ---
>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> index dfb0658713d9..d2219885071f 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> @@ -1661,7 +1661,7 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
>>                                          pfirst->len, pfirst->next,
>>                                          pfirst->prev);
>>                       skb_unlink(pfirst, &bus->glom);
>> -                     if (brcmf_sdio_fromevntchan(pfirst->data))
>> +                     if (brcmf_sdio_fromevntchan(&dptr[SDPCM_HWHDR_LEN]))
>>                               brcmf_rx_event(bus->sdiodev->dev, pfirst);
>>                       else
>>                               brcmf_rx_frame(bus->sdiodev->dev, pfirst,
>>
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]