| Message ID | 20210120114137.200019-1-mailhol.vincent@wanadoo.fr (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix several use after free bugs | expand |
On 1/20/21 12:41 PM, Vincent Mailhol wrote: > This series fix three bugs which all have the same root cause. > > When calling netif_rx(skb) and its variants, the skb will eventually > get consumed (or freed) and thus it is unsafe to dereference it after > the call returns. > > This remark especially applies to any variable with aliases the skb > memory which is the case of the can(fd)_frame. > > The pattern is as this: > skb = alloc_can_skb(dev, &cf); > /* Do stuff */ > netif_rx(skb); > stats->rx_bytes += cf->len; > > Increasing the stats should be done *before* the call to netif_rx() > while the skb is still safe to use. > > Changes since v3: > - Patch 1/3: move the comments for upstream after the --- scissors > > Changes since v2: > - rebase on net/master > - Patch 1/3: Added a comment towards upstream to inform about a > conflict which will occur when net-next and net are merged > Ref: https://lore.kernel.org/linux-can/20210120085356.m7nabbw5zhy7prpo@hardanger.blackshift.org/ > > Changes since v1: > - fix a silly typo in patch 2/3 (variable len was declared twice...) > > Vincent Mailhol (3): > can: dev: can_restart: fix use after free bug > can: vxcan: vxcan_xmit: fix use after free bug > can: peak_usb: fix use after free bugs > > drivers/net/can/dev.c | 4 ++-- > drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 8 ++++---- > drivers/net/can/vxcan.c | 6 ++++-- > 3 files changed, 10 insertions(+), 8 deletions(-) Applied to linux-can-testing. I don't know why 2/3 hasn't made it to the mail archive yet. Marc
Hello: This series was applied to netdev/net.git (refs/heads/master): On Wed, 20 Jan 2021 20:41:34 +0900 you wrote: > This series fix three bugs which all have the same root cause. > > When calling netif_rx(skb) and its variants, the skb will eventually > get consumed (or freed) and thus it is unsafe to dereference it after > the call returns. > > This remark especially applies to any variable with aliases the skb > memory which is the case of the can(fd)_frame. > > [...] Here is the summary with links: - [v4,1/3] can: dev: can_restart: fix use after free bug https://git.kernel.org/netdev/net/c/03f16c5075b2 - [v4,2/3] can: vxcan: vxcan_xmit: fix use after free bug https://git.kernel.org/netdev/net/c/75854cad5d80 - [v4,3/3] can: peak_usb: fix use after free bugs https://git.kernel.org/netdev/net/c/50aca891d7a5 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
This series fix three bugs which all have the same root cause. When calling netif_rx(skb) and its variants, the skb will eventually get consumed (or freed) and thus it is unsafe to dereference it after the call returns. This remark especially applies to any variable with aliases the skb memory which is the case of the can(fd)_frame. The pattern is as this: skb = alloc_can_skb(dev, &cf); /* Do stuff */ netif_rx(skb); stats->rx_bytes += cf->len; Increasing the stats should be done *before* the call to netif_rx() while the skb is still safe to use. Changes since v3: - Patch 1/3: move the comments for upstream after the --- scissors Changes since v2: - rebase on net/master - Patch 1/3: Added a comment towards upstream to inform about a conflict which will occur when net-next and net are merged Ref: https://lore.kernel.org/linux-can/20210120085356.m7nabbw5zhy7prpo@hardanger.blackshift.org/ Changes since v1: - fix a silly typo in patch 2/3 (variable len was declared twice...) Vincent Mailhol (3): can: dev: can_restart: fix use after free bug can: vxcan: vxcan_xmit: fix use after free bug can: peak_usb: fix use after free bugs drivers/net/can/dev.c | 4 ++-- drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 8 ++++---- drivers/net/can/vxcan.c | 6 ++++-- 3 files changed, 10 insertions(+), 8 deletions(-) base-commit: 9c30ae8398b0813e237bde387d67a7f74ab2db2d