Re: [PATCH v3 1/2] can: rx-offload: make skb_irq_queue per-CPU

[Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>]

On 9/1/2026 1:57 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Ignoring the return value of can_rx_offload_add_manual() leads to a kernel panic in gs_usb if per-CPU allocation fails.
> --
> 
> commit d8e59325c82aef9b3489bcce81a1a5ee64835783
> Author: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> 
> can: rx-offload: make skb_irq_queue per-CPU
> 
> This patch converts skb_irq_queue to a per-CPU allocation to fix concurrent
> list enqueueing issues on platforms with multiple mailboxes affined to
> different CPUs.
> 
>> diff --git a/drivers/net/can/dev/rx-offload.c b/drivers/net/can/dev/rx-offload.c
>> index 46e7b6db4a1ef..649bfda08b654 100644
>> --- a/drivers/net/can/dev/rx-offload.c
>> +++ b/drivers/net/can/dev/rx-offload.c
> [ ... ]
>> @@ -353,13 +391,21 @@ static int can_rx_offload_init_queue(struct net_device *dev,
>>   				     struct can_rx_offload *offload,
>>   				     unsigned int weight)
>>   {
> [ ... ]
>> -	__skb_queue_head_init(&offload->skb_irq_queue);
>> +
>> +	offload->skb_irq_queue = alloc_percpu(struct sk_buff_head);
>> +	if (!offload->skb_irq_queue)
>> +		return -ENOMEM;
>> +
> 
> [Severity: High]
> Does this new error path cause a kernel panic in drivers that ignore the
> return value of can_rx_offload_add_manual()?
> 
> Looking at gs_make_candev() in drivers/net/can/usb/gs_usb.c, it does not
> check the return value of can_rx_offload_add_manual(). If alloc_percpu()
> fails here, the gs_usb driver proceeds with registration.
> 
> When a frame is later received, can_rx_offload_queue_tail() uses
> get_cpu_ptr(offload->skb_irq_queue). Passing a NULL pointer to get_cpu_ptr()
> results in an invalid pointer (0 + per-CPU offset), leading to a crash when
> it is unconditionally dereferenced by __skb_queue_tail().
> 
> I note an identical issue in at91_can.c was addressed in a subsequent patch
> ("can: at91_can: fix rx-offload cleanup on unbind and probe errors"), but it
> appears gs_usb.c was missed.
> 

Seems to be the same class issue as 'at91_can' one. Nice finding. I will
address it in V4.

Ciprian