linux-kernel - 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]

Message-ID: <20240615085133.GA234885@kernel.org>
Date: Sat, 15 Jun 2024 09:51:33 +0100
From: Simon Horman <horms@...nel.org>
To: michael.nemanov@...com
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 Sun, Jun 09, 2024 at 09:20:56PM +0300, michael.nemanov@...com wrote:
> From: Michael Nemanov <Michael.Nemanov@...com>
> 
> High-level init code for new vifs
> ---
>  drivers/net/wireless/ti/cc33xx/init.c | 236 ++++++++++++++++++++++++++
>  drivers/net/wireless/ti/cc33xx/init.h |  15 ++
>  2 files changed, 251 insertions(+)
>  create mode 100644 drivers/net/wireless/ti/cc33xx/init.c
>  create mode 100644 drivers/net/wireless/ti/cc33xx/init.h
> 
> diff --git a/drivers/net/wireless/ti/cc33xx/init.c b/drivers/net/wireless/ti/cc33xx/init.c

...

> +int cc33xx_init_vif_specific(struct cc33xx *cc, struct ieee80211_vif *vif)
> +{
> +	struct cc33xx_vif *wlvif = cc33xx_vif_to_data(vif);
> +	struct conf_tx_ac_category *conf_ac;
> +	struct conf_tx_ac_category ac_conf[4];
> +	struct conf_tx_tid tid_conf[8];
> +	struct conf_tx_settings *tx_settings = &cc->conf.host_conf.tx;
> +	struct conf_tx_ac_category *p_wl_host_ac_conf = &tx_settings->ac_conf0;
> +	struct conf_tx_tid *p_wl_host_tid_conf = &tx_settings->tid_conf0;
> +	bool is_ap = (wlvif->bss_type == BSS_TYPE_AP_BSS);
> +	u8 ps_scheme = cc->conf.mac.ps_scheme;
> +	int ret, i;

...

> +	/* Default TID/AC configuration */
> +	WARN_ON(tx_settings->tid_conf_count != tx_settings->ac_conf_count);
> +	memcpy(ac_conf, p_wl_host_ac_conf, 4 * sizeof(struct conf_tx_ac_category));
> +	memcpy(tid_conf, p_wl_host_tid_conf, 8 * sizeof(struct conf_tx_tid));

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)?


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

Thanks!

...