linux-kernel - Re: [PATCH] net/can/gs_usb: increase max interface to 255

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: <b12cd2aa-fc30-46ef-b748-def1b291f377@coelacanthus.name>
Date: Tue, 30 Sep 2025 11:03:18 +0800
From: Celeste Liu <uwu@...lacanthus.name>
To: Vincent Mailhol <mailhol@...nel.org>,
 Marc Kleine-Budde <mkl@...gutronix.de>
Cc: Maximilian Schneider <max@...neidersoft.net>,
 Henrik Brix Andersen <henrik@...xandersen.dk>,
 Wolfgang Grandegger <wg@...ndegger.com>, linux-can@...r.kernel.org,
 linux-kernel@...r.kernel.org, Runcheng Lu <runcheng.lu@...icro.com>
Subject: Re: [PATCH] net/can/gs_usb: increase max interface to 255

On 2025-09-29 22:52, Vincent Mailhol wrote:
> Hi Celeste,
> 
> Thank you for your patch. Here are my comments.
> 
> On 9/29/25 8:10 PM, Celeste Liu wrote:
>> This issue was found by Runcheng Lu when develop HSCanT USB to CAN FD
>> converter[1]. The original developers may have only 3 intefaces device to
>> test so they write 3 here and wait for future change.
>>
>> During the HSCanT development, we actually used 4 interfaces, so the
>> limitation of 3 is not enough now. But just increase one is not
>> future-proofed. Since the channel type in gs_host_frame is u8, use 255
>> as max interface number should be safe.
>>
>> [1]: https://github.com/cherry-embedded/HSCanT-hardware
>>
>> Reported-by: Runcheng Lu <runcheng.lu@...icro.com>
> 
> If you have any links where Runcheng reported the issue, you can add it here as:
> 
>   Closes: <URL to the bug report>
> 
> (Ignore if such message does not exist).

Yeah. No report is public to Internet.

> 
> Also add a Fixes tag so that the fix can be backported to stable.

Added in v2.

> 
>> Signed-off-by: Celeste Liu <uwu@...lacanthus.name>
>> ---
>>  drivers/net/can/usb/gs_usb.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
>> index c9482d6e947b0c7b033dc4f0c35f5b111e1bfd92..35fc257c19e57c1f33e03e7c86ea908d22400254 100644
>> --- a/drivers/net/can/usb/gs_usb.c
>> +++ b/drivers/net/can/usb/gs_usb.c
>> @@ -290,9 +290,9 @@ struct gs_host_frame {
>>  #define GS_NAPI_WEIGHT 32
>>  
>>  /* Maximum number of interfaces the driver supports per device.
>> - * Current hardware only supports 3 interfaces. The future may vary.
>> + * The channel number type of gs_host_frame is u8, so max interfaces can be 255.
>>   */
>> -#define GS_MAX_INTF 3
>> +#define GS_MAX_INTF 255
> 
> After doing this, you are left with an array of 255 pointers in struct gs_usb
> (which represents 2 kilobytes of memory on a 64 bits machine). You also have two
> loops iterating from 0 to 255. This is a bit of a waste of both space and
> processing power.
> 
> It is better to use a flexible array member, like this:

Has done in v2.

> 
> -----8<-----
> diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
> index c9482d6e947b..459a956ac0d9 100644
> --- a/drivers/net/can/usb/gs_usb.c
> +++ b/drivers/net/can/usb/gs_usb.c
> @@ -324,7 +324,6 @@ struct gs_can {
> 
>  /* usb interface struct */
>  struct gs_usb {
> -       struct gs_can *canch[GS_MAX_INTF];
>         struct usb_anchor rx_submitted;
>         struct usb_device *udev;
> 
> @@ -336,9 +335,11 @@ struct gs_usb {
> 
>         unsigned int hf_size_rx;
>         u8 active_channels;
> +       u8 channel_cnt;
> 
>         unsigned int pipe_in;
>         unsigned int pipe_out;
> +       struct gs_can *canch[] __counted_by(channel_cnt);
>  };
> 
>  /* 'allocate' a tx context.
> ----->8-----
> 
> Then all the instances of GS_MAX_INTF are replaced by gs_usb->channel_cnt except
> from the check on dfconf.icount which can be replaced by a:
> 
>   type_max(typeof(gs_usb->channel_cnt))
> 
> 
> Yours sincerely,
> Vincent Mailhol
>