Re: [PATCH v15 5/7] vfio-pci/zdev: Add a device feature for error information

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] UAPI structure `vfio_device_feature_zpci_err` uses non-portable bitfields and contains implicit padding, violating UAPI design guidelines.
--

commit 9302e2545ccb75aab62ae8f60f8aba469ebfdf04
Author: Farhan Ali <alifm@linux.ibm.com>

vfio-pci/zdev: Add a device feature for error information

This commit adds a new device feature to expose zPCI platform specific error
information to userspace. This allows userspace to take appropriate actions to
handle an error on the device and perform mediated recovery.

> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 5de618a3a5eee..3eead86a00ab2 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h

[ ... ]

> +struct vfio_device_feature_zpci_err {
> +	__u32 feature_flags;		/* Indicate future features */
> +	__u32 reserved1;
> +	__u32 fh;			/* function handle */
> +	__u32 fid;			/* function id */
> +	__u32 ett		:  4;	/* expected table type */
> +	__u32 mvn		: 12;	/* MSI vector number */
> +	__u32 dmaas		:  8;	/* DMA address space */
> +	__u32 reserved2		:  6;
> +	__u32 q			:  1;	/* event qualifier */
> +	__u32 rw		:  1;	/* read/write */

Since the memory layout, ordering, and packing of C bitfields are highly
compiler-dependent, could this compromise cross-platform ABI stability or
break compatibility with other languages interacting with the kernel?

Using standard __u32 fields and bitmask macros instead might be a safer
approach for a UAPI header.

> +	__u64 faddr;			/* failing address */

The faddr member requires 8-byte alignment, but it appears to be positioned
after exactly 20 bytes of prior fields. Does this force 4 bytes of implicit
compiler padding before it?

> +	__u32 reserved3;
> +	__u16 reserved4;
> +	__u16 pec;			/* PCI event code */
> +	__u8 reserved5[28];		/* Allow for future expansion */
> +};

The explicit data size of this structure appears to be 68 bytes, which
would force another 4 bytes of implicit trailing padding to align the 72-byte
structure.

Could this implicit padding make ioctl size validation fragile?

Additionally, if the compiler fails to zero the implicit padding during
struct initialization with err = {}, is there a risk that copy_to_user()
will leak uninitialized kernel stack memory to userspace?

Using explicit reserved padding fields to ensure manual, deterministic 8-byte
alignment could avoid these issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260505200510.2954-1-alifm@linux.ibm.com?part=5