2023-09-19 11:32:41

by kernel test robot

[permalink] [raw]
Subject: include/linux/dma-mapping.h:416:36: warning: array subscript i is outside array bounds of 'dma_addr_t[0]' {aka 'long long unsigned int[]'}

tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 2cf0f715623872823a72e451243bbf555d10d032
commit: df8fc4e934c12b906d08050d7779f292b9c5c6b5 kbuild: Enable -fstrict-flex-arrays=3
date: 4 months ago
config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20230919/[email protected]/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230919/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

In file included from include/linux/skbuff.h:28,
from include/net/net_namespace.h:43,
from include/linux/netdevice.h:38,
from drivers/net/ethernet/marvell/sky2.c:18:
drivers/net/ethernet/marvell/sky2.c: In function 'sky2_rx_unmap_skb':
>> include/linux/dma-mapping.h:416:36: warning: array subscript i is outside array bounds of 'dma_addr_t[0]' {aka 'long long unsigned int[]'} [-Warray-bounds=]
416 | #define dma_unmap_page(d, a, s, r) dma_unmap_page_attrs(d, a, s, r, 0)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/marvell/sky2.c:1257:17: note: in expansion of macro 'dma_unmap_page'
1257 | dma_unmap_page(&pdev->dev, re->frag_addr[i],
| ^~~~~~~~~~~~~~
In file included from drivers/net/ethernet/marvell/sky2.c:41:
drivers/net/ethernet/marvell/sky2.h:2198:25: note: while referencing 'frag_addr'
2198 | dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT];
| ^~~~~~~~~
drivers/net/ethernet/marvell/sky2.c: In function 'sky2_rx_submit':
>> drivers/net/ethernet/marvell/sky2.c:1202:59: warning: array subscript i is outside array bounds of 'const dma_addr_t[0]' {aka 'const long long unsigned int[]'} [-Warray-bounds=]
1202 | sky2_rx_add(sky2, OP_BUFFER, re->frag_addr[i], PAGE_SIZE);
| ~~~~~~~~~~~~~^~~
drivers/net/ethernet/marvell/sky2.h:2198:25: note: while referencing 'frag_addr'
2198 | dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT];
| ^~~~~~~~~
drivers/net/ethernet/marvell/sky2.c: In function 'sky2_rx_map_skb':
>> drivers/net/ethernet/marvell/sky2.c:1222:30: warning: array subscript i is outside array bounds of 'dma_addr_t[0]' {aka 'long long unsigned int[]'} [-Warray-bounds=]
1222 | re->frag_addr[i] = skb_frag_dma_map(&pdev->dev, frag, 0,
| ~~~~~~~~~~~~~^~~
drivers/net/ethernet/marvell/sky2.h:2198:25: note: while referencing 'frag_addr'
2198 | dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT];
| ^~~~~~~~~
>> include/linux/dma-mapping.h:416:36: warning: array subscript i is outside array bounds of 'dma_addr_t[0]' {aka 'long long unsigned int[]'} [-Warray-bounds=]
416 | #define dma_unmap_page(d, a, s, r) dma_unmap_page_attrs(d, a, s, r, 0)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/marvell/sky2.c:1233:17: note: in expansion of macro 'dma_unmap_page'
1233 | dma_unmap_page(&pdev->dev, re->frag_addr[i],
| ^~~~~~~~~~~~~~
drivers/net/ethernet/marvell/sky2.h:2198:25: note: while referencing 'frag_addr'
2198 | dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT];
| ^~~~~~~~~


vim +416 include/linux/dma-mapping.h

d9d200bcebc1f6 Marek Szyprowski 2020-05-13 410
00085f1efa387a Krzysztof Kozlowski 2016-08-03 411 #define dma_map_single(d, a, s, r) dma_map_single_attrs(d, a, s, r, 0)
00085f1efa387a Krzysztof Kozlowski 2016-08-03 412 #define dma_unmap_single(d, a, s, r) dma_unmap_single_attrs(d, a, s, r, 0)
00085f1efa387a Krzysztof Kozlowski 2016-08-03 413 #define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, 0)
00085f1efa387a Krzysztof Kozlowski 2016-08-03 414 #define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, 0)
0495c3d367944e Alexander Duyck 2016-12-14 415 #define dma_map_page(d, p, o, s, r) dma_map_page_attrs(d, p, o, s, r, 0)
0495c3d367944e Alexander Duyck 2016-12-14 @416 #define dma_unmap_page(d, a, s, r) dma_unmap_page_attrs(d, a, s, r, 0)
ed6ccf10f24bdf Christoph Hellwig 2018-12-26 417 #define dma_get_sgtable(d, t, v, h, s) dma_get_sgtable_attrs(d, t, v, h, s, 0)
ed6ccf10f24bdf Christoph Hellwig 2018-12-26 418 #define dma_mmap_coherent(d, v, c, h, s) dma_mmap_attrs(d, v, c, h, s, 0)
c9eb6172c328dd Christoph Hellwig 2017-08-27 419

:::::: The code at line 416 was first introduced by commit
:::::: 0495c3d367944e4af053983ff3cdf256b567b053 dma: add calls for dma_map_page_attrs and dma_unmap_page_attrs

:::::: TO: Alexander Duyck <[email protected]>
:::::: CC: Linus Torvalds <[email protected]>

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


2023-09-20 23:50:59

by Kees Cook

[permalink] [raw]
Subject: Re: include/linux/dma-mapping.h:416:36: warning: array subscript i is outside array bounds of 'dma_addr_t[0]' {aka 'long long unsigned int[]'}

On Tue, Sep 19, 2023 at 07:27:26PM +0800, kernel test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 2cf0f715623872823a72e451243bbf555d10d032
> commit: df8fc4e934c12b906d08050d7779f292b9c5c6b5 kbuild: Enable -fstrict-flex-arrays=3
> date: 4 months ago
> config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20230919/[email protected]/config)
> compiler: loongarch64-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230919/[email protected]/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All warnings (new ones prefixed by >>):
>
> In file included from include/linux/skbuff.h:28,
> from include/net/net_namespace.h:43,
> from include/linux/netdevice.h:38,
> from drivers/net/ethernet/marvell/sky2.c:18:
> drivers/net/ethernet/marvell/sky2.c: In function 'sky2_rx_unmap_skb':
> >> include/linux/dma-mapping.h:416:36: warning: array subscript i is outside array bounds of 'dma_addr_t[0]' {aka 'long long unsigned int[]'} [-Warray-bounds=]
> 416 | #define dma_unmap_page(d, a, s, r) dma_unmap_page_attrs(d, a, s, r, 0)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/marvell/sky2.c:1257:17: note: in expansion of macro 'dma_unmap_page'
> 1257 | dma_unmap_page(&pdev->dev, re->frag_addr[i],
> | ^~~~~~~~~~~~~~
> In file included from drivers/net/ethernet/marvell/sky2.c:41:
> drivers/net/ethernet/marvell/sky2.h:2198:25: note: while referencing 'frag_addr'
> 2198 | dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT];
> | ^~~~~~~~~

The .config has:
CONFIG_PAGE_SIZE_16KB=y
which makes PAGE_SHIFT == 14

#ifdef CONFIG_PAGE_SIZE_16KB
#define PAGE_SHIFT 14

ETH_JUMBO_MTU is:

#define ETH_JUMBO_MTU 9000

which forces "ETH_JUMBO_MTU >> PAGE_SHIFT" to be 0.

I think the right fix would be:

dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT ?: 1]

Thoughts?

-Kees

--
Kees Cook

2023-09-21 01:01:48

by Stephen Hemminger

[permalink] [raw]
Subject: Re: include/linux/dma-mapping.h:416:36: warning: array subscript i is outside array bounds of 'dma_addr_t[0]' {aka 'long long unsigned int[]'}

On Wed, 20 Sep 2023 09:09:33 -0700
Kees Cook <[email protected]> wrote:

> On Tue, Sep 19, 2023 at 07:27:26PM +0800, kernel test robot wrote:
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: 2cf0f715623872823a72e451243bbf555d10d032
> > commit: df8fc4e934c12b906d08050d7779f292b9c5c6b5 kbuild: Enable -fstrict-flex-arrays=3
> > date: 4 months ago
> > config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20230919/[email protected]/config)
> > compiler: loongarch64-linux-gcc (GCC) 13.2.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230919/[email protected]/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <[email protected]>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >
> > All warnings (new ones prefixed by >>):
> >
> > In file included from include/linux/skbuff.h:28,
> > from include/net/net_namespace.h:43,
> > from include/linux/netdevice.h:38,
> > from drivers/net/ethernet/marvell/sky2.c:18:
> > drivers/net/ethernet/marvell/sky2.c: In function 'sky2_rx_unmap_skb':
> > >> include/linux/dma-mapping.h:416:36: warning: array subscript i is outside array bounds of 'dma_addr_t[0]' {aka 'long long unsigned int[]'} [-Warray-bounds=]
> > 416 | #define dma_unmap_page(d, a, s, r) dma_unmap_page_attrs(d, a, s, r, 0)
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/net/ethernet/marvell/sky2.c:1257:17: note: in expansion of macro 'dma_unmap_page'
> > 1257 | dma_unmap_page(&pdev->dev, re->frag_addr[i],
> > | ^~~~~~~~~~~~~~
> > In file included from drivers/net/ethernet/marvell/sky2.c:41:
> > drivers/net/ethernet/marvell/sky2.h:2198:25: note: while referencing 'frag_addr'
> > 2198 | dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT];
> > | ^~~~~~~~~
>
> The .config has:
> CONFIG_PAGE_SIZE_16KB=y
> which makes PAGE_SHIFT == 14
>
> #ifdef CONFIG_PAGE_SIZE_16KB
> #define PAGE_SHIFT 14
>
> ETH_JUMBO_MTU is:
>
> #define ETH_JUMBO_MTU 9000
>
> which forces "ETH_JUMBO_MTU >> PAGE_SHIFT" to be 0.
>
> I think the right fix would be:
>
> dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT ?: 1]
>
> Thoughts?
>
> -Kees
>

This is old driver, I don't have the HW anymore, it went to Free Geek.
Most of this code was based off of code in other drivers.

The assumption is that the first part of the data will be received in the
skb itself, then pages are used for overflow.

static unsigned sky2_get_rx_data_size(struct sky2_port *sky2)
{
struct rx_ring_info *re;
unsigned size;

/* Space needed for frame data + headers rounded up */
size = roundup(sky2->netdev->mtu + ETH_HLEN + VLAN_HLEN, 8);

sky2->rx_nfrags = size >> PAGE_SHIFT;
BUG_ON(sky2->rx_nfrags > ARRAY_SIZE(re->frag_addr));

Assuming PAGE_SIZE of 16k and MTU of 9000.

size = roundup(9000 + 14 + 4, 8) => 9024
sky2->rx_nfrags = 9024 >> 14 = 0

Which means no skb frags will be used.

This is probably suboptimal since it will endup calling alloc_skb()
to get a 9024 skb. Which in turn causes a call to kmalloc() of 9024.

Not really worth fixing if not testable.

2023-09-21 06:34:55

by Kees Cook

[permalink] [raw]
Subject: Re: include/linux/dma-mapping.h:416:36: warning: array subscript i is outside array bounds of 'dma_addr_t[0]' {aka 'long long unsigned int[]'}

On Wed, Sep 20, 2023 at 10:29:34AM -0700, Stephen Hemminger wrote:
> On Wed, 20 Sep 2023 09:09:33 -0700
> Kees Cook <[email protected]> wrote:
>
> > On Tue, Sep 19, 2023 at 07:27:26PM +0800, kernel test robot wrote:
> > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > > head: 2cf0f715623872823a72e451243bbf555d10d032
> > > commit: df8fc4e934c12b906d08050d7779f292b9c5c6b5 kbuild: Enable -fstrict-flex-arrays=3
> > > date: 4 months ago
> > > config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20230919/[email protected]/config)
> > > compiler: loongarch64-linux-gcc (GCC) 13.2.0
> > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230919/[email protected]/reproduce)
> > >
> > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > the same patch/commit), kindly add following tags
> > > | Reported-by: kernel test robot <[email protected]>
> > > | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> > >
> > > All warnings (new ones prefixed by >>):
> > >
> > > In file included from include/linux/skbuff.h:28,
> > > from include/net/net_namespace.h:43,
> > > from include/linux/netdevice.h:38,
> > > from drivers/net/ethernet/marvell/sky2.c:18:
> > > drivers/net/ethernet/marvell/sky2.c: In function 'sky2_rx_unmap_skb':
> > > >> include/linux/dma-mapping.h:416:36: warning: array subscript i is outside array bounds of 'dma_addr_t[0]' {aka 'long long unsigned int[]'} [-Warray-bounds=]
> > > 416 | #define dma_unmap_page(d, a, s, r) dma_unmap_page_attrs(d, a, s, r, 0)
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > drivers/net/ethernet/marvell/sky2.c:1257:17: note: in expansion of macro 'dma_unmap_page'
> > > 1257 | dma_unmap_page(&pdev->dev, re->frag_addr[i],
> > > | ^~~~~~~~~~~~~~
> > > In file included from drivers/net/ethernet/marvell/sky2.c:41:
> > > drivers/net/ethernet/marvell/sky2.h:2198:25: note: while referencing 'frag_addr'
> > > 2198 | dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT];
> > > | ^~~~~~~~~
> >
> > The .config has:
> > CONFIG_PAGE_SIZE_16KB=y
> > which makes PAGE_SHIFT == 14
> >
> > #ifdef CONFIG_PAGE_SIZE_16KB
> > #define PAGE_SHIFT 14
> >
> > ETH_JUMBO_MTU is:
> >
> > #define ETH_JUMBO_MTU 9000
> >
> > which forces "ETH_JUMBO_MTU >> PAGE_SHIFT" to be 0.
> >
> > I think the right fix would be:
> >
> > dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT ?: 1]
> >
> > Thoughts?
> >
> > -Kees
> >
>
> This is old driver, I don't have the HW anymore, it went to Free Geek.
> Most of this code was based off of code in other drivers.
>
> The assumption is that the first part of the data will be received in the
> skb itself, then pages are used for overflow.
>
> static unsigned sky2_get_rx_data_size(struct sky2_port *sky2)
> {
> struct rx_ring_info *re;
> unsigned size;
>
> /* Space needed for frame data + headers rounded up */
> size = roundup(sky2->netdev->mtu + ETH_HLEN + VLAN_HLEN, 8);
>
> sky2->rx_nfrags = size >> PAGE_SHIFT;
> BUG_ON(sky2->rx_nfrags > ARRAY_SIZE(re->frag_addr));
>
> Assuming PAGE_SIZE of 16k and MTU of 9000.
>
> size = roundup(9000 + 14 + 4, 8) => 9024
> sky2->rx_nfrags = 9024 >> 14 = 0
>
> Which means no skb frags will be used.
>
> This is probably suboptimal since it will endup calling alloc_skb()
> to get a 9024 skb. Which in turn causes a call to kmalloc() of 9024.
>
> Not really worth fixing if not testable.

Should we drop the driver? Getting "allmodconfig" to build again
with 16k pages is an easy fix here, though. I could just use

min(1, ETH_JUMBO_MTU >> PAGE_SHIFT)

too...

--
Kees Cook