Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Tue, Jun 28, 2022 at 10:54:58AM -0700, Kees Cook wrote: > which must also be assuming it's a header. So probably better to just > drop the driver_data field? I don't see anything using it (that I can > find) besides as a sanity-check that the field exists and is at the end > of the struct. The field is guaranteeing alignment of the following structure. IIRC there are a few cases that we don't have a u64 already to force this. Jason
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Tue, Jun 28, 2022 at 09:27:21AM +0200, Geert Uytterhoeven wrote: > Hi Gustavo, > > Thanks for your patch! > > On Mon, Jun 27, 2022 at 8:04 PM Gustavo A. R. Silva > wrote: > > There is a regular need in the kernel to provide a way to declare > > having a dynamically sized set of trailing elements in a structure. > > Kernel code should always use “flexible array members”[1] for these > > cases. The older style of one-element or zero-length arrays should > > no longer be used[2]. > > These rules apply to the kernel, but uapi is not considered part of the > kernel, so different rules apply. Uapi header files should work with > whatever compiler that can be used for compiling userspace. Right, userspace isn't bound by these rules, but the kernel ends up consuming these structures, so we need to fix them. The [0] -> [] changes (when they are not erroneously being used within other structures) is valid for all compilers. Flexible arrays are C99; it's been 23 years. :) But, yes, where we DO break stuff we need to workaround it, etc. -- Kees Cook
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Mon, Jun 27, 2022 at 09:40:52PM -0300, Jason Gunthorpe wrote: > On Mon, Jun 27, 2022 at 08:27:37PM +0200, Daniel Borkmann wrote: > > [...] > > Fyi, this breaks BPF CI: > > > > https://github.com/kernel-patches/bpf/runs/7078719372?check_suite_focus=true > > > > [...] > > progs/map_ptr_kern.c:314:26: error: field 'trie_key' with variable sized > > type 'struct bpf_lpm_trie_key' not at the end of a struct or class is a GNU > > extension [-Werror,-Wgnu-variable-sized-type-not-at-end] > > struct bpf_lpm_trie_key trie_key; > > ^ The issue here seems to be a collision between "unknown array size" and known sizes: struct bpf_lpm_trie_key { __u32 prefixlen; /* up to 32 for AF_INET, 128 for AF_INET6 */ __u8data[0];/* Arbitrary size */ }; struct lpm_key { struct bpf_lpm_trie_key trie_key; __u32 data; }; This is treating trie_key as a header, which it's not: it's a complete structure. :) Perhaps: struct lpm_key { __u32 prefixlen; __u32 data; }; I don't see anything else trying to include bpf_lpm_trie_key. > > This will break the rdma-core userspace as well, with a similar > error: > > /usr/bin/clang-13 -DVERBS_DEBUG -Dibverbs_EXPORTS -Iinclude > -I/usr/include/libnl3 -I/usr/include/drm -g -O2 -fdebug-prefix-map=/__w/1/s=. > -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time > -D_FORTIFY_SOURCE=2 -Wmissing-prototypes -Wmissing-declarations > -Wwrite-strings -Wformat=2 -Wcast-function-type -Wformat-nonliteral > -Wdate-time -Wnested-externs -Wshadow -Wstrict-prototypes > -Wold-style-definition -Werror -Wredundant-decls -g -fPIC -std=gnu11 -MD > -MT libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o -MF > libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o.d -o > libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o -c ../libibverbs/cmd_flow.c > In file included from ../libibverbs/cmd_flow.c:33: > In file included from include/infiniband/cmd_write.h:36: > In file included from include/infiniband/cmd_ioctl.h:41: > In file included from include/infiniband/verbs.h:48: > In file included from include/infiniband/verbs_api.h:66: > In file included from include/infiniband/ib_user_ioctl_verbs.h:38: > include/rdma/ib_user_verbs.h:436:34: error: field 'base' with variable sized > type 'struct ib_uverbs_create_cq_resp' not at the end of a struct or class is > a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] > struct ib_uverbs_create_cq_resp base; > ^ > include/rdma/ib_user_verbs.h:644:34: error: field 'base' with variable sized > type 'struct ib_uverbs_create_qp_resp' not at the end of a struct or class is > a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] > struct ib_uverbs_create_qp_resp base; This looks very similar, a struct of unknown size is being treated as a header struct: struct ib_uverbs_create_cq_resp { __u32 cq_handle; __u32 cqe; __aligned_u64 driver_data[0]; }; struct ib_uverbs_ex_create_cq_resp { struct ib_uverbs_create_cq_resp base; __u32 comp_mask; __u32 response_length; }; And it only gets used here: DECLARE_UVERBS_WRITE(IB_USER_VERBS_CMD_CREATE_CQ, ib_uverbs_create_cq, UAPI_DEF_WRITE_UDATA_IO( struct ib_uverbs_create_cq, struct ib_uverbs_create_cq_resp), ^^^ UAPI_DEF_METHOD_NEEDS_FN(create_cq)), which must also be assuming it's a header. So probably better to just drop the driver_data field? I don't see anything using it (that I can find) besides as a sanity-check that the field exists and is at the end of the struct. -- Kees Cook
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Mon, Jun 27, 2022 at 12:53:43PM -0700, Stephen Hemminger wrote: > Thanks this fixes warning with gcc-12 in iproute2. > In function ‘xfrm_algo_parse’, > inlined from ‘xfrm_state_modify.constprop’ at xfrm_state.c:573:5: > xfrm_state.c:162:32: warning: writing 1 byte into a region of size 0 > [-Wstringop-overflow=] > 162 | buf[j] = val; > | ~~~^ Great! This gives me hope. :) Thanks -- Gustavo
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Tue, Jun 28, 2022 at 10:36:51AM -0300, Jason Gunthorpe wrote: > On Tue, Jun 28, 2022 at 04:21:29AM +0200, Gustavo A. R. Silva wrote: > > > > > Though maybe we could just switch off > > > > -Wgnu-variable-sized-type-not-at-end during configuration ? > > > We need to think in a different strategy. > > I think we will need to switch off the warning in userspace - this is > doable for rdma-core. > > On the other hand, if the goal is to enable the array size check > compiler warning I would suggest focusing only on those structs that > actually hit that warning in the kernel. IIRC infiniband doesn't > trigger it because it just pointer casts the flex array to some other > struct. Yep; this is actually why I reverted those changes in rdma (before sending out the patch) when 0-day reported the same problems you pointed out[1]. Also, that's the strategy I'm following right now with the one-element array into flex-array member transformations. I'm addressing those cases in which the trailing array is actually being iterated over, first. I just added the patch to my -next tree, so it can be build-tested by other people, and let's see what else is reported this week. :) -- Gustavo [1] https://lore.kernel.org/lkml/620ca2a5.nkaeidefiyoxe9%2fu%25...@intel.com/ > > It isn't actually an array it is a placeholder for a trailing > structure, so it is never indexed. > > This is also why we hit the warning because the convient way for > userspace to compose the message is to squash the header and trailer > structs together in a super struct on the stack, then invoke the > ioctl. > > Jason
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Tue, Jun 28, 2022 at 04:21:29AM +0200, Gustavo A. R. Silva wrote: > > > Though maybe we could just switch off > > > -Wgnu-variable-sized-type-not-at-end during configuration ? > We need to think in a different strategy. I think we will need to switch off the warning in userspace - this is doable for rdma-core. On the other hand, if the goal is to enable the array size check compiler warning I would suggest focusing only on those structs that actually hit that warning in the kernel. IIRC infiniband doesn't trigger it because it just pointer casts the flex array to some other struct. It isn't actually an array it is a placeholder for a trailing structure, so it is never indexed. This is also why we hit the warning because the convient way for userspace to compose the message is to squash the header and trailer structs together in a super struct on the stack, then invoke the ioctl. Jason
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
Hi Gustavo, Thanks for your patch! On Mon, Jun 27, 2022 at 8:04 PM Gustavo A. R. Silva wrote: > There is a regular need in the kernel to provide a way to declare > having a dynamically sized set of trailing elements in a structure. > Kernel code should always use “flexible array members”[1] for these > cases. The older style of one-element or zero-length arrays should > no longer be used[2]. These rules apply to the kernel, but uapi is not considered part of the kernel, so different rules apply. Uapi header files should work with whatever compiler that can be used for compiling userspace. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Mon, 27 Jun 2022 20:04:32 +0200 "Gustavo A. R. Silva" wrote: > There is a regular need in the kernel to provide a way to declare > having a dynamically sized set of trailing elements in a structure. > Kernel code should always use “flexible array members”[1] for these > cases. The older style of one-element or zero-length arrays should > no longer be used[2]. > > This code was transformed with the help of Coccinelle: > (linux-5.19-rc2$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file > script.cocci --include-headers --dir . > output.patch) > > @@ > identifier S, member, array; > type T1, T2; > @@ > > struct S { > ... > T1 member; > T2 array[ > - 0 > ]; > }; > > -fstrict-flex-arrays=3 is coming and we need to land these changes > to prevent issues like these in the short future: > > ../fs/minix/dir.c:337:3: warning: 'strcpy' will always overflow; destination > buffer has size 0, > but the source string has length 2 (including NUL byte) [-Wfortify-source] > strcpy(de3->name, "."); > ^ > > Since these are all [0] to [] changes, the risk to UAPI is nearly zero. If > this breaks anything, we can use a union with a new member name. > > [1] https://en.wikipedia.org/wiki/Flexible_array_member > [2] > https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays > > Link: https://github.com/KSPP/linux/issues/78 > Build-tested-by: > https://lore.kernel.org/lkml/62b675ec.wkx6aoz6cbe71vtf%25...@intel.com/ > Signed-off-by: Gustavo A. R. Silva Thanks this fixes warning with gcc-12 in iproute2. In function ‘xfrm_algo_parse’, inlined from ‘xfrm_state_modify.constprop’ at xfrm_state.c:573:5: xfrm_state.c:162:32: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=] 162 | buf[j] = val; | ~~~^
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Tue, Jun 28, 2022 at 02:58:25AM +0200, Gustavo A. R. Silva wrote: > On Mon, Jun 27, 2022 at 09:40:52PM -0300, Jason Gunthorpe wrote: > > On Mon, Jun 27, 2022 at 08:27:37PM +0200, Daniel Borkmann wrote: > > > On 6/27/22 8:04 PM, Gustavo A. R. Silva wrote: > > > > There is a regular need in the kernel to provide a way to declare > > > > having a dynamically sized set of trailing elements in a structure. > > > > Kernel code should always use “flexible array members”[1] for these > > > > cases. The older style of one-element or zero-length arrays should > > > > no longer be used[2]. > > > > > > > > This code was transformed with the help of Coccinelle: > > > > (linux-5.19-rc2$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file > > > > script.cocci --include-headers --dir . > output.patch) > > > > > > > > @@ > > > > identifier S, member, array; > > > > type T1, T2; > > > > @@ > > > > > > > > struct S { > > > >... > > > >T1 member; > > > >T2 array[ > > > > - 0 > > > >]; > > > > }; > > > > > > > > -fstrict-flex-arrays=3 is coming and we need to land these changes > > > > to prevent issues like these in the short future: > > > > > > > > ../fs/minix/dir.c:337:3: warning: 'strcpy' will always overflow; > > > > destination buffer has size 0, > > > > but the source string has length 2 (including NUL byte) > > > > [-Wfortify-source] > > > > strcpy(de3->name, "."); > > > > ^ > > > > > > > > Since these are all [0] to [] changes, the risk to UAPI is nearly zero. > > > > If > > > > this breaks anything, we can use a union with a new member name. > > > > > > > > [1] https://en.wikipedia.org/wiki/Flexible_array_member > > > > [2] > > > > https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays > > > > > > > > Link: https://github.com/KSPP/linux/issues/78 > > > > Build-tested-by: > > > > https://lore.kernel.org/lkml/62b675ec.wkx6aoz6cbe71vtf%25...@intel.com/ > > > > Signed-off-by: Gustavo A. R. Silva > > > > --- > > > > Hi all! > > > > > > > > JFYI: I'm adding this to my -next tree. :) > > > > > > Fyi, this breaks BPF CI: > > > > > > https://github.com/kernel-patches/bpf/runs/7078719372?check_suite_focus=true > > > > > > [...] > > > progs/map_ptr_kern.c:314:26: error: field 'trie_key' with variable > > > sized type 'struct bpf_lpm_trie_key' not at the end of a struct or class > > > is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] > > > struct bpf_lpm_trie_key trie_key; > > > ^ > > > > This will break the rdma-core userspace as well, with a similar > > error: > > > > /usr/bin/clang-13 -DVERBS_DEBUG -Dibverbs_EXPORTS -Iinclude > > -I/usr/include/libnl3 -I/usr/include/drm -g -O2 > > -fdebug-prefix-map=/__w/1/s=. -fstack-protector-strong -Wformat > > -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 > > -Wmissing-prototypes -Wmissing-declarations -Wwrite-strings -Wformat=2 > > -Wcast-function-type -Wformat-nonliteral -Wdate-time -Wnested-externs > > -Wshadow -Wstrict-prototypes -Wold-style-definition -Werror > > -Wredundant-decls -g -fPIC -std=gnu11 -MD -MT > > libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o -MF > > libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o.d -o > > libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o -c ../libibverbs/cmd_flow.c > > In file included from ../libibverbs/cmd_flow.c:33: > > In file included from include/infiniband/cmd_write.h:36: > > In file included from include/infiniband/cmd_ioctl.h:41: > > In file included from include/infiniband/verbs.h:48: > > In file included from include/infiniband/verbs_api.h:66: > > In file included from include/infiniband/ib_user_ioctl_verbs.h:38: > > include/rdma/ib_user_verbs.h:436:34: error: field 'base' with variable > > sized type 'struct ib_uverbs_create_cq_resp' not at the end of a struct or > > class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] > > struct ib_uverbs_create_cq_resp base; > > ^ > > include/rdma/ib_user_verbs.h:644:34: error: field 'base' with variable > > sized type 'struct ib_uverbs_create_qp_resp' not at the end of a struct or > > class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] > > struct ib_uverbs_create_qp_resp base; > > > > Which is why I gave up trying to change these.. > > > > Though maybe we could just switch off -Wgnu-variable-sized-type-not-at-end > > during configuration ? > > No. I think now we can easily workaround these sorts of problems with > something like this: > > struct flex { > any_type any_member; > union { > type array[0]; > __DECLARE_FLEX_ARRAY(type, array_flex); > }; > }; Mmmh... nope; this doesn't work[1]. We need to think in a different strategy. -- Gustavo [1] https://godbolt.org/z/av79Pqbfz
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Mon, Jun 27, 2022 at 09:40:52PM -0300, Jason Gunthorpe wrote: > On Mon, Jun 27, 2022 at 08:27:37PM +0200, Daniel Borkmann wrote: > > On 6/27/22 8:04 PM, Gustavo A. R. Silva wrote: > > > There is a regular need in the kernel to provide a way to declare > > > having a dynamically sized set of trailing elements in a structure. > > > Kernel code should always use “flexible array members”[1] for these > > > cases. The older style of one-element or zero-length arrays should > > > no longer be used[2]. > > > > > > This code was transformed with the help of Coccinelle: > > > (linux-5.19-rc2$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file > > > script.cocci --include-headers --dir . > output.patch) > > > > > > @@ > > > identifier S, member, array; > > > type T1, T2; > > > @@ > > > > > > struct S { > > >... > > >T1 member; > > >T2 array[ > > > - 0 > > >]; > > > }; > > > > > > -fstrict-flex-arrays=3 is coming and we need to land these changes > > > to prevent issues like these in the short future: > > > > > > ../fs/minix/dir.c:337:3: warning: 'strcpy' will always overflow; > > > destination buffer has size 0, > > > but the source string has length 2 (including NUL byte) [-Wfortify-source] > > > strcpy(de3->name, "."); > > > ^ > > > > > > Since these are all [0] to [] changes, the risk to UAPI is nearly zero. If > > > this breaks anything, we can use a union with a new member name. > > > > > > [1] https://en.wikipedia.org/wiki/Flexible_array_member > > > [2] > > > https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays > > > > > > Link: https://github.com/KSPP/linux/issues/78 > > > Build-tested-by: > > > https://lore.kernel.org/lkml/62b675ec.wkx6aoz6cbe71vtf%25...@intel.com/ > > > Signed-off-by: Gustavo A. R. Silva > > > --- > > > Hi all! > > > > > > JFYI: I'm adding this to my -next tree. :) > > > > Fyi, this breaks BPF CI: > > > > https://github.com/kernel-patches/bpf/runs/7078719372?check_suite_focus=true > > > > [...] > > progs/map_ptr_kern.c:314:26: error: field 'trie_key' with variable sized > > type 'struct bpf_lpm_trie_key' not at the end of a struct or class is a GNU > > extension [-Werror,-Wgnu-variable-sized-type-not-at-end] > > struct bpf_lpm_trie_key trie_key; > > ^ > > This will break the rdma-core userspace as well, with a similar > error: > > /usr/bin/clang-13 -DVERBS_DEBUG -Dibverbs_EXPORTS -Iinclude > -I/usr/include/libnl3 -I/usr/include/drm -g -O2 -fdebug-prefix-map=/__w/1/s=. > -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time > -D_FORTIFY_SOURCE=2 -Wmissing-prototypes -Wmissing-declarations > -Wwrite-strings -Wformat=2 -Wcast-function-type -Wformat-nonliteral > -Wdate-time -Wnested-externs -Wshadow -Wstrict-prototypes > -Wold-style-definition -Werror -Wredundant-decls -g -fPIC -std=gnu11 -MD > -MT libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o -MF > libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o.d -o > libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o -c ../libibverbs/cmd_flow.c > In file included from ../libibverbs/cmd_flow.c:33: > In file included from include/infiniband/cmd_write.h:36: > In file included from include/infiniband/cmd_ioctl.h:41: > In file included from include/infiniband/verbs.h:48: > In file included from include/infiniband/verbs_api.h:66: > In file included from include/infiniband/ib_user_ioctl_verbs.h:38: > include/rdma/ib_user_verbs.h:436:34: error: field 'base' with variable sized > type 'struct ib_uverbs_create_cq_resp' not at the end of a struct or class is > a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] > struct ib_uverbs_create_cq_resp base; > ^ > include/rdma/ib_user_verbs.h:644:34: error: field 'base' with variable sized > type 'struct ib_uverbs_create_qp_resp' not at the end of a struct or class is > a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] > struct ib_uverbs_create_qp_resp base; > > Which is why I gave up trying to change these.. > > Though maybe we could just switch off -Wgnu-variable-sized-type-not-at-end > during configuration ? No. I think now we can easily workaround these sorts of problems with something like this: struct flex { any_type any_member; union { type array[0]; __DECLARE_FLEX_ARRAY(type, array_flex); }; }; and use array_flex in kernel-space. The same for the one-elment arrays in UAPI: struct flex { any_type any_member; union { type array[1]; __DECLARE_FLEX_ARRAY(type, array_flex); }; }; I'll use the idiom above to resolve all these warnings in a follow-up patch. :) Thanks -- Gustavo
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Mon, Jun 27, 2022 at 08:27:37PM +0200, Daniel Borkmann wrote: > On 6/27/22 8:04 PM, Gustavo A. R. Silva wrote: > > There is a regular need in the kernel to provide a way to declare > > having a dynamically sized set of trailing elements in a structure. > > Kernel code should always use “flexible array members”[1] for these > > cases. The older style of one-element or zero-length arrays should > > no longer be used[2]. > > > > This code was transformed with the help of Coccinelle: > > (linux-5.19-rc2$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file > > script.cocci --include-headers --dir . > output.patch) > > > > @@ > > identifier S, member, array; > > type T1, T2; > > @@ > > > > struct S { > >... > >T1 member; > >T2 array[ > > - 0 > >]; > > }; > > > > -fstrict-flex-arrays=3 is coming and we need to land these changes > > to prevent issues like these in the short future: > > > > ../fs/minix/dir.c:337:3: warning: 'strcpy' will always overflow; > > destination buffer has size 0, > > but the source string has length 2 (including NUL byte) [-Wfortify-source] > > strcpy(de3->name, "."); > > ^ > > > > Since these are all [0] to [] changes, the risk to UAPI is nearly zero. If > > this breaks anything, we can use a union with a new member name. > > > > [1] https://en.wikipedia.org/wiki/Flexible_array_member > > [2] > > https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays > > > > Link: https://github.com/KSPP/linux/issues/78 > > Build-tested-by: > > https://lore.kernel.org/lkml/62b675ec.wkx6aoz6cbe71vtf%25...@intel.com/ > > Signed-off-by: Gustavo A. R. Silva > > --- > > Hi all! > > > > JFYI: I'm adding this to my -next tree. :) > > Fyi, this breaks BPF CI: > > https://github.com/kernel-patches/bpf/runs/7078719372?check_suite_focus=true > > [...] > progs/map_ptr_kern.c:314:26: error: field 'trie_key' with variable sized > type 'struct bpf_lpm_trie_key' not at the end of a struct or class is a GNU > extension [-Werror,-Wgnu-variable-sized-type-not-at-end] > struct bpf_lpm_trie_key trie_key; > ^ This will break the rdma-core userspace as well, with a similar error: /usr/bin/clang-13 -DVERBS_DEBUG -Dibverbs_EXPORTS -Iinclude -I/usr/include/libnl3 -I/usr/include/drm -g -O2 -fdebug-prefix-map=/__w/1/s=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -Wmissing-prototypes -Wmissing-declarations -Wwrite-strings -Wformat=2 -Wcast-function-type -Wformat-nonliteral -Wdate-time -Wnested-externs -Wshadow -Wstrict-prototypes -Wold-style-definition -Werror -Wredundant-decls -g -fPIC -std=gnu11 -MD -MT libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o -MF libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o.d -o libibverbs/CMakeFiles/ibverbs.dir/cmd_flow.c.o -c ../libibverbs/cmd_flow.c In file included from ../libibverbs/cmd_flow.c:33: In file included from include/infiniband/cmd_write.h:36: In file included from include/infiniband/cmd_ioctl.h:41: In file included from include/infiniband/verbs.h:48: In file included from include/infiniband/verbs_api.h:66: In file included from include/infiniband/ib_user_ioctl_verbs.h:38: include/rdma/ib_user_verbs.h:436:34: error: field 'base' with variable sized type 'struct ib_uverbs_create_cq_resp' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] struct ib_uverbs_create_cq_resp base; ^ include/rdma/ib_user_verbs.h:644:34: error: field 'base' with variable sized type 'struct ib_uverbs_create_qp_resp' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] struct ib_uverbs_create_qp_resp base; Which is why I gave up trying to change these.. Though maybe we could just switch off -Wgnu-variable-sized-type-not-at-end during configuration ? Jason
RE: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
Gustavo A. R. Silva wrote: > There is a regular need in the kernel to provide a way to declare > having a dynamically sized set of trailing elements in a structure. > Kernel code should always use “flexible array members”[1] for these > cases. The older style of one-element or zero-length arrays should > no longer be used[2]. > > This code was transformed with the help of Coccinelle: > (linux-5.19-rc2$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file > script.cocci --include-headers --dir . > output.patch) > > @@ > identifier S, member, array; > type T1, T2; > @@ > > struct S { > ... > T1 member; > T2 array[ > - 0 > ]; > }; > > -fstrict-flex-arrays=3 is coming and we need to land these changes > to prevent issues like these in the short future: > > ../fs/minix/dir.c:337:3: warning: 'strcpy' will always overflow; destination > buffer has size 0, > but the source string has length 2 (including NUL byte) [-Wfortify-source] > strcpy(de3->name, "."); > ^ > > Since these are all [0] to [] changes, the risk to UAPI is nearly zero. If > this breaks anything, we can use a union with a new member name. > > [1] https://en.wikipedia.org/wiki/Flexible_array_member > [2] > https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays > > Link: https://github.com/KSPP/linux/issues/78 > Build-tested-by: > https://lore.kernel.org/lkml/62b675ec.wkx6aoz6cbe71vtf%25...@intel.com/ > Signed-off-by: Gustavo A. R. Silva > --- > Hi all! > > JFYI: I'm adding this to my -next tree. :) > [..] > include/uapi/linux/ndctl.h| 10 +-- For ndctl.h Acked-by: Dan Williams
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On 6/27/22 8:04 PM, Gustavo A. R. Silva wrote: There is a regular need in the kernel to provide a way to declare having a dynamically sized set of trailing elements in a structure. Kernel code should always use “flexible array members”[1] for these cases. The older style of one-element or zero-length arrays should no longer be used[2]. This code was transformed with the help of Coccinelle: (linux-5.19-rc2$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file script.cocci --include-headers --dir . > output.patch) @@ identifier S, member, array; type T1, T2; @@ struct S { ... T1 member; T2 array[ - 0 ]; }; -fstrict-flex-arrays=3 is coming and we need to land these changes to prevent issues like these in the short future: ../fs/minix/dir.c:337:3: warning: 'strcpy' will always overflow; destination buffer has size 0, but the source string has length 2 (including NUL byte) [-Wfortify-source] strcpy(de3->name, "."); ^ Since these are all [0] to [] changes, the risk to UAPI is nearly zero. If this breaks anything, we can use a union with a new member name. [1] https://en.wikipedia.org/wiki/Flexible_array_member [2] https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays Link: https://github.com/KSPP/linux/issues/78 Build-tested-by: https://lore.kernel.org/lkml/62b675ec.wkx6aoz6cbe71vtf%25...@intel.com/ Signed-off-by: Gustavo A. R. Silva --- Hi all! JFYI: I'm adding this to my -next tree. :) Fyi, this breaks BPF CI: https://github.com/kernel-patches/bpf/runs/7078719372?check_suite_focus=true [...] progs/map_ptr_kern.c:314:26: error: field 'trie_key' with variable sized type 'struct bpf_lpm_trie_key' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end] struct bpf_lpm_trie_key trie_key; ^ 1 error generated. make: *** [Makefile:519: /tmp/runner/work/bpf/bpf/tools/testing/selftests/bpf/map_ptr_kern.o] Error 1 make: *** Waiting for unfinished jobs Error: Process completed with exit code 2.
Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members
On Mon, Jun 27, 2022 at 08:27:37PM +0200, Daniel Borkmann wrote: > On 6/27/22 8:04 PM, Gustavo A. R. Silva wrote: > > There is a regular need in the kernel to provide a way to declare > > having a dynamically sized set of trailing elements in a structure. > > Kernel code should always use “flexible array members”[1] for these > > cases. The older style of one-element or zero-length arrays should > > no longer be used[2]. > > > > This code was transformed with the help of Coccinelle: > > (linux-5.19-rc2$ spatch --jobs $(getconf _NPROCESSORS_ONLN) --sp-file > > script.cocci --include-headers --dir . > output.patch) > > > > @@ > > identifier S, member, array; > > type T1, T2; > > @@ > > > > struct S { > >... > >T1 member; > >T2 array[ > > - 0 > >]; > > }; > > > > -fstrict-flex-arrays=3 is coming and we need to land these changes > > to prevent issues like these in the short future: > > > > ../fs/minix/dir.c:337:3: warning: 'strcpy' will always overflow; > > destination buffer has size 0, > > but the source string has length 2 (including NUL byte) [-Wfortify-source] > > strcpy(de3->name, "."); > > ^ > > > > Since these are all [0] to [] changes, the risk to UAPI is nearly zero. If > > this breaks anything, we can use a union with a new member name. > > > > [1] https://en.wikipedia.org/wiki/Flexible_array_member > > [2] > > https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays > > > > Link: https://github.com/KSPP/linux/issues/78 > > Build-tested-by: > > https://lore.kernel.org/lkml/62b675ec.wkx6aoz6cbe71vtf%25...@intel.com/ > > Signed-off-by: Gustavo A. R. Silva > > --- > > Hi all! > > > > JFYI: I'm adding this to my -next tree. :) > > Fyi, this breaks BPF CI: Thanks for the report! It seems the 0-day robot didn't catch that one. I'll fix it up right away. :) -- Gustavo > > https://github.com/kernel-patches/bpf/runs/7078719372?check_suite_focus=true > > [...] > progs/map_ptr_kern.c:314:26: error: field 'trie_key' with variable sized > type 'struct bpf_lpm_trie_key' not at the end of a struct or class is a GNU > extension [-Werror,-Wgnu-variable-sized-type-not-at-end] > struct bpf_lpm_trie_key trie_key; > ^ > 1 error generated. > make: *** [Makefile:519: > /tmp/runner/work/bpf/bpf/tools/testing/selftests/bpf/map_ptr_kern.o] Error 1 > make: *** Waiting for unfinished jobs > Error: Process completed with exit code 2.