2024-03-28 13:55:36

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 0/2] wifi: ath: false-positive fortified-memset warnings

From: Arnd Bergmann <[email protected]>

While testing some other patch series I worked on across gcc versions, I found
a couple of stringop warnings that only show up with some toolchains but not
others. The warnings I both seem to be false positive.

I have also not found an explanation why both of these happen in atheros
wireless drivers, as I don't see this in other drivers.

Maybe Kees can work out what is going on here.

Arnd Bergmann (2):
[RESEND] carl9170: re-fix fortified-memset warning
ath9k: work around memset overflow warning

drivers/net/wireless/ath/ath.h | 6 ++++--
drivers/net/wireless/ath/ath9k/main.c | 3 +--
drivers/net/wireless/ath/carl9170/tx.c | 3 ++-
3 files changed, 7 insertions(+), 5 deletions(-)

--
2.39.2



2024-03-28 13:56:08

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 2/2] wifi: ath9k: work around memset overflow warning

From: Arnd Bergmann <[email protected]>

gcc-9 and some other older versions produce a false-positive warning
for zeroing two fields

In file included from include/linux/string.h:369,
from drivers/net/wireless/ath/ath9k/main.c:18:
In function 'fortify_memset_chk',
inlined from 'ath9k_ps_wakeup' at drivers/net/wireless/ath/ath9k/main.c:140:3:
include/linux/fortify-string.h:462:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
462 | __write_overflow_field(p_size_field, size);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Using a struct_group seems to reliably avoid the warning and
not make the code much uglier. The combined memset() should even
save a couple of cpu cycles.

Signed-off-by: Arnd Bergmann <[email protected]>
---
This is from randconfig testing, see https://pastebin.com/yjKk5N81
for a reproducer
---
drivers/net/wireless/ath/ath.h | 6 ++++--
drivers/net/wireless/ath/ath9k/main.c | 3 +--
2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index f02a308a9ffc..34654f710d8a 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -171,8 +171,10 @@ struct ath_common {
unsigned int clockrate;

spinlock_t cc_lock;
- struct ath_cycle_counters cc_ani;
- struct ath_cycle_counters cc_survey;
+ struct_group(cc,
+ struct ath_cycle_counters cc_ani;
+ struct ath_cycle_counters cc_survey;
+ );

struct ath_regulatory regulatory;
struct ath_regulatory reg_world_copy;
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index a2943aaecb20..01173aac3045 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -135,8 +135,7 @@ void ath9k_ps_wakeup(struct ath_softc *sc)
if (power_mode != ATH9K_PM_AWAKE) {
spin_lock(&common->cc_lock);
ath_hw_cycle_counters_update(common);
- memset(&common->cc_survey, 0, sizeof(common->cc_survey));
- memset(&common->cc_ani, 0, sizeof(common->cc_ani));
+ memset(&common->cc, 0, sizeof(common->cc));
spin_unlock(&common->cc_lock);
}

--
2.39.2


2024-03-28 21:15:08

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH 2/2] wifi: ath9k: work around memset overflow warning

Arnd Bergmann <[email protected]> writes:

> From: Arnd Bergmann <[email protected]>
>
> gcc-9 and some other older versions produce a false-positive warning
> for zeroing two fields
>
> In file included from include/linux/string.h:369,
> from drivers/net/wireless/ath/ath9k/main.c:18:
> In function 'fortify_memset_chk',
> inlined from 'ath9k_ps_wakeup' at drivers/net/wireless/ath/ath9k/main.c:140:3:
> include/linux/fortify-string.h:462:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
> 462 | __write_overflow_field(p_size_field, size);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Using a struct_group seems to reliably avoid the warning and
> not make the code much uglier. The combined memset() should even
> save a couple of cpu cycles.
>
> Signed-off-by: Arnd Bergmann <[email protected]>

Acked-by: Toke Høiland-Jørgensen <[email protected]>

2024-03-28 21:50:55

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 2/2] wifi: ath9k: work around memset overflow warning

On Thu, Mar 28, 2024 at 02:55:05PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> gcc-9 and some other older versions produce a false-positive warning
> for zeroing two fields
>
> In file included from include/linux/string.h:369,
> from drivers/net/wireless/ath/ath9k/main.c:18:
> In function 'fortify_memset_chk',
> inlined from 'ath9k_ps_wakeup' at drivers/net/wireless/ath/ath9k/main.c:140:3:
> include/linux/fortify-string.h:462:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
> 462 | __write_overflow_field(p_size_field, size);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Using a struct_group seems to reliably avoid the warning and
> not make the code much uglier. The combined memset() should even
> save a couple of cpu cycles.
>
> Signed-off-by: Arnd Bergmann <[email protected]>

Reviewed-by: Kees Cook <[email protected]>

--
Kees Cook