[PATCH net-next 1/2] ath10k: fix gcc-10 zero-length-bounds warnings

[PATCH net-next 1/2] ath10k: fix gcc-10 zero-length-bounds warnings

[Arnd Bergmann]

gcc-10 started warning about out-of-bounds access for zero-length
arrays:

In file included from drivers/net/wireless/ath/ath10k/core.h:18,
                 from drivers/net/wireless/ath/ath10k/htt_rx.c:8:
drivers/net/wireless/ath/ath10k/htt_rx.c: In function 'ath10k_htt_rx_tx_fetch_ind':
drivers/net/wireless/ath/ath10k/htt.h:1683:17: warning: array subscript 65535 is outside the bounds of an interior zero-length array 'struct htt_tx_fetch_record[0]' [-Wzero-length-bounds]
 1683 |  return (void *)&ind->records[le16_to_cpu(ind->num_records)];
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireless/ath/ath10k/htt.h:1676:29: note: while referencing 'records'
 1676 |  struct htt_tx_fetch_record records[0];
      |                             ^~~~~~~

Make records[] a flexible array member to allow this, moving it behind
the other zero-length member that is not accessed in a way that gcc
warns about.

Fixes: 3ba225b506a2 ("treewide: Replace zero-length array with flexible-array member")
Fixes: 22e6b3bc5d96 ("ath10k: add new htt definitions")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/wireless/ath/ath10k/htt.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 8f3710cf28f4..aa056a186402 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1673,8 +1673,8 @@ struct htt_tx_fetch_ind {
 	__le32 token;
 	__le16 num_resp_ids;
 	__le16 num_records;
-	struct htt_tx_fetch_record records[0];
 	__le32 resp_ids[0]; /* ath10k_htt_get_tx_fetch_ind_resp_ids() */
+	struct htt_tx_fetch_record records[];
 } __packed;
 
 static inline void *
-- 
2.26.0

[PATCH net-next 2/2] ath10k: fix ath10k_pci struct layout

[Arnd Bergmann]

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

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

Re: [PATCH net-next 1/2] ath10k: fix gcc-10 zero-length-bounds warnings

[Gustavo A. R. Silva]

Arnd,

On Sat, May 09, 2020 at 02:06:32PM +0200, Arnd Bergmann wrote:
> gcc-10 started warning about out-of-bounds access for zero-length
> arrays:
> 
> In file included from drivers/net/wireless/ath/ath10k/core.h:18,
>                  from drivers/net/wireless/ath/ath10k/htt_rx.c:8:
> drivers/net/wireless/ath/ath10k/htt_rx.c: In function 'ath10k_htt_rx_tx_fetch_ind':
> drivers/net/wireless/ath/ath10k/htt.h:1683:17: warning: array subscript 65535 is outside the bounds of an interior zero-length array 'struct htt_tx_fetch_record[0]' [-Wzero-length-bounds]
>  1683 |  return (void *)&ind->records[le16_to_cpu(ind->num_records)];
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath10k/htt.h:1676:29: note: while referencing 'records'
>  1676 |  struct htt_tx_fetch_record records[0];
>       |                             ^~~~~~~
> 
> Make records[] a flexible array member to allow this, moving it behind
> the other zero-length member that is not accessed in a way that gcc
> warns about.
> 
> Fixes: 3ba225b506a2 ("treewide: Replace zero-length array with flexible-array member")

This treewide patch no longer contains changes for ath10k. I removed them
since Monday (05/04/2020). So, this "Fixes" tag does not apply.

Thanks
--
Gustavo

> Fixes: 22e6b3bc5d96 ("ath10k: add new htt definitions")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/wireless/ath/ath10k/htt.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
> index 8f3710cf28f4..aa056a186402 100644
> --- a/drivers/net/wireless/ath/ath10k/htt.h
> +++ b/drivers/net/wireless/ath/ath10k/htt.h
> @@ -1673,8 +1673,8 @@ struct htt_tx_fetch_ind {
>  	__le32 token;
>  	__le16 num_resp_ids;
>  	__le16 num_records;
> -	struct htt_tx_fetch_record records[0];
>  	__le32 resp_ids[0]; /* ath10k_htt_get_tx_fetch_ind_resp_ids() */
> +	struct htt_tx_fetch_record records[];
>  } __packed;
>  
>  static inline void *
> -- 
> 2.26.0
> 

Re: [PATCH net-next 1/2] ath10k: fix gcc-10 zero-length-bounds warnings

[Kalle Valo]

"Gustavo A. R. Silva" <gustavoars@kernel.org> writes:

