[PATCH RFC] wifi: ath12k: workaround fortify warnings in ath12k_wow_convert_8023_to_80211()

[PATCH RFC] wifi: ath12k: workaround fortify warnings in ath12k_wow_convert_8023_to_80211()

[Kalle Valo]

From: Kalle Valo <quic_kvalo@quicinc.com>

Johannes reported with GCC 11.4 there's a fortify warning below. The warning is
not seen with GCC 12.1 nor 13.2. Weirdly moving the other operand of sum to the
other side the warning goes away. This is safe to do as the value of the
operand is check earlier. But the code looks worse with this so I'm not sure
what to do.

In file included from ./include/linux/string.h:374,
                 from ./include/linux/bitmap.h:13,
                 from ./include/linux/cpumask.h:13,
                 from ./include/linux/sched.h:16,
                 from ./include/linux/delay.h:23,
                 from drivers/net/wireless/ath/ath12k/wow.c:7:
drivers/net/wireless/ath/ath12k/wow.c: In function ‘ath12k_wow_convert_8023_to_80211.constprop’:
./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ accessing 18446744073709551611 or more bytes at offsets 0 and 0 overlaps 9223372036854775799 bytes at offset -9223372036854775804 [-Werror=restrict]
  114 | #define __underlying_memcpy     __builtin_memcpy
      |                                 ^
./include/linux/fortify-string.h:637:9: note: in expansion of macro ‘__underlying_memcpy’
  637 |         __underlying_##op(p, q, __fortify_size);                              |         ^~~~~~~~~~~~~
./include/linux/fortify-string.h:682:26: note: in expansion of macro ‘__fortify_memcpy_chk’
  682 | #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                        |                          ^~~~~~~~~~~~~~~~~~~~
drivers/net/wireless/ath/ath12k/wow.c:190:25: note: in expansion of macro ‘memcpy’
  190 |                         memcpy(pat, eth_pat, eth_pat_len);
      |                         ^~~~~~
./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ accessing 18446744073709551605 or more bytes at offsets 0 and 0 overlaps 9223372036854775787 bytes at offset -9223372036854775798 [-Werror=restrict]
  114 | #define __underlying_memcpy     __builtin_memcpy
      |                                 ^
./include/linux/fortify-string.h:637:9: note: in expansion of macro ‘__underlying_memcpy’
  637 |         __underlying_##op(p, q, __fortify_size);                              |         ^~~~~~~~~~~~~
./include/linux/fortify-string.h:682:26: note: in expansion of macro ‘__fortify_memcpy_chk’
  682 | #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                        |                          ^~~~~~~~~~~~~~~~~~~~
drivers/net/wireless/ath/ath12k/wow.c:232:25: note: in expansion of macro ‘memcpy’
  232 |                         memcpy(pat, eth_pat, eth_pat_len);
      |                         ^~~~~~

Compile tested only.

