| From a2e39bc3dd0ccdc1358d1b941e93b090c316dab7 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Thu, 13 Aug 2020 17:06:04 +0300 |
| Subject: can: peak_usb: add range checking in decode operations |
| |
| From: Dan Carpenter <dan.carpenter@oracle.com> |
| |
| [ Upstream commit a6921dd524fe31d1f460c161d3526a407533b6db ] |
| |
| These values come from skb->data so Smatch considers them untrusted. I |
| believe Smatch is correct but I don't have a way to test this. |
| |
| The usb_if->dev[] array has 2 elements but the index is in the 0-15 |
| range without checks. The cfd->len can be up to 255 but the maximum |
| valid size is CANFD_MAX_DLEN (64) so that could lead to memory |
| corruption. |
| |
| Fixes: 0a25e1f4f185 ("can: peak_usb: add support for PEAK new CANFD USB adapters") |
| Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> |
| Link: https://lore.kernel.org/r/20200813140604.GA456946@mwanda |
| Acked-by: Stephane Grosjean <s.grosjean@peak-system.com> |
| Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 48 +++++++++++++++++----- |
| 1 file changed, 37 insertions(+), 11 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 40647b837b31f..d314e73f3d061 100644 |
| --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c |
| +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c |
| @@ -475,12 +475,18 @@ static int pcan_usb_fd_decode_canmsg(struct pcan_usb_fd_if *usb_if, |
| struct pucan_msg *rx_msg) |
| { |
| struct pucan_rx_msg *rm = (struct pucan_rx_msg *)rx_msg; |
| - struct peak_usb_device *dev = usb_if->dev[pucan_msg_get_channel(rm)]; |
| - struct net_device *netdev = dev->netdev; |
| + struct peak_usb_device *dev; |
| + struct net_device *netdev; |
| struct canfd_frame *cfd; |
| struct sk_buff *skb; |
| const u16 rx_msg_flags = le16_to_cpu(rm->flags); |
| |
| + if (pucan_msg_get_channel(rm) >= ARRAY_SIZE(usb_if->dev)) |
| + return -ENOMEM; |
| + |
| + dev = usb_if->dev[pucan_msg_get_channel(rm)]; |
| + netdev = dev->netdev; |
| + |
| if (rx_msg_flags & PUCAN_MSG_EXT_DATA_LEN) { |
| /* CANFD frame case */ |
| skb = alloc_canfd_skb(netdev, &cfd); |
| @@ -527,15 +533,21 @@ static int pcan_usb_fd_decode_status(struct pcan_usb_fd_if *usb_if, |
| struct pucan_msg *rx_msg) |
| { |
| struct pucan_status_msg *sm = (struct pucan_status_msg *)rx_msg; |
| - struct peak_usb_device *dev = usb_if->dev[pucan_stmsg_get_channel(sm)]; |
| - struct pcan_usb_fd_device *pdev = |
| - container_of(dev, struct pcan_usb_fd_device, dev); |
| + struct pcan_usb_fd_device *pdev; |
| enum can_state new_state = CAN_STATE_ERROR_ACTIVE; |
| enum can_state rx_state, tx_state; |
| - struct net_device *netdev = dev->netdev; |
| + struct peak_usb_device *dev; |
| + struct net_device *netdev; |
| struct can_frame *cf; |
| struct sk_buff *skb; |
| |
| + if (pucan_stmsg_get_channel(sm) >= ARRAY_SIZE(usb_if->dev)) |
| + return -ENOMEM; |
| + |
| + dev = usb_if->dev[pucan_stmsg_get_channel(sm)]; |
| + pdev = container_of(dev, struct pcan_usb_fd_device, dev); |
| + netdev = dev->netdev; |
| + |
| /* nothing should be sent while in BUS_OFF state */ |
| if (dev->can.state == CAN_STATE_BUS_OFF) |
| return 0; |
| @@ -588,9 +600,14 @@ static int pcan_usb_fd_decode_error(struct pcan_usb_fd_if *usb_if, |
| struct pucan_msg *rx_msg) |
| { |
| struct pucan_error_msg *er = (struct pucan_error_msg *)rx_msg; |
| - struct peak_usb_device *dev = usb_if->dev[pucan_ermsg_get_channel(er)]; |
| - struct pcan_usb_fd_device *pdev = |
| - container_of(dev, struct pcan_usb_fd_device, dev); |
| + struct pcan_usb_fd_device *pdev; |
| + struct peak_usb_device *dev; |
| + |
| + if (pucan_ermsg_get_channel(er) >= ARRAY_SIZE(usb_if->dev)) |
| + return -EINVAL; |
| + |
| + dev = usb_if->dev[pucan_ermsg_get_channel(er)]; |
| + pdev = container_of(dev, struct pcan_usb_fd_device, dev); |
| |
| /* keep a trace of tx and rx error counters for later use */ |
| pdev->bec.txerr = er->tx_err_cnt; |
| @@ -604,11 +621,17 @@ static int pcan_usb_fd_decode_overrun(struct pcan_usb_fd_if *usb_if, |
| struct pucan_msg *rx_msg) |
| { |
| struct pcan_ufd_ovr_msg *ov = (struct pcan_ufd_ovr_msg *)rx_msg; |
| - struct peak_usb_device *dev = usb_if->dev[pufd_omsg_get_channel(ov)]; |
| - struct net_device *netdev = dev->netdev; |
| + struct peak_usb_device *dev; |
| + struct net_device *netdev; |
| struct can_frame *cf; |
| struct sk_buff *skb; |
| |
| + if (pufd_omsg_get_channel(ov) >= ARRAY_SIZE(usb_if->dev)) |
| + return -EINVAL; |
| + |
| + dev = usb_if->dev[pufd_omsg_get_channel(ov)]; |
| + netdev = dev->netdev; |
| + |
| /* allocate an skb to store the error frame */ |
| skb = alloc_can_err_skb(netdev, &cf); |
| if (!skb) |
| @@ -726,6 +749,9 @@ static int pcan_usb_fd_encode_msg(struct peak_usb_device *dev, |
| u16 tx_msg_size, tx_msg_flags; |
| u8 can_dlc; |
| |
| + if (cfd->len > CANFD_MAX_DLEN) |
| + return -EINVAL; |
| + |
| tx_msg_size = ALIGN(sizeof(struct pucan_tx_msg) + cfd->len, 4); |
| tx_msg->size = cpu_to_le16(tx_msg_size); |
| tx_msg->type = cpu_to_le16(PUCAN_MSG_CAN_TX); |
| -- |
| 2.27.0 |
| |