2021-03-22 16:04:58

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 00/11] treewide: address gcc-11 -Wstringop-overread warnings

From: Arnd Bergmann <[email protected]>

The coming gcc release introduces a new warning for string operations
reading beyond the end of a fixed-length object. After testing
randconfig kernels for a while, think I have patches for any such
warnings that came up on x86, arm and arm64.

Most of these warnings are false-positive ones, either gcc warning
about something that is entirely correct, or about something that
looks suspicious but turns out to be correct after all.

The two patches for the i915 driver look like something that might
be actual bugs, but I am not sure about those either.

We probably want some combination of workaround like the ones I
post here and changes to gcc to have fewer false positives in the
release. I'm posting the entire set of workaround that give me
a cleanly building kernel for reference here.

Arnd

Arnd Bergmann (11):
x86: compressed: avoid gcc-11 -Wstringop-overread warning
x86: tboot: avoid Wstringop-overread-warning
security: commoncap: fix -Wstringop-overread warning
ath11: Wstringop-overread warning
qnx: avoid -Wstringop-overread warning
cgroup: fix -Wzero-length-bounds warnings
ARM: sharpsl_param: work around -Wstringop-overread warning
atmel: avoid gcc -Wstringop-overflow warning
scsi: lpfc: fix gcc -Wstringop-overread warning
drm/i915: avoid stringop-overread warning on pri_latency
[RFC] drm/i915/dp: fix array overflow warning

arch/arm/common/sharpsl_param.c | 4 ++-
arch/x86/boot/compressed/misc.c | 2 --
arch/x86/kernel/tboot.c | 44 +++++++++++++++----------
drivers/gpu/drm/i915/display/intel_dp.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 6 ++--
drivers/net/wireless/ath/ath11k/mac.c | 2 +-
drivers/net/wireless/atmel/atmel.c | 25 ++++++++------
drivers/scsi/lpfc/lpfc_attr.c | 6 ++--
fs/qnx4/dir.c | 11 +++----
kernel/cgroup/cgroup.c | 15 +++++++--
security/commoncap.c | 2 +-
11 files changed, 69 insertions(+), 50 deletions(-)

Cc: [email protected]
Cc: Ning Sun <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Kalle Valo <[email protected]>
Cc: Simon Kelley <[email protected]>
Cc: James Smart <[email protected]>
Cc: "James E.J. Bottomley" <[email protected]>
Cc: Anders Larsen <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: Serge Hallyn <[email protected]>
Cc: Imre Deak <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]


--
2.29.2


2021-03-22 16:05:05

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 01/11] x86: compressed: avoid gcc-11 -Wstringop-overread warning

From: Arnd Bergmann <[email protected]>

gcc gets confused by the comparison of a pointer to an integer listeral,
with the assumption that this is an offset from a NULL pointer and that
dereferencing it is invalid:

In file included from arch/x86/boot/compressed/misc.c:18:
In function ‘parse_elf’,
inlined from ‘extract_kernel’ at arch/x86/boot/compressed/misc.c:442:2:
arch/x86/boot/compressed/../string.h:15:23: error: ‘__builtin_memcpy’ reading 64 bytes from a region of size 0 [-Werror=stringop-overread]
15 | #define memcpy(d,s,l) __builtin_memcpy(d,s,l)
| ^~~~~~~~~~~~~~~~~~~~~~~
arch/x86/boot/compressed/misc.c:283:9: note: in expansion of macro ‘memcpy’
283 | memcpy(&ehdr, output, sizeof(ehdr));
| ^~~~~~

I could not find any good workaround for this, but as this is only
a warning for a failure during early boot, removing the line entirely
works around the warning.

This should probably get addressed in gcc instead, before 11.1 gets
released.

Signed-off-by: Arnd Bergmann <[email protected]>
---
arch/x86/boot/compressed/misc.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 3a214cc3239f..9ada64e66cb7 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -430,8 +430,6 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
error("Destination address too large");
#endif
#ifndef CONFIG_RELOCATABLE
- if ((unsigned long)output != LOAD_PHYSICAL_ADDR)
- error("Destination address does not match LOAD_PHYSICAL_ADDR");
if (virt_addr != LOAD_PHYSICAL_ADDR)
error("Destination virtual address changed when not relocatable");
#endif
--
2.29.2

2021-03-22 16:06:06

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 05/11] qnx: avoid -Wstringop-overread warning

From: Arnd Bergmann <[email protected]>

gcc-11 warns that the size of the link name is longer than the di_fname
field:

