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

["Gustavo A. R. Silva" <gustavo@embeddedor.com>]

On 2/23/26 23:28, David Woodhouse wrote:
> On 2022-07-28 at 10:54:58 -0700, Kees Cook <keescook@chromium.org> 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 {
>        |        ^~~~~~~~
>