RE: [PATCH v2] can: peak_usb: Add bounds check for USB channel index

[Stéphane Grosjean]

Hi James,

Thanks for the fix.

I've reviewed the patch and it looks good to me.

Acked-by: Stéphane Grosjean <s.grosjean@xxxxxxxxxxxxxx>

— Stéphane

________________________________________
De : James Gao <jamesgao5@xxxxxxxxxxx>
Envoyé : mercredi 13 mai 2026 11:51
À : mkl@xxxxxxxxxxxxxx <mkl@xxxxxxxxxxxxxx>; mailhol@xxxxxxxxxx <mailhol@xxxxxxxxxx>
Cc : linux-can@xxxxxxxxxxxxxxx <linux-can@xxxxxxxxxxxxxxx>; Stéphane Grosjean <s.grosjean@xxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>; James Gao <jamesgao5@xxxxxxxxxxx>
Objet : [PATCH v2] can: peak_usb: Add bounds check for USB channel index
 
The channel control index ctrl_idx is derived from rx->len which comes
directly from a device USB payload. The mask 0x0f allows values 0-15,
but the array size of usb_if->dev[] is only 2. Values 2-15 cause heap
out-of-bounds read, eventually causing kernel panic in the IRQ context.

Add bounds checking for ctrl_idx before the array access in both
pcan_usb_pro_handle_canmsg() and pcan_usb_pro_handle_error().

Fixes: d8a199355f8f ("can: usb: PEAK-System Technik PCAN-USB Pro specific part")
Signed-off-by: James Gao <jamesgao5@xxxxxxxxxxx>
---

Hi Marc,
        Thanks for the review for v1. Since this is not a security bug I have
        moved the discussion to public mail list. Changes in v2:
        - Use ARRAY_SIZE(usb_if->dev) instead of PCAN_USBPRO_CHANNEL_COUNT
        - Use -EINVAL instead of -ENOMEM
        - Add newline between variable declaration and bounds check
        - Wrap commit message

 drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
index 4bfa8d0fbb32..5b1e3501cfb8 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
@@ -534,6 +534,10 @@ static int pcan_usb_pro_handle_canmsg(struct pcan_usb_pro_interface *usb_if,
                                       struct pcan_usb_pro_rxmsg *rx)
 {
         const unsigned int ctrl_idx = (rx->len >> 4) & 0x0f;
+
+       if (ctrl_idx >= ARRAY_SIZE(usb_if->dev))
+               return -EINVAL;
+
         struct peak_usb_device *dev = usb_if->dev[ctrl_idx];
         struct net_device *netdev = dev->netdev;
         struct can_frame *can_frame;
@@ -573,6 +577,10 @@ static int pcan_usb_pro_handle_error(struct pcan_usb_pro_interface *usb_if,
 {
         const u16 raw_status = le16_to_cpu(er->status);
         const unsigned int ctrl_idx = (er->channel >> 4) & 0x0f;
+
+       if (ctrl_idx >= ARRAY_SIZE(usb_if->dev))
+               return -EINVAL;
+
         struct peak_usb_device *dev = usb_if->dev[ctrl_idx];
         struct net_device *netdev = dev->netdev;
         struct can_frame *can_frame;
--
2.53.0