fs/qnx4/dir.c: In function ‘qnx4_readdir’:
fs/qnx4/dir.c:51:32: error: ‘strnlen’ specified bound 48 exceeds source size 16 [-Werror=stringop-overread]
51 | size = strnlen(de->di_fname, size);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from fs/qnx4/qnx4.h:3,
from fs/qnx4/dir.c:16:
include/uapi/linux/qnx4_fs.h:45:25: note: source object declared here
45 | char di_fname[QNX4_SHORT_NAME_MAX];

The problem here is that we access the same pointer using two different
structure layouts, but gcc determines the object size based on
whatever it encounters first.

Change the strnlen to use the correct field size in each case, and
change the first access to be on the longer field.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
Signed-off-by: Arnd Bergmann <[email protected]>
---
fs/qnx4/dir.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/qnx4/dir.c b/fs/qnx4/dir.c
index a6ee23aadd28..68046450e543 100644
--- a/fs/qnx4/dir.c
+++ b/fs/qnx4/dir.c
@@ -39,21 +39,20 @@ static int qnx4_readdir(struct file *file, struct dir_context *ctx)
ix = (ctx->pos >> QNX4_DIR_ENTRY_SIZE_BITS) % QNX4_INODES_PER_BLOCK;
for (; ix < QNX4_INODES_PER_BLOCK; ix++, ctx->pos += QNX4_DIR_ENTRY_SIZE) {
offset = ix * QNX4_DIR_ENTRY_SIZE;
- de = (struct qnx4_inode_entry *) (bh->b_data + offset);
- if (!de->di_fname[0])
+ le = (struct qnx4_link_info *)(bh->b_data + offset);
+ de = (struct qnx4_inode_entry *)(bh->b_data + offset);
+ if (!le->dl_fname[0])
continue;
if (!(de->di_status & (QNX4_FILE_USED|QNX4_FILE_LINK)))
continue;
if (!(de->di_status & QNX4_FILE_LINK))
- size = QNX4_SHORT_NAME_MAX;
+ size = strnlen(de->di_fname, sizeof(de->di_fname));
else
- size = QNX4_NAME_MAX;
- size = strnlen(de->di_fname, size);
+ size = strnlen(le->dl_fname, sizeof(le->dl_fname));
QNX4DEBUG((KERN_INFO "qnx4_readdir:%.*s\n", size, de->di_fname));
if (!(de->di_status & QNX4_FILE_LINK))
ino = blknum * QNX4_INODES_PER_BLOCK + ix - 1;
else {
- le = (struct qnx4_link_info*)de;
ino = ( le32_to_cpu(le->dl_inode_blk) - 1 ) *
QNX4_INODES_PER_BLOCK +
le->dl_inode_ndx;
--
2.29.2

2021-03-22 16:06:11

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 04/11] ath11: Wstringop-overread warning

From: Arnd Bergmann <[email protected]>

gcc-11 with the kernel address sanitizer prints a warning for this
driver:

In function 'ath11k_peer_assoc_h_vht',
inlined from 'ath11k_peer_assoc_prepare' at drivers/net/wireless/ath/ath11k/mac.c:1632:2:
drivers/net/wireless/ath/ath11k/mac.c:1164:13: error: 'ath11k_peer_assoc_h_vht_masked' reading 16 bytes from a region of size 4 [-Werror=stringop-overread]
1164 | if (ath11k_peer_assoc_h_vht_masked(vht_mcs_mask))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireless/ath/ath11k/mac.c: In function 'ath11k_peer_assoc_prepare':
drivers/net/wireless/ath/ath11k/mac.c:1164:13: note: referencing argument 1 of type 'const u16 *' {aka 'const short unsigned int *'}
drivers/net/wireless/ath/ath11k/mac.c:969:1: note: in a call to function 'ath11k_peer_assoc_h_vht_masked'
969 | ath11k_peer_assoc_h_vht_masked(const u16 vht_mcs_mask[NL80211_VHT_NSS_MAX])
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

According to analysis from gcc developers, this is a glitch in the
way gcc tracks the size of struct members. This should really get
fixed in gcc, but it's also easy to work around this instance
by changing the function prototype to no include the length of
the array.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99673
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/wireless/ath/ath11k/mac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index b391169576e2..5cb7ed53f3c4 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -966,7 +966,7 @@ ath11k_peer_assoc_h_ht_masked(const u8 ht_mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
}

static bool
-ath11k_peer_assoc_h_vht_masked(const u16 vht_mcs_mask[NL80211_VHT_NSS_MAX])
+ath11k_peer_assoc_h_vht_masked(const u16 vht_mcs_mask[])
{
int nss;

--
2.29.2

2021-03-22 16:07:19

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 08/11] atmel: avoid gcc -Wstringop-overflow warning

From: Arnd Bergmann <[email protected]>

gcc-11 notices that the fields as defined in the ass_req_format
structure do not match the actual use of that structure:

cc1: error: writing 4 bytes into a region of size between 18446744073709551613 and 2 [-Werror=stringop-overflow=]
drivers/net/wireless/atmel/atmel.c:2884:20: note: at offset [4, 6] into destination object ‘ap’ of size 6
2884 | u8 ap[ETH_ALEN]; /* nothing after here directly accessible */
| ^~
drivers/net/wireless/atmel/atmel.c:2885:20: note: at offset [4, 6] into destination object ‘ssid_el_id’ of size 1
2885 | u8 ssid_el_id;
| ^~~~~~~~~~

This is expected here because the actual structure layout is variable.
As the code does not actually access the individual fields, replace
them with a comment and fixed-length array so prevent gcc from
complaining about it.

Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/wireless/atmel/atmel.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/atmel/atmel.c b/drivers/net/wireless/atmel/atmel.c
index 707fe66727f8..ff9152d600e1 100644
--- a/drivers/net/wireless/atmel/atmel.c
+++ b/drivers/net/wireless/atmel/atmel.c
@@ -2881,13 +2881,18 @@ static void send_association_request(struct atmel_private *priv, int is_reassoc)
struct ass_req_format {
__le16 capability;
__le16 listen_interval;
- u8 ap[ETH_ALEN]; /* nothing after here directly accessible */
- u8 ssid_el_id;
- u8 ssid_len;
- u8 ssid[MAX_SSID_LENGTH];
- u8 sup_rates_el_id;
- u8 sup_rates_len;
- u8 rates[4];
+ u8 ssid_el[ETH_ALEN + 2 + MAX_SSID_LENGTH + 2 + 4];
+ /*
+ * nothing after here directly accessible:
+ *
+ * u8 ap[ETH_ALEN];
+ * u8 ssid_el_id;
+ * u8 ssid_len;
+ * u8 ssid[MAX_SSID_LENGTH];
+ * u8 sup_rates_el_id;
+ * u8 sup_rates_len;
+ * u8 rates[4];
+ */
} body;

header.frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT |
@@ -2907,13 +2912,13 @@ static void send_association_request(struct atmel_private *priv, int is_reassoc)

body.listen_interval = cpu_to_le16(priv->listen_interval * priv->beacon_period);

+ ssid_el_p = body.ssid_el;
/* current AP address - only in reassoc frame */
if (is_reassoc) {
- memcpy(body.ap, priv->CurrentBSSID, ETH_ALEN);
- ssid_el_p = &body.ssid_el_id;
+ memcpy(ssid_el_p, priv->CurrentBSSID, ETH_ALEN);
+ ssid_el_p += ETH_ALEN;
bodysize = 18 + priv->SSID_size;
} else {
- ssid_el_p = &body.ap[0];
bodysize = 12 + priv->SSID_size;
}

--
2.29.2

2021-03-22 16:08:26

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 10/11] drm/i915: avoid stringop-overread warning on pri_latency

