[PATCH] KVM: x86: Fix C++ user API for structures with variable length arrays

David Woodhouse posted 1 patch 1 month, 1 week ago
arch/x86/include/uapi/asm/kvm.h | 29 ++++++++++++++++++-----------
include/uapi/linux/kvm.h        |  9 ++++++---
2 files changed, 24 insertions(+), 14 deletions(-)
[PATCH] KVM: x86: Fix C++ user API for structures with variable length arrays
Posted by David Woodhouse 1 month, 1 week ago
From: David Woodhouse <dwmw@amazon.co.uk>

Commit 94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with
flexible-array members") broke the userspace API for C++. Not just in
the sense of 'userspace needs to be updated, but UAPI is supposed to be
stable", but broken in the sense that I can't actually see *how* the
structures can be used from C++ in the same way that they were usable
before.

These structures ending in VLAs are typically a *header*, which can be
followed by an arbitrary number of entries. Userspace typically creates
a larger structure with some non-zero number of entries, for example in
QEMU's kvm_arch_get_supported_msr_feature():

    struct {
        struct kvm_msrs info;
        struct kvm_msr_entry entries[1];
    } msr_data = {};

While that works in C, it fails in C++ with an error like:
 flexible array member ‘kvm_msrs::entries’ not at end of ‘struct msr_data’

Fix this by using __DECLARE_FLEX_ARRAY() for the VLA, which is a helper
provided by <linux/stddef.h> that already uses [0] for C++ compilation.

Also put the header fields into a struct_group() to provide (in C) a
separate struct (e.g 'struct kvm_msrs_hdr') without the trailing VLA.

