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: <CAMuHMdXxkvE=o7VOpPqSo3dkd6=YP8iWJ5V_=S=uAyrCBygEjQ@mail.gmail.com>
Date: Tue, 18 Mar 2025 11:29:32 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Xin Tian <tianx@...silicon.com>
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 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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds