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

[David Woodhouse]

On Thu, 2026-03-05 at 10:36 -0800, Sean Christopherson wrote:
> On Thu, Feb 26, 2026, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@xxxxxxxxxxxx>
> > 
> > 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@xxxxxxxxxxxxxxx
> 
> 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@xxxxxxxxxxxx>
> > ---
> >  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?

Attachment: smime.p7s
Description: S/MIME cryptographic signature