Fixes: 94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with flexible-array members")

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 arch/x86/include/uapi/asm/kvm.h | 29 ++++++++++++++++++-----------
 include/uapi/linux/kvm.h        |  9 ++++++---
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 7ceff6583652..8fc0bc2d39a6 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -194,16 +194,19 @@ struct kvm_msr_entry {
 
 /* for KVM_GET_MSRS and KVM_SET_MSRS */
 struct kvm_msrs {
-	__u32 nmsrs; /* number of msrs in entries */
-	__u32 pad;
-
-	struct kvm_msr_entry entries[];
+	__struct_group(kvm_msrs_hdr, hdr, /* no attrs */,
+		__u32 nmsrs; /* number of msrs in entries */
+		__u32 pad;
+	);
+	__DECLARE_FLEX_ARRAY(struct kvm_msr_entry, entries);
 };
 
 /* for KVM_GET_MSR_INDEX_LIST */
 struct kvm_msr_list {
-	__u32 nmsrs; /* number of msrs in entries */
-	__u32 indices[];
+	__struct_group(kvm_msr_list_hdr, hdr, /* no attrs */,
+		__u32 nmsrs; /* number of msrs in entries */
+	);
+	__DECLARE_FLEX_ARRAY(__u32, indices);
 };
 
 /* Maximum size of any access bitmap in bytes */
@@ -265,9 +268,11 @@ struct kvm_cpuid_entry2 {
 
 /* for KVM_SET_CPUID2 */
 struct kvm_cpuid2 {
-	__u32 nent;
-	__u32 padding;
-	struct kvm_cpuid_entry2 entries[];
+	__struct_group(kvm_cpuid2_hdr, hdr, /* no attrs */,
+		__u32 nent;
+		__u32 padding;
+	);
+	__DECLARE_FLEX_ARRAY(struct kvm_cpuid_entry2, entries);
 };
 
 /* for KVM_GET_PIT and KVM_SET_PIT */
@@ -397,8 +402,10 @@ struct kvm_xsave {
 	 * The offsets of the state save areas in struct kvm_xsave follow
 	 * the contents of CPUID leaf 0xD on the host.
 	 */
-	__u32 region[1024];
-	__u32 extra[];
+	__struct_group(kvm_xsave_hdr, hdr, /* no attrs */,
+		__u32 region[1024];
+	);
+	__DECLARE_FLEX_ARRAY(__u32, extra);
 };
 
 #define KVM_MAX_XCRS	16
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index dddb781b0507..2849d32ba035 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -11,6 +11,7 @@
 #include <linux/const.h>
 #include <linux/types.h>
 #include <linux/compiler.h>
+#include <linux/stddef.h>
 #include <linux/ioctl.h>
 #include <asm/kvm.h>
 
@@ -1034,9 +1035,11 @@ struct kvm_irq_routing_entry {
 };
 
 struct kvm_irq_routing {
-	__u32 nr;
-	__u32 flags;
-	struct kvm_irq_routing_entry entries[];
+	__struct_group(kvm_irq_routing_hdr, hdr, /* no attrs */,
+		__u32 nr;
+		__u32 flags;
+	);
+	__DECLARE_FLEX_ARRAY(struct kvm_irq_routing_entry, entries);
 };
 
 #define KVM_IRQFD_FLAG_DEASSIGN (1 << 0)
-- 
2.43.0


Re: [PATCH] KVM: x86: Fix C++ user API for structures with variable length arrays
Posted by Sean Christopherson 1 month ago
On Thu, Feb 26, 2026, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> Commit 94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with
> flexible-array members") broke the userspace API for C++. Not just in
> the sense of 'userspace needs to be updated, but UAPI is supposed to be
> stable", but broken in the sense that I can't actually see *how* the
> structures can be used from C++ in the same way that they were usable
> before.
> 
> These structures ending in VLAs are typically a *header*, which can be
> followed by an arbitrary number of entries. Userspace typically creates
> a larger structure with some non-zero number of entries, for example in
> QEMU's kvm_arch_get_supported_msr_feature():
> 
>     struct {
>         struct kvm_msrs info;
>         struct kvm_msr_entry entries[1];
>     } msr_data = {};
> 
> While that works in C, it fails in C++ with an error like:
>  flexible array member ‘kvm_msrs::entries’ not at end of ‘struct msr_data’
> 
> Fix this by using __DECLARE_FLEX_ARRAY() for the VLA, which is a helper
> provided by <linux/stddef.h> that already uses [0] for C++ compilation.
> 
> Also put the header fields into a struct_group() to provide (in C) a
> separate struct (e.g 'struct kvm_msrs_hdr') without the trailing VLA.

Unless I'm missing something, this is an entirely optional change that needs to
be done separately, especialy since I want to tag this for:

  Cc: stable@vger.kernel.org

I definitely don't hate the __struct_group definitions, but I don't know that I
love them either as they make the code a bit harder to read, and more importantly
there's a non-zero chance that defining the new structurs could break userspace
builds and force an update, e.g. if userspace already concocts its own header
overlay, which would be very unpleasant for a stable@ patch.

If we do define headers, I think I'd want a wrapper around __struct_group() to
prettify the common case and force consistent naming, e.g.

#define kvm_struct_header(NAME, MEMBERS...)				\
	__struct_group(NAME ##_header, h, /* no attrs */, MEMBERS)

struct kvm_msrs {
	kvm_struct_header(kvm_msrs,
		__u32 nmsrs; /* number of msrs in entries */
		__u32 pad;
	);

	__DECLARE_FLEX_ARRAY(struct kvm_msr_entry, entries);
};

But that's likely going to lead to some amount of bikeshedding, e.g. arguably
kvm_header() would be sufficient and easier on the eyes.  Which is all the more
reason to handle it separately.

> Fixes: 94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with flexible-array members")
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  arch/x86/include/uapi/asm/kvm.h | 29 ++++++++++++++++++-----------
>  include/uapi/linux/kvm.h        |  9 ++++++---
>  /* for KVM_GET_PIT and KVM_SET_PIT */
> @@ -397,8 +402,10 @@ struct kvm_xsave {
>  	 * The offsets of the state save areas in struct kvm_xsave follow
>  	 * the contents of CPUID leaf 0xD on the host.
>  	 */
> -	__u32 region[1024];
> -	__u32 extra[];
> +	__struct_group(kvm_xsave_hdr, hdr, /* no attrs */,
> +		__u32 region[1024];
> +	);

This is *very* misleading, as XSTATE itself has a header, but this is something
else entirely (just the always-allocated region).

> +	__DECLARE_FLEX_ARRAY(__u32, extra);
>  };

There are several structs that got missed:

  kvm_pmu_event_filter
  kvm_reg_list
  kvm_signal_mask
  kvm_coalesced_mmio_ring
  kvm_cpuid
  kvm_stats_desc
Re: [PATCH] KVM: x86: Fix C++ user API for structures with variable length arrays
Posted by David Woodhouse 1 month ago
On Thu, 2026-03-05 at 10:36 -0800, Sean Christopherson wrote:
> On Thu, Feb 26, 2026, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@amazon.co.uk>
> > 
> > Commit 94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with
> > flexible-array members") broke the userspace API for C++. Not just in
> > the sense of 'userspace needs to be updated, but UAPI is supposed to be
> > stable", but broken in the sense that I can't actually see *how* the
> > structures can be used from C++ in the same way that they were usable
> > before.
> > 
> > These structures ending in VLAs are typically a *header*, which can be
> > followed by an arbitrary number of entries. Userspace typically creates
> > a larger structure with some non-zero number of entries, for example in
> > QEMU's kvm_arch_get_supported_msr_feature():
> > 
> >     struct {
> >         struct kvm_msrs info;
> >         struct kvm_msr_entry entries[1];
> >     } msr_data = {};
> > 
> > While that works in C, it fails in C++ with an error like:
> >  flexible array member ‘kvm_msrs::entries’ not at end of ‘struct msr_data’
> > 
> > Fix this by using __DECLARE_FLEX_ARRAY() for the VLA, which is a helper
> > provided by <linux/stddef.h> that already uses [0] for C++ compilation.
> > 
> > Also put the header fields into a struct_group() to provide (in C) a
> > separate struct (e.g 'struct kvm_msrs_hdr') without the trailing VLA.
> 
> Unless I'm missing something, this is an entirely optional change that needs to
> be done separately, especialy since I want to tag this for:
> 
>   Cc: stable@vger.kernel.org
> 
> I definitely don't hate the __struct_group definitions, but I don't know that I
> love them either as they make the code a bit harder to read, and more importantly
> there's a non-zero chance that defining the new structurs could break userspace
> builds and force an update, e.g. if userspace already concocts its own header
> overlay, which would be very unpleasant for a stable@ patch.
> 
> If we do define headers, I think I'd want a wrapper around __struct_group() to
> prettify the common case and force consistent naming, e.g.
> 
> #define kvm_struct_header(NAME, MEMBERS...)				\
> 	__struct_group(NAME ##_header, h, /* no attrs */, MEMBERS)
> 
> struct kvm_msrs {
> 	kvm_struct_header(kvm_msrs,
> 		__u32 nmsrs; /* number of msrs in entries */
> 		__u32 pad;
> 	);
> 
> 	__DECLARE_FLEX_ARRAY(struct kvm_msr_entry, entries);
> };
> 
> But that's likely going to lead to some amount of bikeshedding, e.g. arguably
> kvm_header() would be sufficient and easier on the eyes.  Which is all the more
> reason to handle it separately.
> 
> > Fixes: 94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with flexible-array members")
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > ---
> >  arch/x86/include/uapi/asm/kvm.h | 29 ++++++++++++++++++-----------
> >  include/uapi/linux/kvm.h        |  9 ++++++---
> >  /* for KVM_GET_PIT and KVM_SET_PIT */
> > @@ -397,8 +402,10 @@ struct kvm_xsave {
> >  	 * The offsets of the state save areas in struct kvm_xsave follow
> >  	 * the contents of CPUID leaf 0xD on the host.
> >  	 */
> > -	__u32 region[1024];
> > -	__u32 extra[];
> > +	__struct_group(kvm_xsave_hdr, hdr, /* no attrs */,
> > +		__u32 region[1024];
> > +	);
> 
> This is *very* misleading, as XSTATE itself has a header, but this is something
> else entirely (just the always-allocated region).
> 
> > +	__DECLARE_FLEX_ARRAY(__u32, extra);
> >  };
> 
> There are several structs that got missed:
> 
>   kvm_pmu_event_filter
>   kvm_reg_list
>   kvm_signal_mask
>   kvm_coalesced_mmio_ring
>   kvm_cpuid
>   kvm_stats_desc

Ack. Shall we do just the __DECLARE_FLEX_ARRAY() part, including those
missed structures? 

Re: [PATCH] KVM: x86: Fix C++ user API for structures with variable length arrays
Posted by Sean Christopherson 1 month ago
On Thu, Mar 05, 2026, David Woodhouse wrote:
> On Thu, 2026-03-05 at 10:36 -0800, Sean Christopherson wrote:
> > > +	__DECLARE_FLEX_ARRAY(__u32, extra);
> > >  };
> > 
> > There are several structs that got missed:
> > 
> >   kvm_pmu_event_filter
> >   kvm_reg_list
> >   kvm_signal_mask
> >   kvm_coalesced_mmio_ring
> >   kvm_cpuid
> >   kvm_stats_desc
> 
> Ack. Shall we do just the __DECLARE_FLEX_ARRAY() part, including those
> missed structures? 

Ya, can you send a v2?
[PATCH v2] KVM: x86: Use __DECLARE_FLEX_ARRAY() for UAPI structures with VLAs
Posted by David Woodhouse 1 month ago
From: David Woodhouse <dwmw@amazon.co.uk>

Commit 94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with
flexible-array members") broke the userspace API for C++.

These structures ending in VLAs are typically a *header*, which can be
followed by an arbitrary number of entries. Userspace typically creates
a larger structure with some non-zero number of entries, for example in
QEMU's kvm_arch_get_supported_msr_feature():

    struct {
        struct kvm_msrs info;
        struct kvm_msr_entry entries[1];
    } msr_data = {};

While that works in C, it fails in C++ with an error like:
 flexible array member 'kvm_msrs::entries' not at end of 'struct msr_data'

Fix this by using __DECLARE_FLEX_ARRAY() for the VLA, which uses [0]
for C++ compilation.

Fixes: 94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with flexible-array members")
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 arch/x86/include/uapi/asm/kvm.h | 12 ++++++------
 include/uapi/linux/kvm.h        | 11 ++++++-----
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 7ceff6583652..d467e56408ef 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -197,13 +197,13 @@ struct kvm_msrs {
 	__u32 nmsrs; /* number of msrs in entries */
 	__u32 pad;
 
-	struct kvm_msr_entry entries[];
+	__DECLARE_FLEX_ARRAY(struct kvm_msr_entry, entries);
 };
 
 /* for KVM_GET_MSR_INDEX_LIST */
 struct kvm_msr_list {
 	__u32 nmsrs; /* number of msrs in entries */
-	__u32 indices[];
+	__DECLARE_FLEX_ARRAY(__u32, indices);
 };
 
 /* Maximum size of any access bitmap in bytes */
@@ -245,7 +245,7 @@ struct kvm_cpuid_entry {
 struct kvm_cpuid {
 	__u32 nent;
 	__u32 padding;
-	struct kvm_cpuid_entry entries[];
+	__DECLARE_FLEX_ARRAY(struct kvm_cpuid_entry, entries);
 };
 
 struct kvm_cpuid_entry2 {
@@ -267,7 +267,7 @@ struct kvm_cpuid_entry2 {
 struct kvm_cpuid2 {
 	__u32 nent;
 	__u32 padding;
-	struct kvm_cpuid_entry2 entries[];
+	__DECLARE_FLEX_ARRAY(struct kvm_cpuid_entry2, entries);
 };
 
 /* for KVM_GET_PIT and KVM_SET_PIT */
@@ -398,7 +398,7 @@ struct kvm_xsave {
 	 * the contents of CPUID leaf 0xD on the host.
 	 */
 	__u32 region[1024];
-	__u32 extra[];
+	__DECLARE_FLEX_ARRAY(__u32, extra);
 };
 
 #define KVM_MAX_XCRS	16
@@ -564,7 +564,7 @@ struct kvm_pmu_event_filter {
 	__u32 fixed_counter_bitmap;
 	__u32 flags;
 	__u32 pad[4];
-	__u64 events[];
+	__DECLARE_FLEX_ARRAY(__u64, events);
 };
 
 #define KVM_PMU_EVENT_ALLOW 0
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index dddb781b0507..3e7b55e5fa1e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -11,6 +11,7 @@
 #include <linux/const.h>
 #include <linux/types.h>
 #include <linux/compiler.h>
+#include <linux/stddef.h>
 #include <linux/ioctl.h>
 #include <asm/kvm.h>
 
@@ -528,7 +529,7 @@ struct kvm_coalesced_mmio {
 
 struct kvm_coalesced_mmio_ring {
 	__u32 first, last;
-	struct kvm_coalesced_mmio coalesced_mmio[];
+	__DECLARE_FLEX_ARRAY(struct kvm_coalesced_mmio, coalesced_mmio);
 };
 
 #define KVM_COALESCED_MMIO_MAX \
@@ -578,7 +579,7 @@ struct kvm_clear_dirty_log {
 /* for KVM_SET_SIGNAL_MASK */
 struct kvm_signal_mask {
 	__u32 len;
-	__u8  sigset[];
+	__DECLARE_FLEX_ARRAY(__u8, sigset);
 };
 
 /* for KVM_TPR_ACCESS_REPORTING */
@@ -1036,7 +1037,7 @@ struct kvm_irq_routing_entry {
 struct kvm_irq_routing {
 	__u32 nr;
 	__u32 flags;
-	struct kvm_irq_routing_entry entries[];
+	__DECLARE_FLEX_ARRAY(struct kvm_irq_routing_entry, entries);
 };
 
 #define KVM_IRQFD_FLAG_DEASSIGN (1 << 0)
@@ -1127,7 +1128,7 @@ struct kvm_dirty_tlb {
 
 struct kvm_reg_list {
 	__u64 n; /* number of regs */
-	__u64 reg[];
+	__DECLARE_FLEX_ARRAY(__u64, reg);
 };
 
 struct kvm_one_reg {
@@ -1579,7 +1580,7 @@ struct kvm_stats_desc {
 	__u16 size;
 	__u32 offset;
 	__u32 bucket_size;
-	char name[];
+	__DECLARE_FLEX_ARRAY(char, name);
 };
 
 #define KVM_GET_STATS_FD  _IO(KVMIO,  0xce)
-- 
2.43.0


Re: [PATCH v2] KVM: x86: Use __DECLARE_FLEX_ARRAY() for UAPI structures with VLAs
Posted by Sean Christopherson 3 days, 22 hours ago
On Thu, 05 Mar 2026 20:49:55 +0100, David Woodhouse wrote:
> Commit 94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with
> flexible-array members") broke the userspace API for C++.
> 
> These structures ending in VLAs are typically a *header*, which can be
> followed by an arbitrary number of entries. Userspace typically creates
> a larger structure with some non-zero number of entries, for example in
> QEMU's kvm_arch_get_supported_msr_feature():
> 
> [...]

Applied to kvm-x86 fixes, way back on March 12th (i.e. this has been in -next
for several weeks).  Thanks!

[1/1] KVM: x86: Use __DECLARE_FLEX_ARRAY() for UAPI structures with VLAs
      https://github.com/kvm-x86/linux/commit/2619da73bb2f

--
https://github.com/kvm-x86/linux/tree/next
Re: [PATCH] KVM: x86: Fix C++ user API for structures with variable length arrays
Posted by Kees Cook 1 month, 1 week ago
On Thu, Feb 26, 2026 at 11:44:21AM +0000, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> Commit 94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with
> flexible-array members") broke the userspace API for C++. Not just in
> the sense of 'userspace needs to be updated, but UAPI is supposed to be
> stable", but broken in the sense that I can't actually see *how* the
> structures can be used from C++ in the same way that they were usable
> before.
> 
> These structures ending in VLAs are typically a *header*, which can be
> followed by an arbitrary number of entries. Userspace typically creates
> a larger structure with some non-zero number of entries, for example in
> QEMU's kvm_arch_get_supported_msr_feature():
> 
>     struct {
>         struct kvm_msrs info;
>         struct kvm_msr_entry entries[1];
>     } msr_data = {};
> 
> While that works in C, it fails in C++ with an error like:
>  flexible array member ‘kvm_msrs::entries’ not at end of ‘struct msr_data’
> 
> Fix this by using __DECLARE_FLEX_ARRAY() for the VLA, which is a helper
> provided by <linux/stddef.h> that already uses [0] for C++ compilation.

This is likely the best plan for these cases. I had to do similar for
ACPICA upstream, leaving these flex arrays as [0] for the non-GCC (and
Clang) builds:
https://github.com/acpica/acpica/commit/e73b227e8e475c20cc394f237ea35d592fdf9ec3

> Also put the header fields into a struct_group() to provide (in C) a
> separate struct (e.g 'struct kvm_msrs_hdr') without the trailing VLA.

Right, my only worry is if C++ would want those header structs too. In
that case, you'd probably want to use a macro to include them (since not
all compilers are supporting transparent struct members yet):

#define __kvm_msrs_hdr	\
	__u32 nmsrs; /* number of msrs in entries */	\
	__u32 pad

struct kvm_msrs_hdr {
	__kvm_msrs_hdr;
};

struct kvm_msrs {
	__kvm_msrs_hdr;
	__DECLARE_FLEX_ARRAY(struct kvm_msr_entry, entries);
};

> Fixes: 94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with flexible-array members")
> 
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>

Regardless:

Reviewed-by: Kees Cook <kees@kernel.org>


-- 
Kees Cook
Re: [PATCH] KVM: x86: Fix C++ user API for structures with variable length arrays
Posted by David Woodhouse 1 month, 1 week ago
On Thu, 2026-02-26 at 11:02 -0800, Kees Cook wrote:
> 
> > Also put the header fields into a struct_group() to provide (in C) a
> > separate struct (e.g 'struct kvm_msrs_hdr') without the trailing VLA.
> 
> Right, my only worry is if C++ would want those header structs too. In
> that case, you'd probably want to use a macro to include them (since not
> all compilers are supporting transparent struct members yet):
> 
> #define __kvm_msrs_hdr	\
> 	__u32 nmsrs; /* number of msrs in entries */	\
> 	__u32 pad
> 
> struct kvm_msrs_hdr {
> 	__kvm_msrs_hdr;
> };
> 
> struct kvm_msrs {
> 	__kvm_msrs_hdr;
> 	__DECLARE_FLEX_ARRAY(struct kvm_msr_entry, entries);
> };

Hm, does the struct_group() not also depend on the compiler supporting
anonymous struct members? Is there a distinction I'm missing?
Re: [PATCH] KVM: x86: Fix C++ user API for structures with variable length arrays
Posted by Kees Cook 1 month, 1 week ago

On February 27, 2026 12:29:11 AM PST, David Woodhouse <dwmw2@infradead.org> wrote:
>On Thu, 2026-02-26 at 11:02 -0800, Kees Cook wrote:
>> 
>> > Also put the header fields into a struct_group() to provide (in C) a
>> > separate struct (e.g 'struct kvm_msrs_hdr') without the trailing VLA.
>> 
>> Right, my only worry is if C++ would want those header structs too. In
>> that case, you'd probably want to use a macro to include them (since not
>> all compilers are supporting transparent struct members yet):
>> 
>> #define __kvm_msrs_hdr	\
>> 	__u32 nmsrs; /* number of msrs in entries */	\
>> 	__u32 pad
>> 
>> struct kvm_msrs_hdr {
>> 	__kvm_msrs_hdr;
>> };
>> 
>> struct kvm_msrs {
>> 	__kvm_msrs_hdr;
>> 	__DECLARE_FLEX_ARRAY(struct kvm_msr_entry, entries);
>> };
>
>Hm, does the struct_group() not also depend on the compiler supporting
>anonymous struct members? Is there a distinction I'm missing?

Sorry, I should have been mor clear: the _tag_ that struct_group creates isn't supported for all C++ compilers (so is commented out for C++) so if a C++ project wanted to use the struct by its tag ("struct kvm_msrs_hdr"), you'd need the construct I wrote above.

-Kees

-- 
Kees Cook
Re: [PATCH] KVM: x86: Fix C++ user API for structures with variable length arrays
Posted by David Woodhouse 1 month, 1 week ago
On Fri, 2026-02-27 at 16:43 -0800, Kees Cook wrote:
> 
> 
> On February 27, 2026 12:29:11 AM PST, David Woodhouse <dwmw2@infradead.org> wrote:
> > On Thu, 2026-02-26 at 11:02 -0800, Kees Cook wrote:
> > > 
> > > > Also put the header fields into a struct_group() to provide (in C) a
> > > > separate struct (e.g 'struct kvm_msrs_hdr') without the trailing VLA.
> > > 
> > > Right, my only worry is if C++ would want those header structs too. In
> > > that case, you'd probably want to use a macro to include them (since not
> > > all compilers are supporting transparent struct members yet):
> > > 
> > > #define __kvm_msrs_hdr	\
> > > 	__u32 nmsrs; /* number of msrs in entries */	\
> > > 	__u32 pad
> > > 
> > > struct kvm_msrs_hdr {
> > > 	__kvm_msrs_hdr;
> > > };
> > > 
> > > struct kvm_msrs {
> > > 	__kvm_msrs_hdr;
> > > 	__DECLARE_FLEX_ARRAY(struct kvm_msr_entry, entries);
> > > };
> > 
> > Hm, does the struct_group() not also depend on the compiler supporting
> > anonymous struct members? Is there a distinction I'm missing?
> 
> Sorry, I should have been mor clear: the _tag_ that struct_group
> creates isn't supported for all C++ compilers (so is commented out
> for C++) so if a C++ project wanted to use the struct by its tag
> ("struct kvm_msrs_hdr"), you'd need the construct I wrote above.
> 

Understood, yes. I was actually getting confused about why we'd use
that #define __kvm_msrs_hdr, and why we wouldn't do it like this:

 struct kvm_msr_hdr {
   __u32 nmsrs;
   __u32 pad;
 }

 struct kvm_msrs {
    struct kvm_msr_hdr /* anonymous */;
    __DECLARE_FLEX_ARRAY(struct kvm_msr_entry, entries);
 };

(In fact, that doesn't even work with GCC). Nevertheless, my question
at the time was intended as: if we can't do it like *that*, then surely
the struct_group() construct will fail on those compilers too?

But that was a brain fart because what struct_group() does is actually
more like:

 struct kvm_msrs {
    union {
       struct { __u32 nmsrs; __u32 pad; };
       struct /* foo in C */ { __u32 nmsrs; __u32 pad; } foo;
    };

    __DECLARE_FLEX_ARRAY(struct kvm_msr_entry, entries);
 };

And that *does* work.

But yeah, I think C++ should be fine without the new header structs.
What I posted at least allows it to *build* with actual instantiations
of these structures with real payload again.