* [PATCH v3] ethtool: Replace 0-length array with flexible array
@ 2023-01-06 4:28 Kees Cook
2023-01-06 5:38 ` Vincent MAILHOL
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Kees Cook @ 2023-01-06 4:28 UTC (permalink / raw)
To: David S. Miller
Cc: Kees Cook, Jakub Kicinski, Andrew Lunn, kernel test robot,
Oleksij Rempel, Sean Anderson, Alexandru Tachici, Amit Cohen,
Gustavo A. R. Silva, Vincent Mailhol, netdev, linux-kernel,
linux-hardening
Zero-length arrays are deprecated[1]. Replace struct ethtool_rxnfc's
"rule_locs" 0-length array with a flexible array. Detected with GCC 13,
using -fstrict-flex-arrays=3:
net/ethtool/common.c: In function 'ethtool_get_max_rxnfc_channel':
net/ethtool/common.c:558:55: warning: array subscript i is outside array bounds of '__u32[0]' {aka 'unsigned int[]'} [-Warray-bounds=]
558 | .fs.location = info->rule_locs[i],
| ~~~~~~~~~~~~~~~^~~
In file included from include/linux/ethtool.h:19,
from include/uapi/linux/ethtool_netlink.h:12,
from include/linux/ethtool_netlink.h:6,
from net/ethtool/common.c:3:
include/uapi/linux/ethtool.h:1186:41: note: while referencing
'rule_locs'
1186 | __u32 rule_locs[0];
| ^~~~~~~~~
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: kernel test robot <lkp@intel.com>
Cc: Oleksij Rempel <linux@rempel-privat.de>
Cc: Sean Anderson <sean.anderson@seco.com>
Cc: Alexandru Tachici <alexandru.tachici@analog.com>
Cc: Amit Cohen <amcohen@nvidia.com>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
v3: don't use helper (vincent)
v2: https://lore.kernel.org/lkml/20230105233420.gonna.036-kees@kernel.org
---
include/uapi/linux/ethtool.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 58e587ba0450..3135fa0ba9a4 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1183,7 +1183,7 @@ struct ethtool_rxnfc {
__u32 rule_cnt;
__u32 rss_context;
};
- __u32 rule_locs[0];
+ __u32 rule_locs[];
};
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3] ethtool: Replace 0-length array with flexible array
2023-01-06 4:28 [PATCH v3] ethtool: Replace 0-length array with flexible array Kees Cook
@ 2023-01-06 5:38 ` Vincent MAILHOL
2023-01-06 20:07 ` Kees Cook
2023-01-06 5:47 ` Vincent MAILHOL
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Vincent MAILHOL @ 2023-01-06 5:38 UTC (permalink / raw)
To: Kees Cook
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, kernel test robot,
Oleksij Rempel, Sean Anderson, Alexandru Tachici, Amit Cohen,
Gustavo A. R. Silva, netdev, linux-kernel, linux-hardening
On Fri. 6 Jan 2023 at 13:28, Kees Cook <keescook@chromium.org> wrote:
> Zero-length arrays are deprecated[1]. Replace struct ethtool_rxnfc's
> "rule_locs" 0-length array with a flexible array. Detected with GCC 13,
> using -fstrict-flex-arrays=3:
>
> net/ethtool/common.c: In function 'ethtool_get_max_rxnfc_channel':
> net/ethtool/common.c:558:55: warning: array subscript i is outside array bounds of '__u32[0]' {aka 'unsigned int[]'} [-Warray-bounds=]
> 558 | .fs.location = info->rule_locs[i],
> | ~~~~~~~~~~~~~~~^~~
> In file included from include/linux/ethtool.h:19,
> from include/uapi/linux/ethtool_netlink.h:12,
> from include/linux/ethtool_netlink.h:6,
> from net/ethtool/common.c:3:
> include/uapi/linux/ethtool.h:1186:41: note: while referencing
> 'rule_locs'
> 1186 | __u32 rule_locs[0];
> | ^~~~~~~~~
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
Side comment, this link does not mention the __DECLARE_FLEX_ARRAY().
It could be good to add a reference to the helper here. But of course,
this is not a criticism of this patch.
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: kernel test robot <lkp@intel.com>
> Cc: Oleksij Rempel <linux@rempel-privat.de>
> Cc: Sean Anderson <sean.anderson@seco.com>
> Cc: Alexandru Tachici <alexandru.tachici@analog.com>
> Cc: Amit Cohen <amcohen@nvidia.com>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> ---
> v3: don't use helper (vincent)
You may want to double check your other patches as well. At least this
one is also using the helper when not needed:
https://lore.kernel.org/netdev/20230105223642.never.980-kees@kernel.org/T/#u
> v2: https://lore.kernel.org/lkml/20230105233420.gonna.036-kees@kernel.org
> ---
> include/uapi/linux/ethtool.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 58e587ba0450..3135fa0ba9a4 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1183,7 +1183,7 @@ struct ethtool_rxnfc {
> __u32 rule_cnt;
> __u32 rss_context;
> };
> - __u32 rule_locs[0];
> + __u32 rule_locs[];
> };
>
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] ethtool: Replace 0-length array with flexible array
2023-01-06 4:28 [PATCH v3] ethtool: Replace 0-length array with flexible array Kees Cook
2023-01-06 5:38 ` Vincent MAILHOL
@ 2023-01-06 5:47 ` Vincent MAILHOL
2023-01-06 6:25 ` Kees Cook
2023-01-06 13:19 ` Jann Horn
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Vincent MAILHOL @ 2023-01-06 5:47 UTC (permalink / raw)
To: Kees Cook
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, kernel test robot,
Oleksij Rempel, Sean Anderson, Alexandru Tachici, Amit Cohen,
Gustavo A. R. Silva, netdev, linux-kernel, linux-hardening
On Fri. 6 Jan 2023 at 13:28, Kees Cook <keescook@chromium.org> wrote:
> Zero-length arrays are deprecated[1]. Replace struct ethtool_rxnfc's
> "rule_locs" 0-length array with a flexible array. Detected with GCC 13,
> using -fstrict-flex-arrays=3:
>
> net/ethtool/common.c: In function 'ethtool_get_max_rxnfc_channel':
> net/ethtool/common.c:558:55: warning: array subscript i is outside array bounds of '__u32[0]' {aka 'unsigned int[]'} [-Warray-bounds=]
> 558 | .fs.location = info->rule_locs[i],
> | ~~~~~~~~~~~~~~~^~~
> In file included from include/linux/ethtool.h:19,
> from include/uapi/linux/ethtool_netlink.h:12,
> from include/linux/ethtool_netlink.h:6,
> from net/ethtool/common.c:3:
> include/uapi/linux/ethtool.h:1186:41: note: while referencing
> 'rule_locs'
> 1186 | __u32 rule_locs[0];
> | ^~~~~~~~~
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: kernel test robot <lkp@intel.com>
> Cc: Oleksij Rempel <linux@rempel-privat.de>
> Cc: Sean Anderson <sean.anderson@seco.com>
> Cc: Alexandru Tachici <alexandru.tachici@analog.com>
> Cc: Amit Cohen <amcohen@nvidia.com>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> v3: don't use helper (vincent)
v1: https://lore.kernel.org/all/20230105214126.never.757-kees@kernel.org
^^^^^
> v2: https://lore.kernel.org/lkml/20230105233420.gonna.036-kees@kernel.org
^^^^^
v3: https://lore.kernel.org/netdev/20230106042844.give.885-kees@kernel.org
^^^^
Seriously... :)
> v2: https://lore.kernel.org/lkml/20230105233420.gonna.036-kees@kernel.org
> ---
> include/uapi/linux/ethtool.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 58e587ba0450..3135fa0ba9a4 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1183,7 +1183,7 @@ struct ethtool_rxnfc {
> __u32 rule_cnt;
> __u32 rss_context;
> };
> - __u32 rule_locs[0];
> + __u32 rule_locs[];
> };
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] ethtool: Replace 0-length array with flexible array
2023-01-06 5:47 ` Vincent MAILHOL
@ 2023-01-06 6:25 ` Kees Cook
0 siblings, 0 replies; 11+ messages in thread
From: Kees Cook @ 2023-01-06 6:25 UTC (permalink / raw)
To: Vincent MAILHOL
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, kernel test robot,
Oleksij Rempel, Sean Anderson, Alexandru Tachici, Amit Cohen,
Gustavo A. R. Silva, netdev, linux-kernel, linux-hardening
On Fri, Jan 06, 2023 at 02:47:35PM +0900, Vincent MAILHOL wrote:
> On Fri. 6 Jan 2023 at 13:28, Kees Cook <keescook@chromium.org> wrote:
> > Zero-length arrays are deprecated[1]. Replace struct ethtool_rxnfc's
> > "rule_locs" 0-length array with a flexible array. Detected with GCC 13,
> > using -fstrict-flex-arrays=3:
> >
> > net/ethtool/common.c: In function 'ethtool_get_max_rxnfc_channel':
> > net/ethtool/common.c:558:55: warning: array subscript i is outside array bounds of '__u32[0]' {aka 'unsigned int[]'} [-Warray-bounds=]
> > 558 | .fs.location = info->rule_locs[i],
> > | ~~~~~~~~~~~~~~~^~~
> > In file included from include/linux/ethtool.h:19,
> > from include/uapi/linux/ethtool_netlink.h:12,
> > from include/linux/ethtool_netlink.h:6,
> > from net/ethtool/common.c:3:
> > include/uapi/linux/ethtool.h:1186:41: note: while referencing
> > 'rule_locs'
> > 1186 | __u32 rule_locs[0];
> > | ^~~~~~~~~
> >
> > [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
> >
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Andrew Lunn <andrew@lunn.ch>
> > Cc: kernel test robot <lkp@intel.com>
> > Cc: Oleksij Rempel <linux@rempel-privat.de>
> > Cc: Sean Anderson <sean.anderson@seco.com>
> > Cc: Alexandru Tachici <alexandru.tachici@analog.com>
> > Cc: Amit Cohen <amcohen@nvidia.com>
> > Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> > Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > v3: don't use helper (vincent)
>
> v1: https://lore.kernel.org/all/20230105214126.never.757-kees@kernel.org
> ^^^^^
> > v2: https://lore.kernel.org/lkml/20230105233420.gonna.036-kees@kernel.org
> ^^^^^
> v3: https://lore.kernel.org/netdev/20230106042844.give.885-kees@kernel.org
> ^^^^
>
> Seriously... :)
Hurray! Someone noticed and it's not even April yet. :) *celebrate*
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] ethtool: Replace 0-length array with flexible array
2023-01-06 4:28 [PATCH v3] ethtool: Replace 0-length array with flexible array Kees Cook
2023-01-06 5:38 ` Vincent MAILHOL
2023-01-06 5:47 ` Vincent MAILHOL
@ 2023-01-06 13:19 ` Jann Horn
2023-01-06 14:25 ` Vincent MAILHOL
2023-01-06 16:25 ` [PATCH v3] ethtool: Replace 0-length array with flexible array Gustavo A. R. Silva
2023-01-07 3:40 ` patchwork-bot+netdevbpf
4 siblings, 1 reply; 11+ messages in thread
From: Jann Horn @ 2023-01-06 13:19 UTC (permalink / raw)
To: Kees Cook
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, kernel test robot,
Oleksij Rempel, Sean Anderson, Alexandru Tachici, Amit Cohen,
Gustavo A. R. Silva, Vincent Mailhol, netdev, linux-kernel,
linux-hardening, Linux API
On Fri, Jan 6, 2023 at 5:28 AM Kees Cook <keescook@chromium.org> wrote:
> Zero-length arrays are deprecated[1]. Replace struct ethtool_rxnfc's
> "rule_locs" 0-length array with a flexible array. Detected with GCC 13,
> using -fstrict-flex-arrays=3:
>
> net/ethtool/common.c: In function 'ethtool_get_max_rxnfc_channel':
> net/ethtool/common.c:558:55: warning: array subscript i is outside array bounds of '__u32[0]' {aka 'unsigned int[]'} [-Warray-bounds=]
> 558 | .fs.location = info->rule_locs[i],
> | ~~~~~~~~~~~~~~~^~~
> In file included from include/linux/ethtool.h:19,
> from include/uapi/linux/ethtool_netlink.h:12,
> from include/linux/ethtool_netlink.h:6,
> from net/ethtool/common.c:3:
> include/uapi/linux/ethtool.h:1186:41: note: while referencing
> 'rule_locs'
> 1186 | __u32 rule_locs[0];
> | ^~~~~~~~~
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: kernel test robot <lkp@intel.com>
> Cc: Oleksij Rempel <linux@rempel-privat.de>
> Cc: Sean Anderson <sean.anderson@seco.com>
> Cc: Alexandru Tachici <alexandru.tachici@analog.com>
> Cc: Amit Cohen <amcohen@nvidia.com>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> v3: don't use helper (vincent)
> v2: https://lore.kernel.org/lkml/20230105233420.gonna.036-kees@kernel.org
> ---
> include/uapi/linux/ethtool.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 58e587ba0450..3135fa0ba9a4 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1183,7 +1183,7 @@ struct ethtool_rxnfc {
> __u32 rule_cnt;
> __u32 rss_context;
> };
> - __u32 rule_locs[0];
> + __u32 rule_locs[];
Stupid question: Is this syntax allowed in UAPI headers despite not
being part of standard C90 or C++? Are we relying on all C/C++
compilers for pre-C99 having gcc/clang extensions?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] ethtool: Replace 0-length array with flexible array
2023-01-06 13:19 ` Jann Horn
@ 2023-01-06 14:25 ` Vincent MAILHOL
2023-01-06 20:13 ` minimum compiler for Linux UAPI (was Re: [PATCH v3] ethtool: Replace 0-length array with flexible array) Kees Cook
0 siblings, 1 reply; 11+ messages in thread
From: Vincent MAILHOL @ 2023-01-06 14:25 UTC (permalink / raw)
To: Jann Horn
Cc: Kees Cook, David S. Miller, Jakub Kicinski, Andrew Lunn,
kernel test robot, Oleksij Rempel, Sean Anderson,
Alexandru Tachici, Amit Cohen, Gustavo A. R. Silva, netdev,
linux-kernel, linux-hardening, Linux API
On Fri. 6 Jan 2023 at 22:19, Jann Horn <jannh@google.com> wrote:
> On Fri, Jan 6, 2023 at 5:28 AM Kees Cook <keescook@chromium.org> wrote:
> > Zero-length arrays are deprecated[1]. Replace struct ethtool_rxnfc's
> > "rule_locs" 0-length array with a flexible array. Detected with GCC 13,
> > using -fstrict-flex-arrays=3:
> >
> > net/ethtool/common.c: In function 'ethtool_get_max_rxnfc_channel':
> > net/ethtool/common.c:558:55: warning: array subscript i is outside array bounds of '__u32[0]' {aka 'unsigned int[]'} [-Warray-bounds=]
> > 558 | .fs.location = info->rule_locs[i],
> > | ~~~~~~~~~~~~~~~^~~
> > In file included from include/linux/ethtool.h:19,
> > from include/uapi/linux/ethtool_netlink.h:12,
> > from include/linux/ethtool_netlink.h:6,
> > from net/ethtool/common.c:3:
> > include/uapi/linux/ethtool.h:1186:41: note: while referencing
> > 'rule_locs'
> > 1186 | __u32 rule_locs[0];
> > | ^~~~~~~~~
> >
> > [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
> >
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Andrew Lunn <andrew@lunn.ch>
> > Cc: kernel test robot <lkp@intel.com>
> > Cc: Oleksij Rempel <linux@rempel-privat.de>
> > Cc: Sean Anderson <sean.anderson@seco.com>
> > Cc: Alexandru Tachici <alexandru.tachici@analog.com>
> > Cc: Amit Cohen <amcohen@nvidia.com>
> > Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> > Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > v3: don't use helper (vincent)
> > v2: https://lore.kernel.org/lkml/20230105233420.gonna.036-kees@kernel.org
> > ---
> > include/uapi/linux/ethtool.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> > index 58e587ba0450..3135fa0ba9a4 100644
> > --- a/include/uapi/linux/ethtool.h
> > +++ b/include/uapi/linux/ethtool.h
> > @@ -1183,7 +1183,7 @@ struct ethtool_rxnfc {
> > __u32 rule_cnt;
> > __u32 rss_context;
> > };
> > - __u32 rule_locs[0];
> > + __u32 rule_locs[];
>
> Stupid question: Is this syntax allowed in UAPI headers despite not
> being part of standard C90 or C++? Are we relying on all C/C++
> compilers for pre-C99 having gcc/clang extensions?
The [0] isn't part of the C90 standard either. So having to choose
between [0] and [], the latter is the most portable nowadays.
If I do a bit of speleology, I can see that C99 flexible array members
were used as early as v2.6.19 (released in November 2006):
https://elixir.bootlin.com/linux/v2.6.19/source/include/linux/usb/audio.h#L36
This is prior to the include/linux and include/uapi/linux split, but
believe me, this usb/audio.h file is indeed part of the uapi.
So, yes, using C99 flexible array members in the UAPI is de facto
allowed because it was used for the last 16 years.
An interesting sub question would be:
What are the minimum compiler requirements to build a program using
the Linux UAPI?
And, after research, I could not find the answer. The requirements to
build the kernel are well documented:
https://docs.kernel.org/process/changes.html#changes
But no clue for the uapi. I guess that at one point in 2006, people
decided that it was time to set the minimum requirement to C99. Maybe
this matches the end of life of the latest pre-C99 GCC version? The
detailed answer must be hidden somewhere on lkml.
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] ethtool: Replace 0-length array with flexible array
2023-01-06 4:28 [PATCH v3] ethtool: Replace 0-length array with flexible array Kees Cook
` (2 preceding siblings ...)
2023-01-06 13:19 ` Jann Horn
@ 2023-01-06 16:25 ` Gustavo A. R. Silva
2023-01-07 3:40 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 11+ messages in thread
From: Gustavo A. R. Silva @ 2023-01-06 16:25 UTC (permalink / raw)
To: Kees Cook
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, kernel test robot,
Oleksij Rempel, Sean Anderson, Alexandru Tachici, Amit Cohen,
Vincent Mailhol, netdev, linux-kernel, linux-hardening
On Thu, Jan 05, 2023 at 08:28:48PM -0800, Kees Cook wrote:
> Zero-length arrays are deprecated[1]. Replace struct ethtool_rxnfc's
> "rule_locs" 0-length array with a flexible array. Detected with GCC 13,
> using -fstrict-flex-arrays=3:
>
> net/ethtool/common.c: In function 'ethtool_get_max_rxnfc_channel':
> net/ethtool/common.c:558:55: warning: array subscript i is outside array bounds of '__u32[0]' {aka 'unsigned int[]'} [-Warray-bounds=]
> 558 | .fs.location = info->rule_locs[i],
> | ~~~~~~~~~~~~~~~^~~
> In file included from include/linux/ethtool.h:19,
> from include/uapi/linux/ethtool_netlink.h:12,
> from include/linux/ethtool_netlink.h:6,
> from net/ethtool/common.c:3:
> include/uapi/linux/ethtool.h:1186:41: note: while referencing
> 'rule_locs'
> 1186 | __u32 rule_locs[0];
> | ^~~~~~~~~
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: kernel test robot <lkp@intel.com>
> Cc: Oleksij Rempel <linux@rempel-privat.de>
> Cc: Sean Anderson <sean.anderson@seco.com>
> Cc: Alexandru Tachici <alexandru.tachici@analog.com>
> Cc: Amit Cohen <amcohen@nvidia.com>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Thanks!
--
Gustavo
> ---
> v3: don't use helper (vincent)
> v2: https://lore.kernel.org/lkml/20230105233420.gonna.036-kees@kernel.org
> ---
> include/uapi/linux/ethtool.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 58e587ba0450..3135fa0ba9a4 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1183,7 +1183,7 @@ struct ethtool_rxnfc {
> __u32 rule_cnt;
> __u32 rss_context;
> };
> - __u32 rule_locs[0];
> + __u32 rule_locs[];
> };
>
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] ethtool: Replace 0-length array with flexible array
2023-01-06 5:38 ` Vincent MAILHOL
@ 2023-01-06 20:07 ` Kees Cook
0 siblings, 0 replies; 11+ messages in thread
From: Kees Cook @ 2023-01-06 20:07 UTC (permalink / raw)
To: Vincent MAILHOL
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, kernel test robot,
Oleksij Rempel, Sean Anderson, Alexandru Tachici, Amit Cohen,
Gustavo A. R. Silva, netdev, linux-kernel, linux-hardening
On Fri, Jan 06, 2023 at 02:38:18PM +0900, Vincent MAILHOL wrote:
> On Fri. 6 Jan 2023 at 13:28, Kees Cook <keescook@chromium.org> wrote:
> > [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays
>
> Side comment, this link does not mention the __DECLARE_FLEX_ARRAY().
> It could be good to add a reference to the helper here. But of course,
> this is not a criticism of this patch.
Good point! I've sent a patch for this now.
> You may want to double check your other patches as well. At least this
> one is also using the helper when not needed:
>
> https://lore.kernel.org/netdev/20230105223642.never.980-kees@kernel.org/T/#u
That one does, actually, need it since otherwise the flex array would be
"alone in a struct" which is the other case that C99 irrationally
disallows.
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: minimum compiler for Linux UAPI (was Re: [PATCH v3] ethtool: Replace 0-length array with flexible array)
2023-01-06 14:25 ` Vincent MAILHOL
@ 2023-01-06 20:13 ` Kees Cook
2023-01-06 20:58 ` Arnd Bergmann
0 siblings, 1 reply; 11+ messages in thread
From: Kees Cook @ 2023-01-06 20:13 UTC (permalink / raw)
To: Vincent MAILHOL
Cc: Arnd Bergmann, Jann Horn, David S. Miller, Jakub Kicinski,
Andrew Lunn, kernel test robot, Oleksij Rempel, Sean Anderson,
Alexandru Tachici, Amit Cohen, Gustavo A. R. Silva, netdev,
linux-kernel, linux-hardening, Linux API
On Fri, Jan 06, 2023 at 11:25:14PM +0900, Vincent MAILHOL wrote:
> On Fri. 6 Jan 2023 at 22:19, Jann Horn <jannh@google.com> wrote:
> > On Fri, Jan 6, 2023 at 5:28 AM Kees Cook <keescook@chromium.org> wrote:
> > > Zero-length arrays are deprecated[1]. Replace struct ethtool_rxnfc's
> > > "rule_locs" 0-length array with a flexible array. Detected with GCC 13,
> > > using -fstrict-flex-arrays=3:
> [...]
> > > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> > > index 58e587ba0450..3135fa0ba9a4 100644
> > > --- a/include/uapi/linux/ethtool.h
> > > +++ b/include/uapi/linux/ethtool.h
> > > @@ -1183,7 +1183,7 @@ struct ethtool_rxnfc {
> > > __u32 rule_cnt;
> > > __u32 rss_context;
> > > };
> > > - __u32 rule_locs[0];
> > > + __u32 rule_locs[];
> >
> > Stupid question: Is this syntax allowed in UAPI headers despite not
> > being part of standard C90 or C++? Are we relying on all C/C++
> > compilers for pre-C99 having gcc/clang extensions?
>
> The [0] isn't part of the C90 standard either. So having to choose
> between [0] and [], the latter is the most portable nowadays.
>
> If I do a bit of speleology, I can see that C99 flexible array members
> were used as early as v2.6.19 (released in November 2006):
>
> https://elixir.bootlin.com/linux/v2.6.19/source/include/linux/usb/audio.h#L36
>
> This is prior to the include/linux and include/uapi/linux split, but
> believe me, this usb/audio.h file is indeed part of the uapi.
> So, yes, using C99 flexible array members in the UAPI is de facto
> allowed because it was used for the last 16 years.
>
> An interesting sub question would be:
>
> What are the minimum compiler requirements to build a program using
> the Linux UAPI?
You're right -- we haven't explicitly documented this. C99 seems like
the defacto minimum, though.
> And, after research, I could not find the answer. The requirements to
> build the kernel are well documented:
>
> https://docs.kernel.org/process/changes.html#changes
>
> But no clue for the uapi. I guess that at one point in 2006, people
> decided that it was time to set the minimum requirement to C99. Maybe
> this matches the end of life of the latest pre-C99 GCC version? The
> detailed answer must be hidden somewhere on lkml.
I would make the argument that the requirements for building Linux UAPI
should match that of building the kernel...
--
Kees Cook
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: minimum compiler for Linux UAPI (was Re: [PATCH v3] ethtool: Replace 0-length array with flexible array)
2023-01-06 20:13 ` minimum compiler for Linux UAPI (was Re: [PATCH v3] ethtool: Replace 0-length array with flexible array) Kees Cook
@ 2023-01-06 20:58 ` Arnd Bergmann
0 siblings, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2023-01-06 20:58 UTC (permalink / raw)
To: Kees Cook, Vincent MAILHOL
Cc: Jann Horn, David S . Miller, Jakub Kicinski, Andrew Lunn,
kernel test robot, Oleksij Rempel, Sean Anderson,
Alexandru Tachici, Amit Cohen, Gustavo A. R. Silva, Netdev,
linux-kernel, linux-hardening, Linux API
On Fri, Jan 6, 2023, at 21:13, Kees Cook wrote:
> On Fri, Jan 06, 2023 at 11:25:14PM +0900, Vincent MAILHOL wrote:
>> On Fri. 6 Jan 2023 at 22:19, Jann Horn <jannh@google.com> wrote:
>>
>> What are the minimum compiler requirements to build a program using
>> the Linux UAPI?
>
> You're right -- we haven't explicitly documented this. C99 seems like
> the defacto minimum, though.
>
>> And, after research, I could not find the answer. The requirements to
>> build the kernel are well documented:
>>
>> https://docs.kernel.org/process/changes.html#changes
>>
>> But no clue for the uapi. I guess that at one point in 2006, people
>> decided that it was time to set the minimum requirement to C99. Maybe
>> this matches the end of life of the latest pre-C99 GCC version? The
>> detailed answer must be hidden somewhere on lkml.
>
> I would make the argument that the requirements for building Linux UAPI
> should match that of building the kernel...
I think it's a bit more nuanced than that: glibc does not require
C99 but does include some kernel headers from generic library
headers, so I would not make the assumption that it's always
safe to use C99 features. On the other hand, Linux specific
device drivers whose header is only really used from one
application are free to make assumptions about the toolchain.
Arnd
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] ethtool: Replace 0-length array with flexible array
2023-01-06 4:28 [PATCH v3] ethtool: Replace 0-length array with flexible array Kees Cook
` (3 preceding siblings ...)
2023-01-06 16:25 ` [PATCH v3] ethtool: Replace 0-length array with flexible array Gustavo A. R. Silva
@ 2023-01-07 3:40 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-01-07 3:40 UTC (permalink / raw)
To: Kees Cook
Cc: davem, kuba, andrew, lkp, linux, sean.anderson,
alexandru.tachici, amcohen, gustavoars, mailhol.vincent, netdev,
linux-kernel, linux-hardening
Hello:
This patch was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 5 Jan 2023 20:28:48 -0800 you wrote:
> Zero-length arrays are deprecated[1]. Replace struct ethtool_rxnfc's
> "rule_locs" 0-length array with a flexible array. Detected with GCC 13,
> using -fstrict-flex-arrays=3:
>
> net/ethtool/common.c: In function 'ethtool_get_max_rxnfc_channel':
> net/ethtool/common.c:558:55: warning: array subscript i is outside array bounds of '__u32[0]' {aka 'unsigned int[]'} [-Warray-bounds=]
> 558 | .fs.location = info->rule_locs[i],
> | ~~~~~~~~~~~~~~~^~~
> In file included from include/linux/ethtool.h:19,
> from include/uapi/linux/ethtool_netlink.h:12,
> from include/linux/ethtool_netlink.h:6,
> from net/ethtool/common.c:3:
> include/uapi/linux/ethtool.h:1186:41: note: while referencing
> 'rule_locs'
> 1186 | __u32 rule_locs[0];
> | ^~~~~~~~~
>
> [...]
Here is the summary with links:
- [v3] ethtool: Replace 0-length array with flexible array
https://git.kernel.org/netdev/net-next/c/b466a25c930f
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-01-07 3:40 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-06 4:28 [PATCH v3] ethtool: Replace 0-length array with flexible array Kees Cook
2023-01-06 5:38 ` Vincent MAILHOL
2023-01-06 20:07 ` Kees Cook
2023-01-06 5:47 ` Vincent MAILHOL
2023-01-06 6:25 ` Kees Cook
2023-01-06 13:19 ` Jann Horn
2023-01-06 14:25 ` Vincent MAILHOL
2023-01-06 20:13 ` minimum compiler for Linux UAPI (was Re: [PATCH v3] ethtool: Replace 0-length array with flexible array) Kees Cook
2023-01-06 20:58 ` Arnd Bergmann
2023-01-06 16:25 ` [PATCH v3] ethtool: Replace 0-length array with flexible array Gustavo A. R. Silva
2023-01-07 3:40 ` patchwork-bot+netdevbpf
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).