* [net 1/3] can: dev: can_restart: fix use after free bug
2021-01-20 12:51 pull-request: can 2021-01-20 Marc Kleine-Budde
@ 2021-01-20 12:52 ` Marc Kleine-Budde
2021-01-20 20:40 ` patchwork-bot+netdevbpf
2021-01-20 12:52 ` [net 2/3] can: vxcan: vxcan_xmit: " Marc Kleine-Budde
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Marc Kleine-Budde @ 2021-01-20 12:52 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, linux-can, kernel, Vincent Mailhol, Marc Kleine-Budde
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
After calling netif_rx_ni(skb), dereferencing skb is unsafe.
Especially, the can_frame cf which aliases skb memory is accessed
after the netif_rx_ni() in:
stats->rx_bytes += cf->len;
Reordering the lines solves the issue.
Fixes: 39549eef3587 ("can: CAN Network device driver and Netlink interface")
Link: https://lore.kernel.org/r/20210120114137.200019-2-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/dev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 3486704c8a95..8b1ae023cb21 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -592,11 +592,11 @@ static void can_restart(struct net_device *dev)
cf->can_id |= CAN_ERR_RESTARTED;
- netif_rx_ni(skb);
-
stats->rx_packets++;
stats->rx_bytes += cf->len;
+ netif_rx_ni(skb);
+
restart:
netdev_dbg(dev, "restarted\n");
priv->can_stats.restarts++;
base-commit: 9c30ae8398b0813e237bde387d67a7f74ab2db2d
--
2.29.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [net 1/3] can: dev: can_restart: fix use after free bug
2021-01-20 12:52 ` [net 1/3] can: dev: can_restart: fix use after free bug Marc Kleine-Budde
@ 2021-01-20 20:40 ` patchwork-bot+netdevbpf
0 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-01-20 20:40 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: netdev, davem, kuba, linux-can, kernel, mailhol.vincent
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Wed, 20 Jan 2021 13:52:00 +0100 you wrote:
> From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
>
> After calling netif_rx_ni(skb), dereferencing skb is unsafe.
> Especially, the can_frame cf which aliases skb memory is accessed
> after the netif_rx_ni() in:
> stats->rx_bytes += cf->len;
>
> [...]
Here is the summary with links:
- [net,1/3] can: dev: can_restart: fix use after free bug
https://git.kernel.org/netdev/net-next/c/03f16c5075b2
- [net,2/3] can: vxcan: vxcan_xmit: fix use after free bug
https://git.kernel.org/netdev/net-next/c/75854cad5d80
- [net,3/3] can: peak_usb: fix use after free bugs
https://git.kernel.org/netdev/net-next/c/50aca891d7a5
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* [net 2/3] can: vxcan: vxcan_xmit: fix use after free bug
2021-01-20 12:51 pull-request: can 2021-01-20 Marc Kleine-Budde
2021-01-20 12:52 ` [net 1/3] can: dev: can_restart: fix use after free bug Marc Kleine-Budde
@ 2021-01-20 12:52 ` Marc Kleine-Budde
2021-01-20 12:52 ` [net 3/3] can: peak_usb: fix use after free bugs Marc Kleine-Budde
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2021-01-20 12:52 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, linux-can, kernel, Vincent Mailhol, Marc Kleine-Budde
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
After calling netif_rx_ni(skb), dereferencing skb is unsafe.
Especially, the canfd_frame cfd which aliases skb memory is accessed
after the netif_rx_ni().
Fixes: a8f820a380a2 ("can: add Virtual CAN Tunnel driver (vxcan)")
Link: https://lore.kernel.org/r/20210120114137.200019-3-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/vxcan.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/vxcan.c b/drivers/net/can/vxcan.c
index fa47bab510bb..f9a524c5f6d6 100644
--- a/drivers/net/can/vxcan.c
+++ b/drivers/net/can/vxcan.c
@@ -39,6 +39,7 @@ static netdev_tx_t vxcan_xmit(struct sk_buff *skb, struct net_device *dev)
struct net_device *peer;
struct canfd_frame *cfd = (struct canfd_frame *)skb->data;
struct net_device_stats *peerstats, *srcstats = &dev->stats;
+ u8 len;
if (can_dropped_invalid_skb(dev, skb))
return NETDEV_TX_OK;
@@ -61,12 +62,13 @@ static netdev_tx_t vxcan_xmit(struct sk_buff *skb, struct net_device *dev)
skb->dev = peer;
skb->ip_summed = CHECKSUM_UNNECESSARY;
+ len = cfd->len;
if (netif_rx_ni(skb) == NET_RX_SUCCESS) {
srcstats->tx_packets++;
- srcstats->tx_bytes += cfd->len;
+ srcstats->tx_bytes += len;
peerstats = &peer->stats;
peerstats->rx_packets++;
- peerstats->rx_bytes += cfd->len;
+ peerstats->rx_bytes += len;
}
out_unlock:
--
2.29.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [net 3/3] can: peak_usb: fix use after free bugs
2021-01-20 12:51 pull-request: can 2021-01-20 Marc Kleine-Budde
2021-01-20 12:52 ` [net 1/3] can: dev: can_restart: fix use after free bug Marc Kleine-Budde
2021-01-20 12:52 ` [net 2/3] can: vxcan: vxcan_xmit: " Marc Kleine-Budde
@ 2021-01-20 12:52 ` Marc Kleine-Budde
2021-01-20 17:19 ` pull-request: can 2021-01-20 Jakub Kicinski
2021-01-20 20:40 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2021-01-20 12:52 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, linux-can, kernel, Vincent Mailhol, Marc Kleine-Budde
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
After calling peak_usb_netif_rx_ni(skb), dereferencing skb is unsafe.
Especially, the can_frame cf which aliases skb memory is accessed
after the peak_usb_netif_rx_ni().
Reordering the lines solves the issue.
Fixes: 0a25e1f4f185 ("can: peak_usb: add support for PEAK new CANFD USB adapters")
Link: https://lore.kernel.org/r/20210120114137.200019-4-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index 61631f4fd92a..f347ecc79aef 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -514,11 +514,11 @@ static int pcan_usb_fd_decode_canmsg(struct pcan_usb_fd_if *usb_if,
else
memcpy(cfd->data, rm->d, cfd->len);
- peak_usb_netif_rx(skb, &usb_if->time_ref, le32_to_cpu(rm->ts_low));
-
netdev->stats.rx_packets++;
netdev->stats.rx_bytes += cfd->len;
+ peak_usb_netif_rx(skb, &usb_if->time_ref, le32_to_cpu(rm->ts_low));
+
return 0;
}
@@ -580,11 +580,11 @@ static int pcan_usb_fd_decode_status(struct pcan_usb_fd_if *usb_if,
if (!skb)
return -ENOMEM;
- peak_usb_netif_rx(skb, &usb_if->time_ref, le32_to_cpu(sm->ts_low));
-
netdev->stats.rx_packets++;
netdev->stats.rx_bytes += cf->len;
+ peak_usb_netif_rx(skb, &usb_if->time_ref, le32_to_cpu(sm->ts_low));
+
return 0;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: pull-request: can 2021-01-20
2021-01-20 12:51 pull-request: can 2021-01-20 Marc Kleine-Budde
` (2 preceding siblings ...)
2021-01-20 12:52 ` [net 3/3] can: peak_usb: fix use after free bugs Marc Kleine-Budde
@ 2021-01-20 17:19 ` Jakub Kicinski
2021-01-20 20:20 ` Marc Kleine-Budde
2021-01-20 20:40 ` patchwork-bot+netdevbpf
4 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2021-01-20 17:19 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: netdev, davem, linux-can, kernel
On Wed, 20 Jan 2021 13:51:59 +0100 Marc Kleine-Budde wrote:
> Hello Jakub, hello David,
>
> this is a pull request of 3 patches for net/master.
>
> All three patches are by Vincent Mailhol and fix a potential use after free bug
> in the CAN device infrastructure, the vxcan driver, and the peak_usk driver. In
> the TX-path the skb is used to read from after it was passed to the networking
> stack with netif_rx_ni().
Pulled, thanks.
Seems like the PR didn't show up in patchwork at all :S Hopefully I can
still pull reight manually without the scripts :)
> Note: Patch 1/3 touches "drivers/net/can/dev.c". In net-next/master this file
> has been moved to drivers/net/can/dev/dev.c [1] and parts of it have been
> transfered into separate files. This may result in a merge conflict. Please
> carry this patch forward, the change is rather simple. Drop us a note if
> needed. Are any actions needed with regards to linux-next?
Thanks for the note, I'm sending the PR to Linus now, so I think
linux-next may never see the the conflict.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: pull-request: can 2021-01-20
2021-01-20 17:19 ` pull-request: can 2021-01-20 Jakub Kicinski
@ 2021-01-20 20:20 ` Marc Kleine-Budde
2021-01-20 20:36 ` Jakub Kicinski
0 siblings, 1 reply; 10+ messages in thread
From: Marc Kleine-Budde @ 2021-01-20 20:20 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, linux-can, kernel
[-- Attachment #1.1: Type: text/plain, Size: 1489 bytes --]
On 1/20/21 6:19 PM, Jakub Kicinski wrote:
>> this is a pull request of 3 patches for net/master.
>>
>> All three patches are by Vincent Mailhol and fix a potential use after free bug
>> in the CAN device infrastructure, the vxcan driver, and the peak_usk driver. In
>> the TX-path the skb is used to read from after it was passed to the networking
>> stack with netif_rx_ni().
>
> Pulled, thanks.
>
> Seems like the PR didn't show up in patchwork at all :S Hopefully I can
> still pull reight manually without the scripts :)
Fingers crossed. :D
Today I noticed a lag of >4h on vger.kernel.org. Even this mail of yours hasn't
made it to the linux-can list, yet. It's 3h delayed.
>> Note: Patch 1/3 touches "drivers/net/can/dev.c". In net-next/master this file
>> has been moved to drivers/net/can/dev/dev.c [1] and parts of it have been
>> transfered into separate files. This may result in a merge conflict. Please
>> carry this patch forward, the change is rather simple. Drop us a note if
>> needed. Are any actions needed with regards to linux-next?
>
> Thanks for the note, I'm sending the PR to Linus now, so I think
> linux-next may never see the the conflict.
thanks,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: pull-request: can 2021-01-20
2021-01-20 20:20 ` Marc Kleine-Budde
@ 2021-01-20 20:36 ` Jakub Kicinski
2021-01-20 21:03 ` Marc Kleine-Budde
0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2021-01-20 20:36 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: netdev, davem, linux-can, kernel
On Wed, 20 Jan 2021 21:20:13 +0100 Marc Kleine-Budde wrote:
> On 1/20/21 6:19 PM, Jakub Kicinski wrote:
> >> this is a pull request of 3 patches for net/master.
> >>
> >> All three patches are by Vincent Mailhol and fix a potential use after free bug
> >> in the CAN device infrastructure, the vxcan driver, and the peak_usk driver. In
> >> the TX-path the skb is used to read from after it was passed to the networking
> >> stack with netif_rx_ni().
> >
> > Pulled, thanks.
> >
> > Seems like the PR didn't show up in patchwork at all :S Hopefully I can
> > still pull reight manually without the scripts :)
>
> Fingers crossed. :D
>
> Today I noticed a lag of >4h on vger.kernel.org. Even this mail of yours hasn't
> made it to the linux-can list, yet. It's 3h delayed.
It's been reported but it's unclear what's causing this one :(
> >> Note: Patch 1/3 touches "drivers/net/can/dev.c". In net-next/master this file
> >> has been moved to drivers/net/can/dev/dev.c [1] and parts of it have been
> >> transfered into separate files. This may result in a merge conflict. Please
> >> carry this patch forward, the change is rather simple. Drop us a note if
> >> needed. Are any actions needed with regards to linux-next?
> >
> > Thanks for the note, I'm sending the PR to Linus now, so I think
> > linux-next may never see the the conflict.
The merge has been done now, could you double check?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: pull-request: can 2021-01-20
2021-01-20 20:36 ` Jakub Kicinski
@ 2021-01-20 21:03 ` Marc Kleine-Budde
0 siblings, 0 replies; 10+ messages in thread
From: Marc Kleine-Budde @ 2021-01-20 21:03 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, kernel, linux-can
[-- Attachment #1.1: Type: text/plain, Size: 925 bytes --]
On 1/20/21 9:36 PM, Jakub Kicinski wrote:
>>>> Note: Patch 1/3 touches "drivers/net/can/dev.c". In net-next/master this file
>>>> has been moved to drivers/net/can/dev/dev.c [1] and parts of it have been
>>>> transfered into separate files. This may result in a merge conflict. Please
>>>> carry this patch forward, the change is rather simple. Drop us a note if
>>>> needed. Are any actions needed with regards to linux-next?
>>>
>>> Thanks for the note, I'm sending the PR to Linus now, so I think
>>> linux-next may never see the the conflict.
>
> The merge has been done now, could you double check?
Looks good!
Thanks,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: pull-request: can 2021-01-20
2021-01-20 12:51 pull-request: can 2021-01-20 Marc Kleine-Budde
` (3 preceding siblings ...)
2021-01-20 17:19 ` pull-request: can 2021-01-20 Jakub Kicinski
@ 2021-01-20 20:40 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-01-20 20:40 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: netdev, davem, kuba, linux-can, kernel
Hello:
This pull request was applied to netdev/net-next.git (refs/heads/master):
On Wed, 20 Jan 2021 13:51:59 +0100 you wrote:
> Hello Jakub, hello David,
>
> this is a pull request of 3 patches for net/master.
>
> All three patches are by Vincent Mailhol and fix a potential use after free bug
> in the CAN device infrastructure, the vxcan driver, and the peak_usk driver. In
> the TX-path the skb is used to read from after it was passed to the networking
> stack with netif_rx_ni().
>
> [...]
Here is the summary with links:
- pull-request: can 2021-01-20
https://git.kernel.org/netdev/net-next/c/535d31593f59
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 10+ messages in thread