From: Arnd Bergmann <[email protected]>

gcc-11 warns about what appears to be an out-of-range array access:

In function ‘snb_wm_latency_quirk’,
inlined from ‘ilk_setup_wm_latency’ at drivers/gpu/drm/i915/intel_pm.c:3108:3:
drivers/gpu/drm/i915/intel_pm.c:3057:9: error: ‘intel_print_wm_latency’ reading 16 bytes from a region of size 10 [-Werror=stringop-overread]
3057 | intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/intel_pm.c: In function ‘ilk_setup_wm_latency’:
drivers/gpu/drm/i915/intel_pm.c:3057:9: note: referencing argument 3 of type ‘const u16 *’ {aka ‘const short unsigned int *’}
drivers/gpu/drm/i915/intel_pm.c:2994:13: note: in a call to function ‘intel_print_wm_latency’
2994 | static void intel_print_wm_latency(struct drm_i915_private *dev_priv,
| ^~~~~~~~~~~~~~~~~~~~~~

My guess is that this code is actually safe because the size of the
array depends on the hardware generation, and the function checks for
that, but at the same time I would not expect the compiler to work it
out correctly, and the code seems a little fragile with regards to
future changes. Simply increasing the size of the array should help.

Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/gpu/drm/i915/i915_drv.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 26d69d06aa6d..3567602e0a35 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1095,11 +1095,11 @@ struct drm_i915_private {
* in 0.5us units for WM1+.
*/
/* primary */
- u16 pri_latency[5];
+ u16 pri_latency[8];
/* sprite */
- u16 spr_latency[5];
+ u16 spr_latency[8];
/* cursor */
- u16 cur_latency[5];
+ u16 cur_latency[8];
/*
* Raw watermark memory latency values
* for SKL for all 8 levels
--
2.29.2

2021-03-22 16:08:34

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 09/11] scsi: lpfc: fix gcc -Wstringop-overread warning

From: Arnd Bergmann <[email protected]>

gcc-11 warns about an strnlen with a length larger than the size of the
passed buffer:

drivers/scsi/lpfc/lpfc_attr.c: In function 'lpfc_nvme_info_show':
drivers/scsi/lpfc/lpfc_attr.c:518:25: error: 'strnlen' specified bound 4095 exceeds source size 24 [-Werror=stringop-overread]
518 | strnlen(LPFC_NVME_INFO_MORE_STR, PAGE_SIZE - 1)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In this case, the code is entirely valid, as the string is properly
terminated, and the size argument is only there out of extra caution
in case it exceeds a page.

This cannot really happen here, so just simplify it to a sizeof().

Fixes: afff0d2321ea ("scsi: lpfc: Add Buffer overflow check, when nvme_info larger than PAGE_SIZE")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/scsi/lpfc/lpfc_attr.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index bdd9a29f4201..f6d886f9dfb3 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -512,11 +512,9 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
"6314 Catching potential buffer "
"overflow > PAGE_SIZE = %lu bytes\n",
PAGE_SIZE);
- strlcpy(buf + PAGE_SIZE - 1 -
- strnlen(LPFC_NVME_INFO_MORE_STR, PAGE_SIZE - 1),
+ strlcpy(buf + PAGE_SIZE - 1 - sizeof(LPFC_NVME_INFO_MORE_STR),
LPFC_NVME_INFO_MORE_STR,
- strnlen(LPFC_NVME_INFO_MORE_STR, PAGE_SIZE - 1)
- + 1);
+ sizeof(LPFC_NVME_INFO_MORE_STR) + 1);
}

return len;
--
2.29.2

2021-03-24 15:32:33

by Jani Nikula

[permalink] [raw]
Subject: Re: [PATCH 10/11] drm/i915: avoid stringop-overread warning on pri_latency

On Mon, 22 Mar 2021, Arnd Bergmann <[email protected]> wrote:
> From: Arnd Bergmann <[email protected]>
>
> gcc-11 warns about what appears to be an out-of-range array access:
>
> In function ‘snb_wm_latency_quirk’,
> inlined from ‘ilk_setup_wm_latency’ at drivers/gpu/drm/i915/intel_pm.c:3108:3:
> drivers/gpu/drm/i915/intel_pm.c:3057:9: error: ‘intel_print_wm_latency’ reading 16 bytes from a region of size 10 [-Werror=stringop-overread]
> 3057 | intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/i915/intel_pm.c: In function ‘ilk_setup_wm_latency’:
> drivers/gpu/drm/i915/intel_pm.c:3057:9: note: referencing argument 3 of type ‘const u16 *’ {aka ‘const short unsigned int *’}
> drivers/gpu/drm/i915/intel_pm.c:2994:13: note: in a call to function ‘intel_print_wm_latency’
> 2994 | static void intel_print_wm_latency(struct drm_i915_private *dev_priv,
> | ^~~~~~~~~~~~~~~~~~~~~~
>
> My guess is that this code is actually safe because the size of the
> array depends on the hardware generation, and the function checks for
> that, but at the same time I would not expect the compiler to work it
> out correctly, and the code seems a little fragile with regards to
> future changes. Simply increasing the size of the array should help.

Agreed, I don't think there's an issue, but the code could use a bunch
of improvements.

Like, we have intel_print_wm_latency() for debug logging and
wm_latency_show() for debugfs, and there's a bunch of duplication and
ugh.

But this seems like the easiest fix for the warning.

Reviewed-by: Jani Nikula <[email protected]>


> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 26d69d06aa6d..3567602e0a35 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1095,11 +1095,11 @@ struct drm_i915_private {
> * in 0.5us units for WM1+.
> */
> /* primary */
> - u16 pri_latency[5];
> + u16 pri_latency[8];
> /* sprite */
> - u16 spr_latency[5];
> + u16 spr_latency[8];
> /* cursor */
> - u16 cur_latency[5];
> + u16 cur_latency[8];
> /*
> * Raw watermark memory latency values
> * for SKL for all 8 levels

--
Jani Nikula, Intel Open Source Graphics Center

2021-03-24 17:24:05

by Ville Syrjälä

[permalink] [raw]
Subject: Re: [PATCH 10/11] drm/i915: avoid stringop-overread warning on pri_latency

On Wed, Mar 24, 2021 at 05:30:24PM +0200, Jani Nikula wrote:
> On Mon, 22 Mar 2021, Arnd Bergmann <[email protected]> wrote:
> > From: Arnd Bergmann <[email protected]>
> >
> > gcc-11 warns about what appears to be an out-of-range array access:
> >
> > In function ‘snb_wm_latency_quirk’,
> > inlined from ‘ilk_setup_wm_latency’ at drivers/gpu/drm/i915/intel_pm.c:3108:3:
> > drivers/gpu/drm/i915/intel_pm.c:3057:9: error: ‘intel_print_wm_latency’ reading 16 bytes from a region of size 10 [-Werror=stringop-overread]
> > 3057 | intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency);
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/gpu/drm/i915/intel_pm.c: In function ‘ilk_setup_wm_latency’:
> > drivers/gpu/drm/i915/intel_pm.c:3057:9: note: referencing argument 3 of type ‘const u16 *’ {aka ‘const short unsigned int *’}
> > drivers/gpu/drm/i915/intel_pm.c:2994:13: note: in a call to function ‘intel_print_wm_latency’
> > 2994 | static void intel_print_wm_latency(struct drm_i915_private *dev_priv,
> > | ^~~~~~~~~~~~~~~~~~~~~~
> >
> > My guess is that this code is actually safe because the size of the
> > array depends on the hardware generation, and the function checks for
> > that, but at the same time I would not expect the compiler to work it
> > out correctly, and the code seems a little fragile with regards to
> > future changes. Simply increasing the size of the array should help.
>
> Agreed, I don't think there's an issue, but the code could use a bunch
> of improvements.
>
> Like, we have intel_print_wm_latency() for debug logging and
> wm_latency_show() for debugfs, and there's a bunch of duplication and
> ugh.

There is all this ancient stuff in review limbo...
https://patchwork.freedesktop.org/series/50802/

--
Ville Syrjälä
Intel

2021-04-06 13:17:12

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH 00/11] treewide: address gcc-11 -Wstringop-overread warnings

On Mon, 22 Mar 2021 17:02:38 +0100, Arnd Bergmann wrote:

> The coming gcc release introduces a new warning for string operations
> reading beyond the end of a fixed-length object. After testing
> randconfig kernels for a while, think I have patches for any such
> warnings that came up on x86, arm and arm64.
>
> Most of these warnings are false-positive ones, either gcc warning
> about something that is entirely correct, or about something that
> looks suspicious but turns out to be correct after all.
>
> [...]

Applied to 5.13/scsi-queue, thanks!

[09/11] scsi: lpfc: fix gcc -Wstringop-overread warning
https://git.kernel.org/mkp/scsi/c/ada48ba70f6b

--
Martin K. Petersen Oracle Linux Engineering

2021-09-28 09:05:42

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 04/11] ath11: Wstringop-overread warning

Arnd Bergmann <[email protected]> wrote:

> gcc-11 with the kernel address sanitizer prints a warning for this
> driver:
>
> In function 'ath11k_peer_assoc_h_vht',
> inlined from 'ath11k_peer_assoc_prepare' at drivers/net/wireless/ath/ath11k/mac.c:1632:2:
> drivers/net/wireless/ath/ath11k/mac.c:1164:13: error: 'ath11k_peer_assoc_h_vht_masked' reading 16 bytes from a region of size 4 [-Werror=stringop-overread]
> 1164 | if (ath11k_peer_assoc_h_vht_masked(vht_mcs_mask))
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath11k/mac.c: In function 'ath11k_peer_assoc_prepare':
> drivers/net/wireless/ath/ath11k/mac.c:1164:13: note: referencing argument 1 of type 'const u16 *' {aka 'const short unsigned int *'}
> drivers/net/wireless/ath/ath11k/mac.c:969:1: note: in a call to function 'ath11k_peer_assoc_h_vht_masked'
> 969 | ath11k_peer_assoc_h_vht_masked(const u16 vht_mcs_mask[NL80211_VHT_NSS_MAX])
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> According to analysis from gcc developers, this is a glitch in the
> way gcc tracks the size of struct members. This should really get
> fixed in gcc, but it's also easy to work around this instance
> by changing the function prototype to no include the length of
> the array.
>
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99673
> Signed-off-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

eb19efed836a ath11k: Wstringop-overread warning

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

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