lkml.org 
[lkml]   [2020]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.9 14/78] can: peak_usb: add range checking in decode operations
    Date
    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


    \
     
     \ /
      Last update: 2020-11-17 14:11    [W:2.782 / U:0.176 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site