| Message ID | 20230922165036.gonna.464-kees@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6a70e5cbedaf8ad10528ac9ac114f3ec20f422df |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] sky2: Make sure there is at least one frag_addr available | expand |
On 9/22/23 10:50, Kees Cook wrote: > In the pathological case of building sky2 with 16k PAGE_SIZE, the > frag_addr[] array would never be used, so the original code was correct > that size should be 0. But the compiler now gets upset with 0 size arrays > in places where it hasn't eliminated the code that might access such an > array (it can't figure out that in this case an rx skb with fragments > would never be created). To keep the compiler happy, make sure there is > at least 1 frag_addr in struct rx_ring_info: > > 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]; > | ^~~~~~~~~ > > With CONFIG_PAGE_SIZE_16KB=y, PAGE_SHIFT == 14, so: > > #define ETH_JUMBO_MTU 9000 > > causes "ETH_JUMBO_MTU >> PAGE_SHIFT" to be 0. Use "?: 1" to solve this build warning. > > Cc: Mirko Lindner <mlindner@marvell.com> > Cc: Stephen Hemminger <stephen@networkplumber.org> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Eric Dumazet <edumazet@google.com> > Cc: Jakub Kicinski <kuba@kernel.org> > Cc: Paolo Abeni <pabeni@redhat.com> > Cc: netdev@vger.kernel.org > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202309191958.UBw1cjXk-lkp@intel.com/ > Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com> > Signed-off-by: Kees Cook <keescook@chromium.org> Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> Thanks -- Gustavo > --- > v2 - improve commit message, add Ack > v1 - https://lore.kernel.org/netdev/20230920202509.never.299-kees@kernel.org/ > --- > drivers/net/ethernet/marvell/sky2.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/marvell/sky2.h b/drivers/net/ethernet/marvell/sky2.h > index ddec1627f1a7..8d0bacf4e49c 100644 > --- a/drivers/net/ethernet/marvell/sky2.h > +++ b/drivers/net/ethernet/marvell/sky2.h > @@ -2195,7 +2195,7 @@ struct rx_ring_info { > struct sk_buff *skb; > dma_addr_t data_addr; > DEFINE_DMA_UNMAP_LEN(data_size); > - dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT]; > + dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT ?: 1]; > }; > > enum flow_control {
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Fri, 22 Sep 2023 09:50:39 -0700 you wrote: > In the pathological case of building sky2 with 16k PAGE_SIZE, the > frag_addr[] array would never be used, so the original code was correct > that size should be 0. But the compiler now gets upset with 0 size arrays > in places where it hasn't eliminated the code that might access such an > array (it can't figure out that in this case an rx skb with fragments > would never be created). To keep the compiler happy, make sure there is > at least 1 frag_addr in struct rx_ring_info: > > [...] Here is the summary with links: - [v2] sky2: Make sure there is at least one frag_addr available https://git.kernel.org/netdev/net/c/6a70e5cbedaf You are awesome, thank you!
diff --git a/drivers/net/ethernet/marvell/sky2.h b/drivers/net/ethernet/marvell/sky2.h index ddec1627f1a7..8d0bacf4e49c 100644 --- a/drivers/net/ethernet/marvell/sky2.h +++ b/drivers/net/ethernet/marvell/sky2.h @@ -2195,7 +2195,7 @@ struct rx_ring_info { struct sk_buff *skb; dma_addr_t data_addr; DEFINE_DMA_UNMAP_LEN(data_size); - dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT]; + dma_addr_t frag_addr[ETH_JUMBO_MTU >> PAGE_SHIFT ?: 1]; }; enum flow_control {
In the pathological case of building sky2 with 16k PAGE_SIZE, the frag_addr[] array would never be used, so the original code was correct that size should be 0. But the compiler now gets upset with 0 size arrays in places where it hasn't eliminated the code that might access such an array (it can't figure out that in this case an rx skb with fragments would never be created). To keep the compiler happy, make sure there is at least 1 frag_addr in struct rx_ring_info: 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]; | ^~~~~~~~~ With CONFIG_PAGE_SIZE_16KB=y, PAGE_SHIFT == 14, so: #define ETH_JUMBO_MTU 9000 causes "ETH_JUMBO_MTU >> PAGE_SHIFT" to be 0. Use "?: 1" to solve this build warning. Cc: Mirko Lindner <mlindner@marvell.com> Cc: Stephen Hemminger <stephen@networkplumber.org> Cc: "David S. Miller" <davem@davemloft.net> Cc: Eric Dumazet <edumazet@google.com> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Paolo Abeni <pabeni@redhat.com> Cc: netdev@vger.kernel.org Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202309191958.UBw1cjXk-lkp@intel.com/ Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com> Signed-off-by: Kees Cook <keescook@chromium.org> --- v2 - improve commit message, add Ack v1 - https://lore.kernel.org/netdev/20230920202509.never.299-kees@kernel.org/ --- drivers/net/ethernet/marvell/sky2.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)