Re: [PATCH][next] treewide: uapi: Replace zero-length arrays with flexible-array members

[Gustavo A. R. Silva]

On 2/23/26 23:28, David Woodhouse wrote:
> On 2022-07-28 at 10:54:58 -0700, Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> > 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 */
> >          __u8    data[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.
>
> What was the eventual conclusion here?
>
> These changes broke userspace compilation, because a *lot* of these
> structures ending with VLAs are very much used as headers.
>
> QEMU does it, for example...
>
> uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
> {
>      struct {
>          struct kvm_msrs info;
>          struct kvm_msr_entry entries[1];
>      } msr_data = {};
>
>
> ... which works in C but not in C++, which gives an error I can't work
> out how to avoid.
>
> Am I missing something, or did we break our userspace headers for C++
> and make them C-only?
>
>   $ cat test_kvm.cpp
> #include <cstdlib>
> #include <asm/kvm.h>
>
> struct msr_data {
> 	struct kvm_msrs info;
> 	struct kvm_msr_entry entries[1];
> };

Something like this should eventually land:

diff --git a/tools/arch/x86/include/uapi/asm/kvm.h b/tools/arch/x86/include/uapi/asm/kvm.h
index 7ceff6583652..295b2a54b25d 100644
--- a/tools/arch/x86/include/uapi/asm/kvm.h
+++ b/tools/arch/x86/include/uapi/asm/kvm.h
@@ -192,11 +192,14 @@ struct kvm_msr_entry {
        __u64 data;
 };

-/* for KVM_GET_MSRS and KVM_SET_MSRS */
-struct kvm_msrs {
+struct kvm_msrs_hdr {
        __u32 nmsrs; /* number of msrs in entries */
        __u32 pad;
+};

+/* for KVM_GET_MSRS and KVM_SET_MSRS */
+struct kvm_msrs {
+       struct kvm_msrs_hdr;
        struct kvm_msr_entry entries[];
 };

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index fdec90e85467..a354ee1ef359 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -261,6 +261,8 @@ CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \
        -fno-builtin-memcmp -fno-builtin-memcpy \
        -fno-builtin-memset -fno-builtin-strnlen \
        -fno-stack-protector -fno-PIE -fno-strict-aliasing \
+       -Wno-microsoft-anon-tag \
+       -fms-extensions \
        -I$(LINUX_TOOL_INCLUDE) -I$(LINUX_TOOL_ARCH_INCLUDE) \
        -I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(ARCH) \
        -I ../rseq -I.. $(EXTRA_CFLAGS) $(KHDR_INCLUDES)
diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index fab18e9be66c..0f881ee8c37c 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -1076,7 +1076,7 @@ uint64_t vcpu_get_msr(struct kvm_vcpu *vcpu, uint64_t msr_index)
 int _vcpu_set_msr(struct kvm_vcpu *vcpu, uint64_t msr_index, uint64_t msr_value)
 {
        struct {
-               struct kvm_msrs header;
+               struct kvm_msrs_hdr header;
                struct kvm_msr_entry entry;
        } buffer = {};


(patch above untested)

-Gustavo

> struct msr_data *alloc_msr_data(void)
> {
> 	return static_cast<struct msr_data *>(malloc(sizeof(struct msr_data)));
> }
>
>   $ g++    -c -o test_kvm.o test_kvm.cpp
> In file included from test_kvm.cpp:2:
> /usr/include/x86_64-linux-gnu/asm/kvm.h:194:30: error: flexible array member ‘kvm_msrs::entries’ not at end of ‘struct msr_data’
>    194 |         struct kvm_msr_entry entries[];
>        |                              ^~~~~~~
> test_kvm.cpp:6:30: note: next member ‘kvm_msr_entry msr_data::entries [1]’ declared here
>      6 |         struct kvm_msr_entry entries[1];
>        |                              ^~~~~~~
> test_kvm.cpp:4:8: note: in the definition of ‘struct msr_data’
>      4 | struct msr_data {
>        |        ^~~~~~~~