linux-kernel - Re: [PATCH v2] can: kvaser_usb: leaf: Fix potential infinite loop in command parsers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1d960d0d-06ab-4f38-817f-b9a5e949d3c7@kvaser.com>
Date: Sun, 26 Oct 2025 14:26:59 +0100
From: Jimmy Assarsson <extja@...ser.com>
To: pip-izony <eeodqql09@...il.com>, Marc Kleine-Budde <mkl@...gutronix.de>,
 Vincent Mailhol <mailhol@...nel.org>
Cc: Kyungtae Kim <Kyungtae.Kim@...tmouth.edu>, linux-can@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] can: kvaser_usb: leaf: Fix potential infinite loop in
 command parsers

Hi Seungjin,

Thanks for fixing this!
I'll do some testing in the beginning of next week.
Which Kvaser device did you use when you discovered the problem?

Best regards,
jimmy

On 10/23/25 18:27, pip-izony wrote:
> From: Seungjin Bae <eeodqql09@...il.com>
> 
> The `kvaser_usb_leaf_wait_cmd()` and `kvaser_usb_leaf_read_bulk_callback`
> functions contain logic to zero-length commands. These commands are used
> to align data to the USB endpoint's wMaxPacketSize boundary.
> 
> The driver attempts to skip these placeholders by aligning the buffer
> position `pos` to the next packet boundary using `round_up()` function.
> 
> However, if zero-length command is found exactly on a packet boundary
> (i.e., `pos` is a multiple of wMaxPacketSize, including 0), `round_up`
> function will return the unchanged value of `pos`. This prevents `pos`
> to be increased, causing an infinite loop in the parsing logic.
> 
> This patch fixes this in the function by using `pos + 1` instead.
> This ensures that even if `pos` is on a boundary, the calculation is
> based on `pos + 1`, forcing `round_up()` to always return the next
> aligned boundary.
> 
> Fixes: 7259124eac7d ("can: kvaser_usb: Split driver into kvaser_usb_core.c and kvaser_usb_leaf.c")
> Signed-off-by: Seungjin Bae <eeodqql09@...il.com>
> ---
>   v1 -> v2: Apply the same infinite loop fix to kvaser_usb_leaf_wait_cmd()
>   
>   drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> index c29828a94ad0..1167d38344f1 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> @@ -685,7 +685,7 @@ static int kvaser_usb_leaf_wait_cmd(const struct kvaser_usb *dev, u8 id,
>   			 * for further details.
>   			 */
>   			if (tmp->len == 0) {
> -				pos = round_up(pos,
> +				pos = round_up(pos + 1,
>   					       le16_to_cpu
>   						(dev->bulk_in->wMaxPacketSize));
>   				continue;
> @@ -1732,7 +1732,7 @@ static void kvaser_usb_leaf_read_bulk_callback(struct kvaser_usb *dev,
>   		 * number of events in case of a heavy rx load on the bus.
>   		 */
>   		if (cmd->len == 0) {
> -			pos = round_up(pos, le16_to_cpu
> +			pos = round_up(pos + 1, le16_to_cpu
>   						(dev->bulk_in->wMaxPacketSize));
>   			continue;
>   		}