* [PATCH] sky2: Make sure there is at least one frag_addr available
@ 2023-09-20 20:25 Kees Cook
2023-09-21 3:37 ` Stephen Hemminger
2023-09-21 14:14 ` Alexander Lobakin
0 siblings, 2 replies; 3+ messages in thread
From: Kees Cook @ 2023-09-20 20:25 UTC (permalink / raw)
To: Mirko Lindner
Cc: Kees Cook, Stephen Hemminger, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, kernel test robot,
linux-kernel, linux-hardening
In the likely pathological case of building sky2 with 16k PAGE_SIZE,
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/
Signed-off-by: Kees Cook <keescook@chromium.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 {
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] sky2: Make sure there is at least one frag_addr available
2023-09-20 20:25 [PATCH] sky2: Make sure there is at least one frag_addr available Kees Cook
@ 2023-09-21 3:37 ` Stephen Hemminger
2023-09-21 14:14 ` Alexander Lobakin
1 sibling, 0 replies; 3+ messages in thread
From: Stephen Hemminger @ 2023-09-21 3:37 UTC (permalink / raw)
To: Kees Cook
Cc: Mirko Lindner, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, kernel test robot, linux-kernel,
linux-hardening
On Wed, 20 Sep 2023 13:25:13 -0700
Kees Cook <keescook@chromium.org> wrote:
> In the likely pathological case of building sky2 with 16k PAGE_SIZE,
> 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/
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
With page size of 16K 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 thinking this is some flex array leftover? Or can't figure out that
in this case an rx skb with fragments would never be created.
The workaround is fine, but could you add an explanatory comment?
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] sky2: Make sure there is at least one frag_addr available
2023-09-20 20:25 [PATCH] sky2: Make sure there is at least one frag_addr available Kees Cook
2023-09-21 3:37 ` Stephen Hemminger
@ 2023-09-21 14:14 ` Alexander Lobakin
1 sibling, 0 replies; 3+ messages in thread
From: Alexander Lobakin @ 2023-09-21 14:14 UTC (permalink / raw)
To: Kees Cook
Cc: Mirko Lindner, Stephen Hemminger, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, kernel test robot,
linux-kernel, linux-hardening
From: Kees Cook <keescook@chromium.org>
Date: Wed, 20 Sep 2023 13:25:13 -0700
> In the likely pathological case of building sky2 with 16k PAGE_SIZE,
> 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/
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>
That "nobody uses this HW on non-x86 systems, why bother" is fun each time.
> ---
> 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 {
Thanks,
Olek
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-21 19:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-20 20:25 [PATCH] sky2: Make sure there is at least one frag_addr available Kees Cook
2023-09-21 3:37 ` Stephen Hemminger
2023-09-21 14:14 ` Alexander Lobakin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).