| Message ID | 20200509120707.188595-2-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [net-next,1/2] ath10k: fix gcc-10 zero-length-bounds warnings | expand |
Arnd Bergmann <arnd@arndb.de> writes: > gcc-10 correctly points out a bug with a zero-length array in > struct ath10k_pci: > > drivers/net/wireless/ath/ath10k/ahb.c: In function 'ath10k_ahb_remove': > drivers/net/wireless/ath/ath10k/ahb.c:30:9: error: array subscript 0 > is outside the bounds of an interior zero-length array 'struct > ath10k_ahb[0]' [-Werror=zero-length-bounds] > 30 | return &((struct ath10k_pci *)ar->drv_priv)->ahb[0]; > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > In file included from drivers/net/wireless/ath/ath10k/ahb.c:13: > drivers/net/wireless/ath/ath10k/pci.h:185:20: note: while referencing 'ahb' > 185 | struct ath10k_ahb ahb[0]; > | ^~~ > > The last addition to the struct ignored the comments and added > new members behind the array that must remain last. > > Change it to a flexible-array member and move it last again to > make it work correctly, prevent the same thing from happening > again (all compilers warn about flexible-array members in the > middle of a struct) and get it to build without warnings. Very good find, thanks! This bug would cause all sort of strange memory corruption issues.
Kalle Valo <kvalo@codeaurora.org> writes: > Arnd Bergmann <arnd@arndb.de> writes: > >> gcc-10 correctly points out a bug with a zero-length array in >> struct ath10k_pci: >> >> drivers/net/wireless/ath/ath10k/ahb.c: In function 'ath10k_ahb_remove': >> drivers/net/wireless/ath/ath10k/ahb.c:30:9: error: array subscript 0 >> is outside the bounds of an interior zero-length array 'struct >> ath10k_ahb[0]' [-Werror=zero-length-bounds] >> 30 | return &((struct ath10k_pci *)ar->drv_priv)->ahb[0]; >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> In file included from drivers/net/wireless/ath/ath10k/ahb.c:13: >> drivers/net/wireless/ath/ath10k/pci.h:185:20: note: while referencing 'ahb' >> 185 | struct ath10k_ahb ahb[0]; >> | ^~~ >> >> The last addition to the struct ignored the comments and added >> new members behind the array that must remain last. >> >> Change it to a flexible-array member and move it last again to >> make it work correctly, prevent the same thing from happening >> again (all compilers warn about flexible-array members in the >> middle of a struct) and get it to build without warnings. > > Very good find, thanks! This bug would cause all sort of strange memory > corruption issues. This motivated me to switch to using GCC 10.x and I noticed that you had already upgraded crosstool so it was a trivial thing to do, awesome :) https://mirrors.edge.kernel.org/pub/tools/crosstool/ I use crosstool like this using GNUmakefile: CROSS_COMPILE=/opt/cross/gcc-10.1.0-nolibc/x86_64-linux/bin/x86_64-linux- include Makefile I think it's handy trick and would be good to mention that in the crosstool main page. That way I could just point people to the crosstool main page when they are using ancient compilers and would need to upgrade.
On Mon, May 11, 2020 at 2:17 PM Kalle Valo <kvalo@codeaurora.org> wrote: > > Kalle Valo <kvalo@codeaurora.org> writes: > >> > >> Change it to a flexible-array member and move it last again to > >> make it work correctly, prevent the same thing from happening > >> again (all compilers warn about flexible-array members in the > >> middle of a struct) and get it to build without warnings. > > > > Very good find, thanks! This bug would cause all sort of strange memory > > corruption issues. > > This motivated me to switch to using GCC 10.x and I noticed that you had > already upgraded crosstool so it was a trivial thing to do, awesome :) > > https://mirrors.edge.kernel.org/pub/tools/crosstool/ > > I use crosstool like this using GNUmakefile: > > CROSS_COMPILE=/opt/cross/gcc-10.1.0-nolibc/x86_64-linux/bin/x86_64-linux- > include Makefile Right, I have something similar (with many more additional things) in a local makefile here. I mainly use that to pick the correct cross toolchain based on ${ARCH}, and to build multiple randconfig kernels in parallel with 'make -j${NR_CPUS}' for better CPU utilization. > I think it's handy trick and would be good to mention that in the > crosstool main page. That way I could just point people to the crosstool > main page when they are using ancient compilers and would need to > upgrade. I actually started working on a script that I'd like to include the kernel sources to list the installed compilers, automatically pick on that works for the current architecture, or download one for local installation. Arnd
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h index e3cbd259a2dc..862d0901c5b8 100644 --- a/drivers/net/wireless/ath/ath10k/pci.h +++ b/drivers/net/wireless/ath/ath10k/pci.h @@ -178,15 +178,16 @@ struct ath10k_pci { */ u32 (*targ_cpu_to_ce_addr)(struct ath10k *ar, u32 addr); + struct ce_attr *attr; + struct ce_pipe_config *pipe_config; + struct ce_service_to_pipe *serv_to_pipe; + /* Keep this entry in the last, memory for struct ath10k_ahb is * allocated (ahb support enabled case) in the continuation of * this struct. */ - struct ath10k_ahb ahb[0]; + struct ath10k_ahb ahb[]; - struct ce_attr *attr; - struct ce_pipe_config *pipe_config; - struct ce_service_to_pipe *serv_to_pipe; }; static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
gcc-10 correctly points out a bug with a zero-length array in struct ath10k_pci: drivers/net/wireless/ath/ath10k/ahb.c: In function 'ath10k_ahb_remove': drivers/net/wireless/ath/ath10k/ahb.c:30:9: error: array subscript 0 is outside the bounds of an interior zero-length array 'struct ath10k_ahb[0]' [-Werror=zero-length-bounds] 30 | return &((struct ath10k_pci *)ar->drv_priv)->ahb[0]; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from drivers/net/wireless/ath/ath10k/ahb.c:13: drivers/net/wireless/ath/ath10k/pci.h:185:20: note: while referencing 'ahb' 185 | struct ath10k_ahb ahb[0]; | ^~~ The last addition to the struct ignored the comments and added new members behind the array that must remain last. Change it to a flexible-array member and move it last again to make it work correctly, prevent the same thing from happening again (all compilers warn about flexible-array members in the middle of a struct) and get it to build without warnings. Fixes: 521fc37be3d8 ("ath10k: Avoid override CE5 configuration for QCA99X0 chipsets") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/net/wireless/ath/ath10k/pci.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)