netdev - Re: [PATCH v2 11/17] wifi: cc33xx: Add init.c, init.h

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]

Date: Thu, 20 Jun 2024 11:40:31 +0300
From: "Nemanov, Michael" <michael.nemanov@...com>
To: Simon Horman <horms@...nel.org>
CC: Sabeeh Khan <sabeeh-khan@...com>, Kalle Valo <kvalo@...nel.org>,
        Johannes
 Berg <johannes.berg@...el.com>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
        Paolo
 Abeni <pabeni@...hat.com>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski
	<krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, <linux-wireless@...r.kernel.org>,
        <netdev@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 11/17] wifi: cc33xx: Add init.c, init.h

On 6/15/2024 11:51 AM, Simon Horman wrote:
...

> 
> Hi Michael,
> 
> allmodconfig builds on x86_64 with gcc-13 flag the following:
> 
> In file included from ./include/linux/string.h:374,
>                   from ./include/linux/bitmap.h:13,
>                   from ./include/linux/cpumask.h:13,
>                   from ./arch/x86/include/asm/paravirt.h:21,
>                   from ./arch/x86/include/asm/irqflags.h:60,
>                   from ./include/linux/irqflags.h:18,
>                   from ./include/linux/spinlock.h:59,
>                   from ./include/linux/mmzone.h:8,
>                   from ./include/linux/gfp.h:7,
>                   from ./include/linux/firmware.h:8,
>                   from drivers/net/wireless/ti/cc33xx/init.c:6:
> In function 'fortify_memcpy_chk',
>      inlined from 'cc33xx_init_vif_specific' at drivers/net/wireless/ti/cc33xx/init.c:156:2:
> ./include/linux/fortify-string.h:580:25: warning: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Wattribute-warning]
>    580 |                         __read_overflow2_field(q_size_field, size);
>        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In function 'fortify_memcpy_chk',
>      inlined from 'cc33xx_init_vif_specific' at drivers/net/wireless/ti/cc33xx/init.c:157:2:
> ./include/linux/fortify-string.h:580:25: warning: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Wattribute-warning]
>    580 |                         __read_overflow2_field(q_size_field, size);
>        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    CC [M]  drivers/net/wireless/ti/cc33xx/rx.o
> 
> I believe that this is because the destination for each of the two memcpy()
> calls immediately above is too narrow - 1 structure wide instead of 4 or 8.
> 
> I think this can be resolved by either using:
> 1. struct_group in .../cc33xx/conf.h:struct conf_tx_settings
>     to wrap ac_conf0 ... ac_conf3, and separately tid_conf0 ... tid_conf7.
> 2. Using arrays for ac_conf and tid_conf in
>     .../cc33xx/conf.h:struct conf_tx_settings, in which case perhaps
>     .../wlcore/conf.h:struct conf_tx_settings can be reused somehow
>     (I did not check closely)?
> 

Thank you for checking. I agree this code should be rewritten so it is 
more clear and w/o any warnings. Will fix.

I was unsuccessful reproducing the warning on my end. Tried with GCC 
13.2.0 (ARCH=x86_64, allmodconfig) and Arm GNU Toolchain 13.2 (ARCH=arm, 
allmodconfig) and only got errors in scan.c which I assume you refer to 
below (will also be fixed).

> 
> Similar errors are flagged elsewhere in this series.
> Please take a look at allmodconfig builds and make sure
> no warnings are introduced.
> 
> Lastly, more related to the series as a whole than this patch in
> particular, please consider running checkpatch.pl --codespell

Sure, will add checkpatch.pl --codespell to my tests.

Michael.