Re: [PATCH net] qmi_wwan: Clone the skb when in pass-through mode
flat view
From: Bjørn Mork <bjorn@mork.no>
Date: 2021-06-14 14:46:08
Kristian Evensen [off-list ref] writes:
quoted hunk ↗ jump to hunk
The skb that we pass to the rmnet driver is owned by usbnet and is freed soon after the rx_fixup() callback is called (in usbnet_bh()). There is no guarantee that rmnet is done handling the skb before it is freed. We should clone the skb before we call netif_rx() to prevent use-after-free and misc. kernel oops. Fixes: 59e139cf0b32 ("net: qmi_wwan: Add pass through mode") Signed-off-by: Kristian Evensen <redacted> --- drivers/net/usb/qmi_wwan.c | 4 ++++ 1 file changed, 4 insertions(+)diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c index db8d3a4f2678..5ac307eb0bfd 100644 --- a/drivers/net/usb/qmi_wwan.c +++ b/drivers/net/usb/qmi_wwan.c@@ -620,6 +620,10 @@ static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb) return qmimux_rx_fixup(dev, skb); if (info->flags & QMI_WWAN_FLAG_PASS_THROUGH) { + skb = skb_clone(skb, GFP_ATOMIC); + if (!skb) + return 0; + skb->protocol = htons(ETH_P_MAP); return (netif_rx(skb) == NET_RX_SUCCESS); }
Thanks for pointing this out. But it still looks strange to me. Why do we call netif_rx(skb) here instead of just returning 1 and leave that for usbnet_skb_return()? With cloning we end up doing eth_type_trans() on the duplicate - is that wise? Bjørn