2017-04-25 09:15:13

by James Hughes

[permalink] [raw]
Subject: [PATCH v3] brcmfmac: Make skb header writable before use

The driver was making changes to the skb_header without
ensuring it was writable (i.e. uncloned).
This patch also removes some boiler plate header size
checking/adjustment code as that is also handled by the
skb_cow_header function used to make header writable.

Please apply to 4.12, important fix.

This patch depends on
brcmfmac: Ensure pointer correctly set if skb data location changes

Signed-off-by: James Hughes <[email protected]>
Acked-by: Arend van Spriel <[email protected]>
---

Changes in v2
Makes the _cow_ call at the entry point of the skb in to the
stack, means only needs to be done once, and error handling
is easier.
Split a separate minor bug fix off to a separate patch (which
this patch depends on)

Changes in v3
Minor change to the 'if' logic to reduce patch size as per
maintainers request.
Flagged as important fix for 4.12 in commit message

.../net/wireless/broadcom/brcm80211/brcmfmac/core.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 9b7c19a508ac..433f2c8408e9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -211,22 +211,13 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
goto done;
}

- /* Make sure there's enough room for any header */
- if (skb_headroom(skb) < drvr->hdrlen) {
- struct sk_buff *skb2;
-
- brcmf_dbg(INFO, "%s: insufficient headroom\n",
- brcmf_ifname(ifp));
- drvr->bus_if->tx_realloc++;
- skb2 = skb_realloc_headroom(skb, drvr->hdrlen);
+ /* Make sure there's enough writable headroom*/
+ ret = skb_cow_head(skb, drvr->hdrlen);
+ if (ret < 0) {
+ brcmf_err("%s: skb_cow_head failed\n",
+ brcmf_ifname(ifp));
dev_kfree_skb(skb);
- skb = skb2;
- if (skb == NULL) {
- brcmf_err("%s: skb_realloc_headroom failed\n",
- brcmf_ifname(ifp));
- ret = -ENOMEM;
- goto done;
- }
+ goto done;
}

/* validate length for ether packet */
--
2.11.0


2017-04-26 09:05:33

by Kalle Valo

[permalink] [raw]
Subject: Re: [v3] brcmfmac: Make skb header writable before use

James Hughes <[email protected]> wrote:
> The driver was making changes to the skb_header without
> ensuring it was writable (i.e. uncloned).
> This patch also removes some boiler plate header size
> checking/adjustment code as that is also handled by the
> skb_cow_header function used to make header writable.
>
> Signed-off-by: James Hughes <[email protected]>
> Acked-by: Arend van Spriel <[email protected]>

Patch applied to wireless-drivers-next.git, thanks.

9cc4b7cb86cb brcmfmac: Make skb header writable before use

--
https://patchwork.kernel.org/patch/9697763/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches