netdev - Re: [PATCH net-next v8 02/14] xsc: Enable command queue

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <ca224adb-9604-4e96-b34e-79620ab7ae54@yunsilicon.com>
Date: Tue, 18 Mar 2025 18:59:32 +0800
From: "Xin Tian" <tianx@...silicon.com>
To: "Geert Uytterhoeven" <geert@...ux-m68k.org>
Cc: "Simon Horman" <horms@...nel.org>, <netdev@...r.kernel.org>, 
	<leon@...nel.org>, <andrew+netdev@...n.ch>, <kuba@...nel.org>, 
	<pabeni@...hat.com>, <edumazet@...gle.com>, <davem@...emloft.net>, 
	<jeff.johnson@....qualcomm.com>, <przemyslaw.kitszel@...el.com>, 
	<weihg@...silicon.com>, <wanry@...silicon.com>, <jacky@...silicon.com>, 
	<parthiban.veerasooran@...rochip.com>, <masahiroy@...nel.org>, 
	<kalesh-anakkur.purayil@...adcom.com>, <geert+renesas@...der.be>
Subject: Re: [PATCH net-next v8 02/14] xsc: Enable command queue

On 2025/3/18 18:29, Geert Uytterhoeven wrote:
> On Tue, 18 Mar 2025 at 11:06, Xin Tian <tianx@...silicon.com> wrote:
>> On 2025/3/12 17:17, Xin Tian wrote:
>>> On 2025/3/10 14:34, Simon Horman wrote:
>>>> On Fri, Mar 07, 2025 at 06:08:29PM +0800, Xin Tian wrote:
>>>>> The command queue is a hardware channel for sending
>>>>> commands between the driver and the firmware.
>>>>> xsc_cmd.h defines the command protocol structures.
>>>>> The logic for command allocation, sending,
>>>>> completion handling, and error handling is implemented
>>>>> in cmdq.c.
>>>>>
>>>>> Co-developed-by: Honggang Wei <weihg@...silicon.com>
>>>>> Signed-off-by: Honggang Wei <weihg@...silicon.com>
>>>>> Co-developed-by: Lei Yan <jacky@...silicon.com>
>>>>> Signed-off-by: Lei Yan <jacky@...silicon.com>
>>>>> Signed-off-by: Xin Tian <tianx@...silicon.com>
>>>> Hi Xin,
>>>>
>>>> Some minor feedback from my side.
>>>>
>>>> ...
>>>>
>>>>> diff --git a/drivers/net/ethernet/yunsilicon/xsc/pci/cmdq.c b/drivers/net/ethernet/yunsilicon/xsc/pci/cmdq.c
>>>> ...
>>>>
>>>>> +static int xsc_copy_to_cmd_msg(struct xsc_cmd_msg *to, void *from, int size)
>>>>> +{
>>>>> +   struct xsc_cmd_prot_block *block;
>>>>> +   struct xsc_cmd_mailbox *next;
>>>>> +   int copy;
>>>>> +
>>>>> +   if (!to || !from)
>>>>> +           return -ENOMEM;
>>>>> +
>>>>> +   copy = min_t(int, size, sizeof(to->first.data));
>>>> nit: I expect that using min() is sufficient here...
>>> Ack
>> min(size, sizeof(to->first.data)) will lead to a compile warning.
>> size is int and sizeof(to->first.data) is size_t.
>> So I kept this in v9
> Sizes should be unsigned, perhaps even size_t (depending on the
> expected maximum size).
> What if someone passes a negative number? Then copy will be negative
> too.  When calling memcpy(), it will be promoted to a very large
> unsigned number, ... boom!
>
> Gr{oetje,eeting}s,
>
>                          Geert
You're right; I think I still have some sizes that need to be modified. 
Thanks for the reminder!🙂
>