Reported-by: Johannes Berg <johannes@sipsolutions.net>
Fixes: 4a3c212eee0e ("wifi: ath12k: add basic WoW functionalities")
Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/wow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/wow.c b/drivers/net/wireless/ath/ath12k/wow.c
index c5cba825a84a..e9588bb7561c 100644
--- a/drivers/net/wireless/ath/ath12k/wow.c
+++ b/drivers/net/wireless/ath/ath12k/wow.c
@@ -186,7 +186,7 @@ ath12k_wow_convert_8023_to_80211(struct ath12k *ar,
 	if (eth_pkt_ofs < ETH_ALEN) {
 		pkt_ofs = eth_pkt_ofs + a1_ofs;
 
-		if (eth_pkt_ofs + eth_pat_len < ETH_ALEN) {
+		if (eth_pat_len < ETH_ALEN - eth_pkt_ofs) {
 			memcpy(pat, eth_pat, eth_pat_len);
 			memcpy(bytemask, eth_bytemask, eth_pat_len);
 
@@ -228,7 +228,7 @@ ath12k_wow_convert_8023_to_80211(struct ath12k *ar,
 	} else if (eth_pkt_ofs < prot_ofs) {
 		pkt_ofs = eth_pkt_ofs - ETH_ALEN + a3_ofs;
 
-		if (eth_pkt_ofs + eth_pat_len < prot_ofs) {
+		if (eth_pat_len < prot_ofs - eth_pkt_ofs) {
 			memcpy(pat, eth_pat, eth_pat_len);
 			memcpy(bytemask, eth_bytemask, eth_pat_len);
 

base-commit: c1cacb01f35589bd41360cdb7535afc792c08a7c
-- 
2.39.2

Re: [PATCH RFC] wifi: ath12k: workaround fortify warnings in ath12k_wow_convert_8023_to_80211()

[Kees Cook]

On Thu, Jul 04, 2024 at 05:43:41PM +0300, Kalle Valo wrote:
> From: Kalle Valo <quic_kvalo@quicinc.com>
> 
> Johannes reported with GCC 11.4 there's a fortify warning below. The warning is
> not seen with GCC 12.1 nor 13.2. Weirdly moving the other operand of sum to the
> other side the warning goes away. This is safe to do as the value of the
> operand is check earlier. But the code looks worse with this so I'm not sure
> what to do.

FWIW, this isn't fortify, but -Wrestrict. I would expect the same
warnings even with CONFIG_FORTIFY_SOURCE disabled. Regardless, it's
worth figuring out what's going on. It looks like this is GCC's value
range tracker deciding it sees a way for things to go weird.

I suspect they fixed -Wrestrict in later GCC versions. It might need to
be version-limited...

> In file included from ./include/linux/string.h:374,
>                  from ./include/linux/bitmap.h:13,
>                  from ./include/linux/cpumask.h:13,
>                  from ./include/linux/sched.h:16,
>                  from ./include/linux/delay.h:23,
>                  from drivers/net/wireless/ath/ath12k/wow.c:7:
> drivers/net/wireless/ath/ath12k/wow.c: In function ‘ath12k_wow_convert_8023_to_80211.constprop’:
> ./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ accessing 18446744073709551611 or more bytes at offsets 0 and 0 overlaps 9223372036854775799 bytes at offset -9223372036854775804 [-Werror=restrict]

These huge negative values imply to me that GCC is looking at some
signed values somewhere.

> [...]
> diff --git a/drivers/net/wireless/ath/ath12k/wow.c b/drivers/net/wireless/ath/ath12k/wow.c
> index c5cba825a84a..e9588bb7561c 100644
> --- a/drivers/net/wireless/ath/ath12k/wow.c
> +++ b/drivers/net/wireless/ath/ath12k/wow.c
> @@ -186,7 +186,7 @@ ath12k_wow_convert_8023_to_80211(struct ath12k *ar,
>  	if (eth_pkt_ofs < ETH_ALEN) {
>  		pkt_ofs = eth_pkt_ofs + a1_ofs;
>  
> -		if (eth_pkt_ofs + eth_pat_len < ETH_ALEN) {
> +		if (eth_pat_len < ETH_ALEN - eth_pkt_ofs) {
>  			memcpy(pat, eth_pat, eth_pat_len);
>  			memcpy(bytemask, eth_bytemask, eth_pat_len);

Both eth_pkt_ofs and eth_pat_len are size_t. ETH_ALEN isn't, but it
would be promoted to size_t here. The value tracker should see that
eth_pkt_ofs could be [0..ETH_ALEN). eth_pat_len is coming from an "int",
though, so that might be the confusion. It may think eth_pat_len could
be [0..UINT_MAX] (i.e. the full range of int within size_t).

So [0..ETH_ALEN) + [0..UINT_MAX] < 6 might be doing something wrong in
GCC 11.x, and it's not actually doing the size_t promotion correctly,
or deciding something has wrapped and then thinking eth_pat_len could
span a giant region of the address space, which freaks out -Wrestrict.
i.e. it's seeing that for the "if" to be true, eth_pat_len could be large
enough to wrap around the addition (though this shouldn't be possible
for 64-bit size_t).

So I could see how [0..UINT_MAX] < 6 - [0..ETH_ALEN) would make it
happier: the right side is now [1..6], so eth_pat_len becomes [1..6).

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

Re: [PATCH RFC] wifi: ath12k: workaround fortify warnings in ath12k_wow_convert_8023_to_80211()

[Kalle Valo]

Kees Cook <kees@kernel.org> writes:

> On Thu, Jul 04, 2024 at 05:43:41PM +0300, Kalle Valo wrote:
>> From: Kalle Valo <quic_kvalo@quicinc.com>
>> 
>> Johannes reported with GCC 11.4 there's a fortify warning below. The warning is
>> not seen with GCC 12.1 nor 13.2. Weirdly moving the other operand of sum to the
>> other side the warning goes away. This is safe to do as the value of the
>> operand is check earlier. But the code looks worse with this so I'm not sure
>> what to do.
>
> FWIW, this isn't fortify, but -Wrestrict.

Ah, thanks for correcting. I just saw fortify-string.h and made the
wrong assumption.

> I would expect the same warnings even with CONFIG_FORTIFY_SOURCE
> disabled. Regardless, it's worth figuring out what's going on. It
> looks like this is GCC's value range tracker deciding it sees a way
> for things to go weird.
>
> I suspect they fixed -Wrestrict in later GCC versions. It might need to
> be version-limited...
>
>> In file included from ./include/linux/string.h:374,
>>                  from ./include/linux/bitmap.h:13,
>>                  from ./include/linux/cpumask.h:13,
>>                  from ./include/linux/sched.h:16,
>>                  from ./include/linux/delay.h:23,
>>                  from drivers/net/wireless/ath/ath12k/wow.c:7:
>> drivers/net/wireless/ath/ath12k/wow.c: In function
>> ‘ath12k_wow_convert_8023_to_80211.constprop’:
>> ./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’
>> accessing 18446744073709551611 or more bytes at offsets 0 and 0
>> overlaps 9223372036854775799 bytes at offset -9223372036854775804
>> [-Werror=restrict]
>
> These huge negative values imply to me that GCC is looking at some
> signed values somewhere.
>
>> [...]
>> diff --git a/drivers/net/wireless/ath/ath12k/wow.c b/drivers/net/wireless/ath/ath12k/wow.c
>> index c5cba825a84a..e9588bb7561c 100644
>> --- a/drivers/net/wireless/ath/ath12k/wow.c
>> +++ b/drivers/net/wireless/ath/ath12k/wow.c
>> @@ -186,7 +186,7 @@ ath12k_wow_convert_8023_to_80211(struct ath12k *ar,
>>  	if (eth_pkt_ofs < ETH_ALEN) {
>>  		pkt_ofs = eth_pkt_ofs + a1_ofs;
>>  
>> -		if (eth_pkt_ofs + eth_pat_len < ETH_ALEN) {
>> +		if (eth_pat_len < ETH_ALEN - eth_pkt_ofs) {
>>  			memcpy(pat, eth_pat, eth_pat_len);
>>  			memcpy(bytemask, eth_bytemask, eth_pat_len);
>
> Both eth_pkt_ofs and eth_pat_len are size_t. ETH_ALEN isn't, but it
> would be promoted to size_t here. The value tracker should see that
> eth_pkt_ofs could be [0..ETH_ALEN). eth_pat_len is coming from an "int",
> though, so that might be the confusion. It may think eth_pat_len could
> be [0..UINT_MAX] (i.e. the full range of int within size_t).
>
> So [0..ETH_ALEN) + [0..UINT_MAX] < 6 might be doing something wrong in
> GCC 11.x, and it's not actually doing the size_t promotion correctly,
> or deciding something has wrapped and then thinking eth_pat_len could
> span a giant region of the address space, which freaks out -Wrestrict.
> i.e. it's seeing that for the "if" to be true, eth_pat_len could be large
> enough to wrap around the addition (though this shouldn't be possible
> for 64-bit size_t).
>
> So I could see how [0..UINT_MAX] < 6 - [0..ETH_ALEN) would make it
> happier: the right side is now [1..6], so eth_pat_len becomes [1..6).

Earlier I did some testing and I noticed that this if test also gives a
warning:

1 + eth_pat_len < ETH_ALEN

But this doesn't have any warning:

0 + eth_pat_len < ETH_ALEN

And I stopped my investigation there :)

> Reviewed-by: Kees Cook <kees@kernel.org>

So you think this should be applied? It's not really logical so I would
prefer to avoid taking it if possible. Or should we just ignore the
warning? It only happens on GCC 11 anyway.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH RFC] wifi: ath12k: workaround fortify warnings in ath12k_wow_convert_8023_to_80211()

[Kees Cook]

On Mon, Jul 08, 2024 at 06:51:52PM +0300, Kalle Valo wrote:
> Kees Cook <kees@kernel.org> writes:
> 
> > On Thu, Jul 04, 2024 at 05:43:41PM +0300, Kalle Valo wrote:
> >> From: Kalle Valo <quic_kvalo@quicinc.com>
> >> 
> >> Johannes reported with GCC 11.4 there's a fortify warning below. The warning is
> >> not seen with GCC 12.1 nor 13.2. Weirdly moving the other operand of sum to the
> >> other side the warning goes away. This is safe to do as the value of the
> >> operand is check earlier. But the code looks worse with this so I'm not sure
> >> what to do.
> >
> > FWIW, this isn't fortify, but -Wrestrict.
> 
> Ah, thanks for correcting. I just saw fortify-string.h and made the
> wrong assumption.
> 
> > I would expect the same warnings even with CONFIG_FORTIFY_SOURCE
> > disabled. Regardless, it's worth figuring out what's going on. It
> > looks like this is GCC's value range tracker deciding it sees a way
> > for things to go weird.
> >
> > I suspect they fixed -Wrestrict in later GCC versions. It might need to
> > be version-limited...
> >
> >> In file included from ./include/linux/string.h:374,
> >>                  from ./include/linux/bitmap.h:13,
> >>                  from ./include/linux/cpumask.h:13,
> >>                  from ./include/linux/sched.h:16,
> >>                  from ./include/linux/delay.h:23,
> >>                  from drivers/net/wireless/ath/ath12k/wow.c:7:
> >> drivers/net/wireless/ath/ath12k/wow.c: In function
> >> ‘ath12k_wow_convert_8023_to_80211.constprop’:
> >> ./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’
> >> accessing 18446744073709551611 or more bytes at offsets 0 and 0
> >> overlaps 9223372036854775799 bytes at offset -9223372036854775804
> >> [-Werror=restrict]
> >
> > These huge negative values imply to me that GCC is looking at some
> > signed values somewhere.
> >
> >> [...]
> >> diff --git a/drivers/net/wireless/ath/ath12k/wow.c b/drivers/net/wireless/ath/ath12k/wow.c
> >> index c5cba825a84a..e9588bb7561c 100644
> >> --- a/drivers/net/wireless/ath/ath12k/wow.c
> >> +++ b/drivers/net/wireless/ath/ath12k/wow.c
> >> @@ -186,7 +186,7 @@ ath12k_wow_convert_8023_to_80211(struct ath12k *ar,
> >>  	if (eth_pkt_ofs < ETH_ALEN) {
> >>  		pkt_ofs = eth_pkt_ofs + a1_ofs;
> >>  
> >> -		if (eth_pkt_ofs + eth_pat_len < ETH_ALEN) {
> >> +		if (eth_pat_len < ETH_ALEN - eth_pkt_ofs) {
> >>  			memcpy(pat, eth_pat, eth_pat_len);
> >>  			memcpy(bytemask, eth_bytemask, eth_pat_len);
> >
> > Both eth_pkt_ofs and eth_pat_len are size_t. ETH_ALEN isn't, but it
> > would be promoted to size_t here. The value tracker should see that
> > eth_pkt_ofs could be [0..ETH_ALEN). eth_pat_len is coming from an "int",
> > though, so that might be the confusion. It may think eth_pat_len could
> > be [0..UINT_MAX] (i.e. the full range of int within size_t).
> >
> > So [0..ETH_ALEN) + [0..UINT_MAX] < 6 might be doing something wrong in
> > GCC 11.x, and it's not actually doing the size_t promotion correctly,
> > or deciding something has wrapped and then thinking eth_pat_len could
> > span a giant region of the address space, which freaks out -Wrestrict.
> > i.e. it's seeing that for the "if" to be true, eth_pat_len could be large
> > enough to wrap around the addition (though this shouldn't be possible
> > for 64-bit size_t).
> >
> > So I could see how [0..UINT_MAX] < 6 - [0..ETH_ALEN) would make it
> > happier: the right side is now [1..6], so eth_pat_len becomes [1..6).
> 
> Earlier I did some testing and I noticed that this if test also gives a
> warning:
> 
> 1 + eth_pat_len < ETH_ALEN
> 
> But this doesn't have any warning:
> 
> 0 + eth_pat_len < ETH_ALEN
> 
> And I stopped my investigation there :)
> 
> > Reviewed-by: Kees Cook <kees@kernel.org>
> 
> So you think this should be applied? It's not really logical so I would
> prefer to avoid taking it if possible. Or should we just ignore the
> warning? It only happens on GCC 11 anyway.

I suspect this won't be the only place in the kernel where -Wrestrict
will give weird results with GCC 11, and there are still plenty of folks
using GCC 11. I think the best option would probably be to version-check
GCC to gate the addition of -Wrestrict.

Arnd, what do you think? This looks like a more extreme version of
commit f9fc1ec28bae ("crypto: drivers - avoid memcpy size warning")

-Kees

-- 
Kees Cook

Re: [PATCH RFC] wifi: ath12k: workaround fortify warnings in ath12k_wow_convert_8023_to_80211()

[Arnd Bergmann]

On Mon, Jul 8, 2024, at 21:31, Kees Cook wrote:
> On Mon, Jul 08, 2024 at 06:51:52PM +0300, Kalle Valo wrote:
>
> I suspect this won't be the only place in the kernel where -Wrestrict
> will give weird results with GCC 11, and there are still plenty of folks
> using GCC 11. I think the best option would probably be to version-check
> GCC to gate the addition of -Wrestrict.
>
> Arnd, what do you think? This looks like a more extreme version of
> commit f9fc1ec28bae ("crypto: drivers - avoid memcpy size warning")

The f9fc1ec28bae patch was the other way around, it showed up
in new compilers but not old ones. I don't think I've seen
more gcc-11 -Wrestrict warnings during testing, but I'm currently
not set up to do a thorough search. If it's the only one, then
Kalle's suggested workaround is probably best, but if there
are additional warnings on gcc-11, making the warning depend
newer compilers is also fine. I just don't want to give up the
warning for new compilers altogether.

      Arnd

Re: [PATCH RFC] wifi: ath12k: workaround fortify warnings in ath12k_wow_convert_8023_to_80211()

[Kalle Valo]

"Arnd Bergmann" <arnd@arndb.de> writes:

> On Mon, Jul 8, 2024, at 21:31, Kees Cook wrote:
>> On Mon, Jul 08, 2024 at 06:51:52PM +0300, Kalle Valo wrote:
>>
>> I suspect this won't be the only place in the kernel where -Wrestrict
>> will give weird results with GCC 11, and there are still plenty of folks
>> using GCC 11. I think the best option would probably be to version-check
>> GCC to gate the addition of -Wrestrict.
>>
>> Arnd, what do you think? This looks like a more extreme version of
>> commit f9fc1ec28bae ("crypto: drivers - avoid memcpy size warning")
>
> The f9fc1ec28bae patch was the other way around, it showed up
> in new compilers but not old ones. I don't think I've seen
> more gcc-11 -Wrestrict warnings during testing, but I'm currently
> not set up to do a thorough search. If it's the only one, then
> Kalle's suggested workaround is probably best, but if there
> are additional warnings on gcc-11, making the warning depend
> newer compilers is also fine. 

Honestly I was hoping that we could disable the warning for GCC 11 :)

I feel bad making the code worse due to a compiler problem. For example,
Intel's zero day bot doesn't seem to use GCC 11 that much anymore, so it
might surprise more people than just us ath12k folks. (The bot said
everything was fine but Johannes saw the warning when the code was
pulled to wireless-next.)

> I just don't want to give up the warning for new compilers altogether.

Me neither. I'm just hoping that we could disable it for GCC 11. But of
course if you think it's better to add the workaround to ath12k, I can
submit a proper (non-RFC) patch to do that.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH RFC] wifi: ath12k: workaround fortify warnings in ath12k_wow_convert_8023_to_80211()

[Kalle Valo]

Kalle Valo <kvalo@kernel.org> writes:

> "Arnd Bergmann" <arnd@arndb.de> writes:
>
>> On Mon, Jul 8, 2024, at 21:31, Kees Cook wrote:
>>> On Mon, Jul 08, 2024 at 06:51:52PM +0300, Kalle Valo wrote:
>>>
>>> I suspect this won't be the only place in the kernel where -Wrestrict
>>> will give weird results with GCC 11, and there are still plenty of folks
>>> using GCC 11. I think the best option would probably be to version-check
>>> GCC to gate the addition of -Wrestrict.
>>>
>>> Arnd, what do you think? This looks like a more extreme version of
>>> commit f9fc1ec28bae ("crypto: drivers - avoid memcpy size warning")
>>
>> The f9fc1ec28bae patch was the other way around, it showed up
>> in new compilers but not old ones. I don't think I've seen
>> more gcc-11 -Wrestrict warnings during testing, but I'm currently
>> not set up to do a thorough search. If it's the only one, then
>> Kalle's suggested workaround is probably best, but if there
>> are additional warnings on gcc-11, making the warning depend
>> newer compilers is also fine. 
>
> Honestly I was hoping that we could disable the warning for GCC 11 :)
>
> I feel bad making the code worse due to a compiler problem. For example,
> Intel's zero day bot doesn't seem to use GCC 11 that much anymore, so it
> might surprise more people than just us ath12k folks. (The bot said
> everything was fine but Johannes saw the warning when the code was
> pulled to wireless-next.)
>
>> I just don't want to give up the warning for new compilers altogether.
>
> Me neither. I'm just hoping that we could disable it for GCC 11. But of
> course if you think it's better to add the workaround to ath12k, I can
> submit a proper (non-RFC) patch to do that.

For the archives: Paolo found a nicer way which is now commited:

https://git.kernel.org/linus/b49991d83bba

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH RFC] wifi: ath12k: workaround fortify warnings in ath12k_wow_convert_8023_to_80211()

[Paul E. McKenney]

On Thu, Jul 04, 2024 at 05:43:41PM +0300, Kalle Valo wrote:
> From: Kalle Valo <quic_kvalo@quicinc.com>
> 
> Johannes reported with GCC 11.4 there's a fortify warning below. The warning is
> not seen with GCC 12.1 nor 13.2. Weirdly moving the other operand of sum to the
> other side the warning goes away. This is safe to do as the value of the
> operand is check earlier. But the code looks worse with this so I'm not sure
> what to do.
> 
> In file included from ./include/linux/string.h:374,
>                  from ./include/linux/bitmap.h:13,
>                  from ./include/linux/cpumask.h:13,
>                  from ./include/linux/sched.h:16,
>                  from ./include/linux/delay.h:23,
>                  from drivers/net/wireless/ath/ath12k/wow.c:7:
> drivers/net/wireless/ath/ath12k/wow.c: In function ‘ath12k_wow_convert_8023_to_80211.constprop’:
> ./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ accessing 18446744073709551611 or more bytes at offsets 0 and 0 overlaps 9223372036854775799 bytes at offset -9223372036854775804 [-Werror=restrict]
>   114 | #define __underlying_memcpy     __builtin_memcpy
>       |                                 ^
> ./include/linux/fortify-string.h:637:9: note: in expansion of macro ‘__underlying_memcpy’
>   637 |         __underlying_##op(p, q, __fortify_size);                              |         ^~~~~~~~~~~~~
> ./include/linux/fortify-string.h:682:26: note: in expansion of macro ‘__fortify_memcpy_chk’
>   682 | #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                        |                          ^~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath12k/wow.c:190:25: note: in expansion of macro ‘memcpy’
>   190 |                         memcpy(pat, eth_pat, eth_pat_len);
>       |                         ^~~~~~
> ./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ accessing 18446744073709551605 or more bytes at offsets 0 and 0 overlaps 9223372036854775787 bytes at offset -9223372036854775798 [-Werror=restrict]
>   114 | #define __underlying_memcpy     __builtin_memcpy
>       |                                 ^
> ./include/linux/fortify-string.h:637:9: note: in expansion of macro ‘__underlying_memcpy’
>   637 |         __underlying_##op(p, q, __fortify_size);                              |         ^~~~~~~~~~~~~
> ./include/linux/fortify-string.h:682:26: note: in expansion of macro ‘__fortify_memcpy_chk’
>   682 | #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                        |                          ^~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath12k/wow.c:232:25: note: in expansion of macro ‘memcpy’
>   232 |                         memcpy(pat, eth_pat, eth_pat_len);
>       |                         ^~~~~~
> 
> Compile tested only.
> 
> Reported-by: Johannes Berg <johannes@sipsolutions.net>
> Fixes: 4a3c212eee0e ("wifi: ath12k: add basic WoW functionalities")
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

Reviewed-by: Paul E. McKenney <paulmck@kernel.org>

(Also compile retested, for whatever that is worth.)

This still shows up in allmodconfig next-20240709 builds.

						Thanx, Paul

> ---
>  drivers/net/wireless/ath/ath12k/wow.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/wow.c b/drivers/net/wireless/ath/ath12k/wow.c
> index c5cba825a84a..e9588bb7561c 100644
> --- a/drivers/net/wireless/ath/ath12k/wow.c
> +++ b/drivers/net/wireless/ath/ath12k/wow.c
> @@ -186,7 +186,7 @@ ath12k_wow_convert_8023_to_80211(struct ath12k *ar,
>  	if (eth_pkt_ofs < ETH_ALEN) {
>  		pkt_ofs = eth_pkt_ofs + a1_ofs;
>  
> -		if (eth_pkt_ofs + eth_pat_len < ETH_ALEN) {
> +		if (eth_pat_len < ETH_ALEN - eth_pkt_ofs) {
>  			memcpy(pat, eth_pat, eth_pat_len);
>  			memcpy(bytemask, eth_bytemask, eth_pat_len);
>  
> @@ -228,7 +228,7 @@ ath12k_wow_convert_8023_to_80211(struct ath12k *ar,
>  	} else if (eth_pkt_ofs < prot_ofs) {
>  		pkt_ofs = eth_pkt_ofs - ETH_ALEN + a3_ofs;
>  
> -		if (eth_pkt_ofs + eth_pat_len < prot_ofs) {
> +		if (eth_pat_len < prot_ofs - eth_pkt_ofs) {
>  			memcpy(pat, eth_pat, eth_pat_len);
>  			memcpy(bytemask, eth_bytemask, eth_pat_len);
>  
> 
> base-commit: c1cacb01f35589bd41360cdb7535afc792c08a7c
> -- 
> 2.39.2
>