> Arnd,
>
> On Sat, May 09, 2020 at 02:06:32PM +0200, Arnd Bergmann wrote:
>> gcc-10 started warning about out-of-bounds access for zero-length
>> arrays:
>> 
>> In file included from drivers/net/wireless/ath/ath10k/core.h:18,
>>                  from drivers/net/wireless/ath/ath10k/htt_rx.c:8:
>> drivers/net/wireless/ath/ath10k/htt_rx.c: In function 'ath10k_htt_rx_tx_fetch_ind':
>> drivers/net/wireless/ath/ath10k/htt.h:1683:17: warning: array subscript 65535 is outside the bounds of an interior zero-length array 'struct htt_tx_fetch_record[0]' [-Wzero-length-bounds]
>>  1683 |  return (void *)&ind->records[le16_to_cpu(ind->num_records)];
>>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/wireless/ath/ath10k/htt.h:1676:29: note: while referencing 'records'
>>  1676 |  struct htt_tx_fetch_record records[0];
>>       |                             ^~~~~~~
>> 
>> Make records[] a flexible array member to allow this, moving it behind
>> the other zero-length member that is not accessed in a way that gcc
>> warns about.
>> 
>> Fixes: 3ba225b506a2 ("treewide: Replace zero-length array with
>> flexible-array member")
>
> This treewide patch no longer contains changes for ath10k. I removed them
> since Monday (05/04/2020). So, this "Fixes" tag does not apply.

Ok, I'll remove it. Also I'll take these to my ath.git tree, not to
net-next.

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

Re: [PATCH net-next 2/2] ath10k: fix ath10k_pci struct layout

[Kalle Valo]

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.

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

Re: [PATCH net-next 2/2] ath10k: fix ath10k_pci struct layout

[Kalle Valo]

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.

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

Re: [PATCH net-next 2/2] ath10k: fix ath10k_pci struct layout

[Arnd Bergmann]

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

Re: [PATCH net-next 1/2] ath10k: fix gcc-10 zero-length-bounds warnings

[Arnd Bergmann]

On Mon, May 11, 2020 at 2:03 PM Kalle Valo <kvalo@codeaurora.org> wrote:
> "Gustavo A. R. Silva" <gustavoars@kernel.org> writes:

> >
> > This treewide patch no longer contains changes for ath10k. I removed them
> > since Monday (05/04/2020). So, this "Fixes" tag does not apply.

Oops, I forgot to update the changelog trext when rebasing.

> Ok, I'll remove it. Also I'll take these to my ath.git tree, not to
> net-next.

Thanks a lot!

       Arnd

Re: [PATCH net-next 1/2] ath10k: fix gcc-10 zero-length-bounds warnings

[Kalle Valo]

Arnd Bergmann <arnd@arndb.de> writes:

> On Mon, May 11, 2020 at 2:03 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>> "Gustavo A. R. Silva" <gustavoars@kernel.org> writes:
>
>> >
>> > This treewide patch no longer contains changes for ath10k. I removed them
>> > since Monday (05/04/2020). So, this "Fixes" tag does not apply.
>
> Oops, I forgot to update the changelog trext when rebasing.
>
>> Ok, I'll remove it. Also I'll take these to my ath.git tree, not to
>> net-next.
>
> Thanks a lot!

Weird, I had a conflict with this patch but couldn't figure out why.
Anyway, I fixed it in my pending branch and please double check:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=c3e5221f3c3ddabc76a33ff08440ff1dc664998d

At least GCC-10 is happy now.

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

Re: [PATCH net-next 1/2] ath10k: fix gcc-10 zero-length-bounds warnings

[Arnd Bergmann]

On Mon, May 11, 2020 at 3:10 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Arnd Bergmann <arnd@arndb.de> writes:
>
> > On Mon, May 11, 2020 at 2:03 PM Kalle Valo <kvalo@codeaurora.org> wrote:
> >> "Gustavo A. R. Silva" <gustavoars@kernel.org> writes:
> >
> >> >
> >> > This treewide patch no longer contains changes for ath10k. I removed them
> >> > since Monday (05/04/2020). So, this "Fixes" tag does not apply.
> >
> > Oops, I forgot to update the changelog trext when rebasing.
> >
> >> Ok, I'll remove it. Also I'll take these to my ath.git tree, not to
> >> net-next.
> >
> > Thanks a lot!
>
> Weird, I had a conflict with this patch but couldn't figure out why.
> Anyway, I fixed it in my pending branch and please double check:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=c3e5221f3c3ddabc76a33ff08440ff1dc664998d

Looks good to me. It may have been an artifact on my side, as I
have applied and later reverted Gustavo's patch on the same branch.

     Arnd

Re: [PATCH net-next 1/2] ath10k: fix gcc-10 zero-length-bounds warnings

[Kalle Valo]

Arnd Bergmann <arnd@arndb.de> wrote:

> gcc-10 started warning about out-of-bounds access for zero-length
> arrays:
> 
> In file included from drivers/net/wireless/ath/ath10k/core.h:18,
>                  from drivers/net/wireless/ath/ath10k/htt_rx.c:8:
> drivers/net/wireless/ath/ath10k/htt_rx.c: In function 'ath10k_htt_rx_tx_fetch_ind':
> drivers/net/wireless/ath/ath10k/htt.h:1683:17: warning: array subscript 65535 is outside the bounds of an interior zero-length array 'struct htt_tx_fetch_record[0]' [-Wzero-length-bounds]
>  1683 |  return (void *)&ind->records[le16_to_cpu(ind->num_records)];
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath10k/htt.h:1676:29: note: while referencing 'records'
>  1676 |  struct htt_tx_fetch_record records[0];
>       |                             ^~~~~~~
> 
> Make records[] a flexible array member to allow this, moving it behind
> the other zero-length member that is not accessed in a way that gcc
> warns about.
> 
> Fixes: 22e6b3bc5d96 ("ath10k: add new htt definitions")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

2 patches applied to ath-next branch of ath.git, thanks.

9f12bebd512c ath10k: fix gcc-10 zero-length-bounds warnings
32221df6765b ath10k: fix ath10k_pci struct layout

-- 
https://patchwork.kernel.org/patch/11538233/

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