On 20 April 2017 at 20:22, Arend van Spriel
<arend.vanspr...@broadcom.com> wrote:
> On 4/20/2017 5:47 PM, James Hughes wrote:
>>
>> Driver was writing to skb header area without ensuring it was
>> writable i.e. uncloned.
>>
>> Used skb_cow_header to ensure that header buffer is large enough
>> and is uncloned.
>>
>> Detected when, in combination with the smsc85xx driver, both drivers
>> attempted to write different headers to the same header buffer.
>
>
> I guess this patch is limited to one file to address the comment from Eric
> Dumazet. I prefer to talk through the initial patch you sent. Just a
> procedural remark or two here. 1) Please drop 'brcm80211:' from the Subject
> as the 'brcmfmac' prefix is sufficient, and 2) despite the change of the
> Subject you should consider this version 2 of the initial patch so the
> Subject should read '[PATCH V2] brcmfmac: ....'.
>
>> Signed-off-by: James Hughes <james.hug...@raspberrypi.org>
>> ---
>
> Related to remark 2) above you should provide a changelog here listing what
> is changed compared to the initial patch.
>
> Regards,
> Arend
>
> ---
>>
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c | 15
>> +++++++++++++--
>> 1 file changed, 13 insertions(+), 2 deletions(-)
>
>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
>> index 038a960..b9d7d08 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
>> @@ -249,14 +249,19 @@ brcmf_proto_bcdc_set_dcmd(struct brcmf_pub *drvr,
>> int ifidx, uint cmd,
>> return ret;
>> }
>> -static void
>> +static int
>> brcmf_proto_bcdc_hdrpush(struct brcmf_pub *drvr, int ifidx, u8 offset,
>> struct sk_buff *pktbuf)
>> {
>> struct brcmf_proto_bcdc_header *h;
>> + int err;
>> brcmf_dbg(BCDC, "Enter\n");
>> + err = skb_cow_head(pktbuf, BCDC_HEADER_LEN);
>> + if (err)
>> + return err;
>> +
>> /* Push BDC header used to convey priority for buses that don't */
>> skb_push(pktbuf, BCDC_HEADER_LEN);
>> @@ -271,6 +276,8 @@ brcmf_proto_bcdc_hdrpush(struct brcmf_pub *drvr, int
>> ifidx, u8 offset,
>> h->data_offset = offset;
>> BCDC_SET_IF_IDX(h, ifidx);
>> trace_brcmf_bcdchdr(pktbuf->data);
>> +
>> + return 0;
>> }
>> static int
>> @@ -330,7 +337,11 @@ static int
>> brcmf_proto_bcdc_txdata(struct brcmf_pub *drvr, int ifidx, u8 offset,
>> struct sk_buff *pktbuf)
>> {
>> - brcmf_proto_bcdc_hdrpush(drvr, ifidx, offset, pktbuf);
>> + int err = brcmf_proto_bcdc_hdrpush(drvr, ifidx, offset, pktbuf);
>> +
>> + if (err)
>> + return err;
>> +
>> return brcmf_bus_txdata(drvr->bus_if, pktbuf);
>> }
>>
This patch has been superseded, please ignore.
