Commit a6921dd5 authored by Dan Carpenter's avatar Dan Carpenter Committed by Marc Kleine-Budde

can: peak_usb: add range checking in decode operations

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: 0a25e1f4 ("can: peak_usb: add support for PEAK new CANFD USB adapters")
Signed-off-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
Link: https://lore.kernel.org/r/20200813140604.GA456946@mwandaAcked-by: default avatarStephane Grosjean <s.grosjean@peak-system.com>
Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parent 79c43333
...@@ -468,12 +468,18 @@ static int pcan_usb_fd_decode_canmsg(struct pcan_usb_fd_if *usb_if, ...@@ -468,12 +468,18 @@ static int pcan_usb_fd_decode_canmsg(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg) struct pucan_msg *rx_msg)
{ {
struct pucan_rx_msg *rm = (struct pucan_rx_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 peak_usb_device *dev;
struct net_device *netdev = dev->netdev; struct net_device *netdev;
struct canfd_frame *cfd; struct canfd_frame *cfd;
struct sk_buff *skb; struct sk_buff *skb;
const u16 rx_msg_flags = le16_to_cpu(rm->flags); 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) { if (rx_msg_flags & PUCAN_MSG_EXT_DATA_LEN) {
/* CANFD frame case */ /* CANFD frame case */
skb = alloc_canfd_skb(netdev, &cfd); skb = alloc_canfd_skb(netdev, &cfd);
...@@ -519,15 +525,21 @@ static int pcan_usb_fd_decode_status(struct pcan_usb_fd_if *usb_if, ...@@ -519,15 +525,21 @@ static int pcan_usb_fd_decode_status(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg) struct pucan_msg *rx_msg)
{ {
struct pucan_status_msg *sm = (struct pucan_status_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;
struct pcan_usb_fd_device *pdev =
container_of(dev, struct pcan_usb_fd_device, dev);
enum can_state new_state = CAN_STATE_ERROR_ACTIVE; enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
enum can_state rx_state, tx_state; 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 can_frame *cf;
struct sk_buff *skb; 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 */ /* nothing should be sent while in BUS_OFF state */
if (dev->can.state == CAN_STATE_BUS_OFF) if (dev->can.state == CAN_STATE_BUS_OFF)
return 0; return 0;
...@@ -579,9 +591,14 @@ static int pcan_usb_fd_decode_error(struct pcan_usb_fd_if *usb_if, ...@@ -579,9 +591,14 @@ static int pcan_usb_fd_decode_error(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg) struct pucan_msg *rx_msg)
{ {
struct pucan_error_msg *er = (struct pucan_error_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;
struct pcan_usb_fd_device *pdev = struct peak_usb_device *dev;
container_of(dev, struct pcan_usb_fd_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 */ /* keep a trace of tx and rx error counters for later use */
pdev->bec.txerr = er->tx_err_cnt; pdev->bec.txerr = er->tx_err_cnt;
...@@ -595,11 +612,17 @@ static int pcan_usb_fd_decode_overrun(struct pcan_usb_fd_if *usb_if, ...@@ -595,11 +612,17 @@ static int pcan_usb_fd_decode_overrun(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg) struct pucan_msg *rx_msg)
{ {
struct pcan_ufd_ovr_msg *ov = (struct pcan_ufd_ovr_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 peak_usb_device *dev;
struct net_device *netdev = dev->netdev; struct net_device *netdev;
struct can_frame *cf; struct can_frame *cf;
struct sk_buff *skb; 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 */ /* allocate an skb to store the error frame */
skb = alloc_can_err_skb(netdev, &cf); skb = alloc_can_err_skb(netdev, &cf);
if (!skb) if (!skb)
...@@ -716,6 +739,9 @@ static int pcan_usb_fd_encode_msg(struct peak_usb_device *dev, ...@@ -716,6 +739,9 @@ static int pcan_usb_fd_encode_msg(struct peak_usb_device *dev,
u16 tx_msg_size, tx_msg_flags; u16 tx_msg_size, tx_msg_flags;
u8 can_dlc; 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 = ALIGN(sizeof(struct pucan_tx_msg) + cfd->len, 4);
tx_msg->size = cpu_to_le16(tx_msg_size); tx_msg->size = cpu_to_le16(tx_msg_size);
tx_msg->type = cpu_to_le16(PUCAN_MSG_CAN_TX); tx_msg->type = cpu_to_le16(PUCAN_MSG_CAN_TX);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment