[PATCH 1/2] vduse: avoid adding implicit padding

[PATCH 1/2] vduse: avoid adding implicit padding

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The vduse_iova_range_v2 and vduse_iotlb_entry_v2 structures are both
defined in a way that adds implicit padding and is incompatible between
i386 and x86_64 userspace because of the different structure alignment
requirements. Building the header with -Wpadded shows these new warnings:

vduse.h:305:1: error: padding struct size to alignment boundary with 4 bytes [-Werror=padded]
vduse.h:374:1: error: padding struct size to alignment boundary with 4 bytes [-Werror=padded]

Change the amount of padding in these two structures to align them to
64 bit words and avoid those problems. Since the v1 vduse_iotlb_entry
already has an inconsistent size, do not attempt to reuse the structure
but rather list the members indiviudally, with a fixed amount of
padding.

Fixes: 079212f6877e ("vduse: add vq group asid support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 40 +++++++++++-------------------
 include/uapi/linux/vduse.h         |  9 +++++--
 2 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 73d1d517dc6c..405d59610f76 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1301,7 +1301,7 @@ static int vduse_dev_iotlb_entry(struct vduse_dev *dev,
 	int r = -EINVAL;
 	struct vhost_iotlb_map *map;
 
-	if (entry->v1.start > entry->v1.last || entry->asid >= dev->nas)
+	if (entry->start > entry->last || entry->asid >= dev->nas)
 		return -EINVAL;
 
 	asid = array_index_nospec(entry->asid, dev->nas);
@@ -1312,18 +1312,18 @@ static int vduse_dev_iotlb_entry(struct vduse_dev *dev,
 
 	spin_lock(&dev->as[asid].domain->iotlb_lock);
 	map = vhost_iotlb_itree_first(dev->as[asid].domain->iotlb,
-				      entry->v1.start, entry->v1.last);
+				      entry->start, entry->last);
 	if (map) {
 		if (f) {
 			const struct vdpa_map_file *map_file;
 
 			map_file = (struct vdpa_map_file *)map->opaque;
-			entry->v1.offset = map_file->offset;
+			entry->offset = map_file->offset;
 			*f = get_file(map_file->file);
 		}
-		entry->v1.start = map->start;
-		entry->v1.last = map->last;
-		entry->v1.perm = map->perm;
+		entry->start = map->start;
+		entry->last = map->last;
+		entry->perm = map->perm;
 		if (capability) {
 			*capability = 0;
 
@@ -1363,14 +1363,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 			break;
 
 		ret = -EFAULT;
-		if (cmd == VDUSE_IOTLB_GET_FD2) {
-			if (copy_from_user(&entry, argp, sizeof(entry)))
-				break;
-		} else {
-			if (copy_from_user(&entry.v1, argp,
-					   sizeof(entry.v1)))
-				break;
-		}
+		if (copy_from_user(&entry, argp, _IOC_SIZE(cmd)))
+			break;
 
 		ret = -EINVAL;
 		if (!is_mem_zero((const char *)entry.reserved,
@@ -1385,19 +1379,13 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 		if (!f)
 			break;
 
-		if (cmd == VDUSE_IOTLB_GET_FD2)
-			ret = copy_to_user(argp, &entry,
-					   sizeof(entry));
-		else
-			ret = copy_to_user(argp, &entry.v1,
-					   sizeof(entry.v1));
-
+		ret = copy_to_user(argp, &entry, _IOC_SIZE(cmd));
 		if (ret) {
 			ret = -EFAULT;
 			fput(f);
 			break;
 		}
-		ret = receive_fd(f, NULL, perm_to_file_flags(entry.v1.perm));
+		ret = receive_fd(f, NULL, perm_to_file_flags(entry.perm));
 		fput(f);
 		break;
 	}
@@ -1603,16 +1591,16 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 		} else if (info.asid >= dev->nas)
 			break;
 
-		entry.v1.start = info.start;
-		entry.v1.last = info.last;
+		entry.start = info.start;
+		entry.last = info.last;
 		entry.asid = info.asid;
 		ret = vduse_dev_iotlb_entry(dev, &entry, NULL,
 					    &info.capability);
 		if (ret < 0)
 			break;
 
-		info.start = entry.v1.start;
-		info.last = entry.v1.last;
+		info.start = entry.start;
+		info.last = entry.last;
 		info.asid = entry.asid;
 
 		ret = -EFAULT;
diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
index faae7718bd2e..deca8c7b9178 100644
--- a/include/uapi/linux/vduse.h
+++ b/include/uapi/linux/vduse.h
@@ -299,9 +299,13 @@ struct vduse_iova_info {
  * Structure used by VDUSE_IOTLB_GET_FD2 ioctl to find an overlapped IOVA region.
  */
 struct vduse_iotlb_entry_v2 {
-	struct vduse_iotlb_entry v1;
+	__u64 offset;
+	__u64 start;
+	__u64 last;
+	__u8 perm;
+	__u8 padding[7];
 	__u32 asid;
-	__u32 reserved[12];
+	__u32 reserved[11];
 };
 
 /*
@@ -371,6 +375,7 @@ struct vduse_iova_range_v2 {
 	__u64 start;
 	__u64 last;
 	__u32 asid;
+	__u32 padding;
 };
 
 /**
-- 
2.39.5

[PATCH 2/2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

These two ioctls are incompatible on 32-bit x86 userspace, because
the data structures are shorter than they are on 64-bit.

Add compad handling to the regular ioctl handler to just handle
them the same way and ignore the extra padding. This could be
done in a separate .compat_ioctl handler, but the main one already
handles two versions of VDUSE_IOTLB_GET_FD, so adding a third one
fits in rather well.

Fixes: ad146355bfad ("vduse: Support querying information of IOVA regions")
Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 43 +++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 405d59610f76..39cbff2f379d 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1341,6 +1341,37 @@ static int vduse_dev_iotlb_entry(struct vduse_dev *dev,
 	return r;
 }
 
+#if defined(CONFIG_X86_64) && defined(CONFIG_COMPAT)
+/*
+ * i386 has different alignment constraints than x86_64,
+ * so there are only 3 bytes of padding instead of 7.
+ */
+struct compat_vduse_iotlb_entry {
+	compat_u64 offset;
+	compat_u64 start;
+	compat_u64 last;
+	__u8 perm;
+	__u8 padding[__alignof__(compat_u64) - 1];
+};
+#define COMPAT_VDUSE_IOTLB_GET_FD	_IOWR(VDUSE_BASE, 0x10, struct compat_vduse_iotlb_entry)
+
+struct compat_vduse_vq_info {
+	__u32 index;
+	__u32 num;
+	compat_u64 desc_addr;
+	compat_u64 driver_addr;
+	compat_u64 device_addr;
+	union {
+		struct vduse_vq_state_split split;
+		struct vduse_vq_state_packed packed;
+	};
+	__u8 ready;
+	__u8 padding[__alignof__(compat_u64) - 1];
+} __uapi_arch_align;
+#define COMPAT_VDUSE_VQ_GET_INFO	_IOWR(VDUSE_BASE, 0x15, struct compat_vduse_vq_info)
+
+#endif
+
 static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 			    unsigned long arg)
 {
@@ -1352,6 +1383,9 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 		return -EPERM;
 
 	switch (cmd) {
+#if defined(CONFIG_X86_64) && defined(CONFIG_COMPAT)
+	case COMPAT_VDUSE_IOTLB_GET_FD:
+#endif
 	case VDUSE_IOTLB_GET_FD:
 	case VDUSE_IOTLB_GET_FD2: {
 		struct vduse_iotlb_entry_v2 entry = {0};
@@ -1455,13 +1489,16 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 		ret = 0;
 		break;
 	}
+#if defined(CONFIG_X86_64) && defined(CONFIG_COMPAT)
+	case COMPAT_VDUSE_VQ_GET_INFO:
+#endif
 	case VDUSE_VQ_GET_INFO: {
-		struct vduse_vq_info vq_info;
+		struct vduse_vq_info vq_info = {};
 		struct vduse_virtqueue *vq;
 		u32 index;
 
 		ret = -EFAULT;
-		if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
+		if (copy_from_user(&vq_info, argp, _IOC_SIZE(cmd)))
 			break;
 
 		ret = -EINVAL;
@@ -1491,7 +1528,7 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 		vq_info.ready = vq->ready;
 
 		ret = -EFAULT;
-		if (copy_to_user(argp, &vq_info, sizeof(vq_info)))
+		if (copy_to_user(argp, &vq_info, _IOC_SIZE(cmd)))
 			break;
 
 		ret = 0;
-- 
2.39.5

Re: [PATCH 2/2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO

[Eugenio Perez Martin]

On Mon, Feb 2, 2026 at 11:07 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> These two ioctls are incompatible on 32-bit x86 userspace, because
> the data structures are shorter than they are on 64-bit.
>
> Add compad handling to the regular ioctl handler to just handle
> them the same way and ignore the extra padding. This could be
> done in a separate .compat_ioctl handler, but the main one already
> handles two versions of VDUSE_IOTLB_GET_FD, so adding a third one
> fits in rather well.
>

I'm just learning about the COMPAT_ stuff but does this mean the
userland app need to call a different ioctl depending if it is
compiled for 32 bits or 64 bits? I guess it is not the case, but how
is that handled?

> Fixes: ad146355bfad ("vduse: Support querying information of IOVA regions")
> Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 43 +++++++++++++++++++++++++++---
>  1 file changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 405d59610f76..39cbff2f379d 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1341,6 +1341,37 @@ static int vduse_dev_iotlb_entry(struct vduse_dev *dev,
>         return r;
>  }
>
> +#if defined(CONFIG_X86_64) && defined(CONFIG_COMPAT)
> +/*
> + * i386 has different alignment constraints than x86_64,
> + * so there are only 3 bytes of padding instead of 7.
> + */
> +struct compat_vduse_iotlb_entry {
> +       compat_u64 offset;
> +       compat_u64 start;
> +       compat_u64 last;
> +       __u8 perm;
> +       __u8 padding[__alignof__(compat_u64) - 1];
> +};
> +#define COMPAT_VDUSE_IOTLB_GET_FD      _IOWR(VDUSE_BASE, 0x10, struct compat_vduse_iotlb_entry)
> +
> +struct compat_vduse_vq_info {
> +       __u32 index;
> +       __u32 num;
> +       compat_u64 desc_addr;
> +       compat_u64 driver_addr;
> +       compat_u64 device_addr;
> +       union {
> +               struct vduse_vq_state_split split;
> +               struct vduse_vq_state_packed packed;
> +       };
> +       __u8 ready;
> +       __u8 padding[__alignof__(compat_u64) - 1];
> +} __uapi_arch_align;
> +#define COMPAT_VDUSE_VQ_GET_INFO       _IOWR(VDUSE_BASE, 0x15, struct compat_vduse_vq_info)
> +
> +#endif
> +
>  static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>                             unsigned long arg)
>  {
> @@ -1352,6 +1383,9 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>                 return -EPERM;
>
>         switch (cmd) {
> +#if defined(CONFIG_X86_64) && defined(CONFIG_COMPAT)
> +       case COMPAT_VDUSE_IOTLB_GET_FD:
> +#endif
>         case VDUSE_IOTLB_GET_FD:
>         case VDUSE_IOTLB_GET_FD2: {
>                 struct vduse_iotlb_entry_v2 entry = {0};
> @@ -1455,13 +1489,16 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>                 ret = 0;
>                 break;
>         }
> +#if defined(CONFIG_X86_64) && defined(CONFIG_COMPAT)
> +       case COMPAT_VDUSE_VQ_GET_INFO:
> +#endif
>         case VDUSE_VQ_GET_INFO: {
> -               struct vduse_vq_info vq_info;
> +               struct vduse_vq_info vq_info = {};
>                 struct vduse_virtqueue *vq;
>                 u32 index;
>
>                 ret = -EFAULT;
> -               if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
> +               if (copy_from_user(&vq_info, argp, _IOC_SIZE(cmd)))
>                         break;
>
>                 ret = -EINVAL;
> @@ -1491,7 +1528,7 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>                 vq_info.ready = vq->ready;
>
>                 ret = -EFAULT;
> -               if (copy_to_user(argp, &vq_info, sizeof(vq_info)))
> +               if (copy_to_user(argp, &vq_info, _IOC_SIZE(cmd)))
>                         break;
>
>                 ret = 0;
> --
> 2.39.5
>
>

Re: [PATCH 2/2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO

[Arnd Bergmann]

On Mon, Feb 2, 2026, at 12:34, Eugenio Perez Martin wrote:
> On Mon, Feb 2, 2026 at 11:07 AM Arnd Bergmann <arnd@kernel.org> wrote:
>>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> These two ioctls are incompatible on 32-bit x86 userspace, because
>> the data structures are shorter than they are on 64-bit.
>>
>> Add compad handling to the regular ioctl handler to just handle
>> them the same way and ignore the extra padding. This could be
>> done in a separate .compat_ioctl handler, but the main one already
>> handles two versions of VDUSE_IOTLB_GET_FD, so adding a third one
>> fits in rather well.
>>
>
> I'm just learning about the COMPAT_ stuff but does this mean the
> userland app need to call a different ioctl depending if it is
> compiled for 32 bits or 64 bits? I guess it is not the case, but how
> is that handled?

In a definition like

#define VDUSE_IOTLB_GET_FD      _IOWR(VDUSE_BASE, 0x10, struct vduse_iotlb_entry)

The resulting integer value encodes sizeof(struct vduse_iotlb_entry)
in some of the bits. Since x86-32 and x86-64 have different
sizes for this particular structure, the command codes are
different for the same macro. The recommendation from 
Documentation/driver-api/ioctl.rst is to use structures with
a consistent layout across all architectures to avoid that.

The normal way to handle this once it has gone wrong is to split
out the actual handler into a function that takes the kernel
structure, and a .compat_ioctl() handler that copies the
32-bit structure to the stack in the correct format.

Since the v1 structures here are /almost/ compatible aside from
the padding at the end, my patch here takes a shortcut and does
not add a custom .compat_ioctl handler but instead changes
the native version on x86-64 to deal with both layouts.
This does mean that the kernel driver now also accepts the
64-bit layout coming from compat tasks, and the compat layout
coming from 64-bit tasks.

Nothing in userspace changes, as it still just uses the existing
VDUSE_IOTLB_GET_FD macro, and the kernel continues to handle
the native layout as before.

    Arnd

Re: [PATCH 2/2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO

[Michael S. Tsirkin]

On Mon, Feb 02, 2026 at 12:59:03PM +0100, Arnd Bergmann wrote:
> On Mon, Feb 2, 2026, at 12:34, Eugenio Perez Martin wrote:
> > On Mon, Feb 2, 2026 at 11:07 AM Arnd Bergmann <arnd@kernel.org> wrote:
> >>
> >> From: Arnd Bergmann <arnd@arndb.de>
> >>
> >> These two ioctls are incompatible on 32-bit x86 userspace, because
> >> the data structures are shorter than they are on 64-bit.
> >>
> >> Add compad handling to the regular ioctl handler to just handle
> >> them the same way and ignore the extra padding. This could be
> >> done in a separate .compat_ioctl handler, but the main one already
> >> handles two versions of VDUSE_IOTLB_GET_FD, so adding a third one
> >> fits in rather well.
> >>
> >
> > I'm just learning about the COMPAT_ stuff but does this mean the
> > userland app need to call a different ioctl depending if it is
> > compiled for 32 bits or 64 bits? I guess it is not the case, but how
> > is that handled?
> 
> In a definition like
> 
> #define VDUSE_IOTLB_GET_FD      _IOWR(VDUSE_BASE, 0x10, struct vduse_iotlb_entry)
> 
> The resulting integer value encodes sizeof(struct vduse_iotlb_entry)
> in some of the bits. Since x86-32 and x86-64 have different
> sizes for this particular structure, the command codes are
> different for the same macro. The recommendation from 
> Documentation/driver-api/ioctl.rst is to use structures with
> a consistent layout across all architectures to avoid that.
> 
> The normal way to handle this once it has gone wrong is to split
> out the actual handler into a function that takes the kernel
> structure, and a .compat_ioctl() handler that copies the
> 32-bit structure to the stack in the correct format.
> 
> Since the v1 structures here are /almost/ compatible aside from
> the padding at the end, my patch here takes a shortcut and does
> not add a custom .compat_ioctl handler but instead changes
> the native version on x86-64 to deal with both layouts.
> This does mean that the kernel driver now also accepts the
> 64-bit layout coming from compat tasks, and the compat layout
> coming from 64-bit tasks.
> 
> Nothing in userspace changes, as it still just uses the existing
> VDUSE_IOTLB_GET_FD macro, and the kernel continues to handle
> the native layout as before.
> 
>     Arnd

I think .compat_ioctl would be cleaner frankly. Just look at
all the ifdefery. And who knows what broken-ness userspace
comes up with with this approach. Better use the standard approach.

Re: [PATCH 2/2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO

[Arnd Bergmann]

On Mon, Feb 2, 2026, at 17:45, Michael S. Tsirkin wrote:
> On Mon, Feb 02, 2026 at 12:59:03PM +0100, Arnd Bergmann wrote:
>
> I think .compat_ioctl would be cleaner frankly. Just look at
> all the ifdefery. And who knows what broken-ness userspace
> comes up with with this approach. Better use the standard approach.

Sent now.

I'm not sure it's much better because there is quite a bit of
code duplication, and reducing that would be a larger rework.

It may be best to hold off on patch 2 for the coming merge window
since the compat ioctl code has apparently always been broken for
x86 here.

I hope we can at least get patch 1/2 merged along with the
new code though, otherwise it would get a lot harder to sort
it out properly, with the v2 struct members overlapping the
old padding fields.

     Arnd

Re: [PATCH 2/2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO

[Michael S. Tsirkin]

On Mon, Feb 02, 2026 at 11:54:17PM +0100, Arnd Bergmann wrote:
> On Mon, Feb 2, 2026, at 17:45, Michael S. Tsirkin wrote:
> > On Mon, Feb 02, 2026 at 12:59:03PM +0100, Arnd Bergmann wrote:
> >
> > I think .compat_ioctl would be cleaner frankly. Just look at
> > all the ifdefery. And who knows what broken-ness userspace
> > comes up with with this approach. Better use the standard approach.
> 
> Sent now.
> 
> I'm not sure it's much better because there is quite a bit of
> code duplication, and reducing that would be a larger rework.

yes but on the flip side, we can put it all inside ifdef CONFIG_COMPAT
(which this code did not do, but should IMHO).

> It may be best to hold off on patch 2 for the coming merge window
> since the compat ioctl code has apparently always been broken for
> x86 here.

And it needs testing.

> I hope we can at least get patch 1/2 merged along with the
> new code though, otherwise it would get a lot harder to sort
> it out properly, with the v2 struct members overlapping the
> old padding fields.
> 
>      Arnd

Along with it or no, surely before the release.
Given 32 on 64 with this apparently has been broken forever,
I will merge this just based on even you did not bother testing compat, I am
inclined to say I am merging this but not rebasing because
of this.

Oh and we got lucky this didn't leak kernel stack info.

Eugenio, note for the future: please help make sure UAPI
structs do not have hidden padding.

-- 
MST

Re: [PATCH 2/2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO

[Eugenio Perez Martin]

On Tue, Feb 3, 2026 at 11:35 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Feb 02, 2026 at 11:54:17PM +0100, Arnd Bergmann wrote:
> > On Mon, Feb 2, 2026, at 17:45, Michael S. Tsirkin wrote:
> > > On Mon, Feb 02, 2026 at 12:59:03PM +0100, Arnd Bergmann wrote:
> > >
> > > I think .compat_ioctl would be cleaner frankly. Just look at
> > > all the ifdefery. And who knows what broken-ness userspace
> > > comes up with with this approach. Better use the standard approach.
> >
> > Sent now.
> >
> > I'm not sure it's much better because there is quite a bit of
> > code duplication, and reducing that would be a larger rework.
>
> yes but on the flip side, we can put it all inside ifdef CONFIG_COMPAT
> (which this code did not do, but should IMHO).
>
> > It may be best to hold off on patch 2 for the coming merge window
> > since the compat ioctl code has apparently always been broken for
> > x86 here.
>
> And it needs testing.
>
> > I hope we can at least get patch 1/2 merged along with the
> > new code though, otherwise it would get a lot harder to sort
> > it out properly, with the v2 struct members overlapping the
> > old padding fields.
> >
> >      Arnd
>
> Along with it or no, surely before the release.
> Given 32 on 64 with this apparently has been broken forever,
> I will merge this just based on even you did not bother testing compat, I am
> inclined to say I am merging this but not rebasing because
> of this.
>
> Oh and we got lucky this didn't leak kernel stack info.
>
> Eugenio, note for the future: please help make sure UAPI
> structs do not have hidden padding.
>

Sure. I'm trying to find an automatic way to check for this but with
no luck :(. Arnd, did you use some tool for this or you just found it
by visual inspection? I'm trying pahole and -Wpadded but the output
includes a lot of struct not related to uapi.

I guess it is possible to filter it with some clang or awk machinery,
but I'm asking in case I avoid developing something already existing.

Thanks!

Re: [PATCH 2/2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO

[Arnd Bergmann]

On Tue, Feb 3, 2026, at 15:13, Eugenio Perez Martin wrote:
> On Tue, Feb 3, 2026 at 11:35 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>> Along with it or no, surely before the release.
>> Given 32 on 64 with this apparently has been broken forever,
>> I will merge this just based on even you did not bother testing compat, I am
>> inclined to say I am merging this but not rebasing because
>> of this.
>>
>> Oh and we got lucky this didn't leak kernel stack info.
>>
>> Eugenio, note for the future: please help make sure UAPI
>> structs do not have hidden padding.
>>
>
> Sure. I'm trying to find an automatic way to check for this but with
> no luck :(. Arnd, did you use some tool for this or you just found it
> by visual inspection? I'm trying pahole and -Wpadded but the output
> includes a lot of struct not related to uapi.
>
> I guess it is possible to filter it with some clang or awk machinery,
> but I'm asking in case I avoid developing something already existing.

I have the patch series, but it's not quite ready for submission.
In total, I annotated around 10% of the structure definitions
(500 files) in include/uapi/ to shut up -Wpadded for all the
existing structures with holes in them.

In the end I turn on the check with

--- a/usr/include/Makefile
+++ b/usr/include/Makefile
@@ -6,7 +6,7 @@
 #
 # -std=c90 (equivalent to -ansi) catches the violation of those.
 # We cannot go as far as adding -Wpedantic since it emits too many warnings.
-UAPI_CFLAGS := -std=c90 -Werror=implicit-function-declaration
+UAPI_CFLAGS := -std=c90 -Werror=implicit-function-declaration -Werror=padded
 
 # when cross-compiling with a minimal toolchain, use nolibc headers
 UAPI_CFLAGS += -I$(srctree)/tools/include/nolibc/

which of course warns for all the existing holes. I will continue
to send fixes for new instances in the meantime, as I'm testing
linux-next.

      Arnd

Re: [PATCH 2/2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO

[Eugenio Perez Martin]

On Tue, Feb 3, 2026 at 3:42 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Feb 3, 2026, at 15:13, Eugenio Perez Martin wrote:
> > On Tue, Feb 3, 2026 at 11:35 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >> Along with it or no, surely before the release.
> >> Given 32 on 64 with this apparently has been broken forever,
> >> I will merge this just based on even you did not bother testing compat, I am
> >> inclined to say I am merging this but not rebasing because
> >> of this.
> >>
> >> Oh and we got lucky this didn't leak kernel stack info.
> >>
> >> Eugenio, note for the future: please help make sure UAPI
> >> structs do not have hidden padding.
> >>
> >
> > Sure. I'm trying to find an automatic way to check for this but with
> > no luck :(. Arnd, did you use some tool for this or you just found it
> > by visual inspection? I'm trying pahole and -Wpadded but the output
> > includes a lot of struct not related to uapi.
> >
> > I guess it is possible to filter it with some clang or awk machinery,
> > but I'm asking in case I avoid developing something already existing.
>
> I have the patch series, but it's not quite ready for submission.
> In total, I annotated around 10% of the structure definitions
> (500 files) in include/uapi/ to shut up -Wpadded for all the
> existing structures with holes in them.
>
> In the end I turn on the check with
>
> --- a/usr/include/Makefile
> +++ b/usr/include/Makefile
> @@ -6,7 +6,7 @@
>  #
>  # -std=c90 (equivalent to -ansi) catches the violation of those.
>  # We cannot go as far as adding -Wpedantic since it emits too many warnings.
> -UAPI_CFLAGS := -std=c90 -Werror=implicit-function-declaration
> +UAPI_CFLAGS := -std=c90 -Werror=implicit-function-declaration -Werror=padded
>
>  # when cross-compiling with a minimal toolchain, use nolibc headers
>  UAPI_CFLAGS += -I$(srctree)/tools/include/nolibc/
>
> which of course warns for all the existing holes. I will continue
> to send fixes for new instances in the meantime, as I'm testing
> linux-next.
>

Got it, to develop something in parallel would be to duplicate work
then. Looking forward to it then, thank you very much!

Re: [PATCH 1/2] vduse: avoid adding implicit padding

[Eugenio Perez Martin]

On Mon, Feb 2, 2026 at 11:06 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> The vduse_iova_range_v2 and vduse_iotlb_entry_v2 structures are both
> defined in a way that adds implicit padding and is incompatible between
> i386 and x86_64 userspace because of the different structure alignment
> requirements. Building the header with -Wpadded shows these new warnings:
>
> vduse.h:305:1: error: padding struct size to alignment boundary with 4 bytes [-Werror=padded]
> vduse.h:374:1: error: padding struct size to alignment boundary with 4 bytes [-Werror=padded]
>
> Change the amount of padding in these two structures to align them to
> 64 bit words and avoid those problems. Since the v1 vduse_iotlb_entry
> already has an inconsistent size, do not attempt to reuse the structure
> but rather list the members indiviudally, with a fixed amount of

s/indiviudally/individually/ if v2

> padding.
>

That's something I didn't take into account, thanks!

> Fixes: 079212f6877e ("vduse: add vq group asid support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 40 +++++++++++-------------------
>  include/uapi/linux/vduse.h         |  9 +++++--
>  2 files changed, 21 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 73d1d517dc6c..405d59610f76 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1301,7 +1301,7 @@ static int vduse_dev_iotlb_entry(struct vduse_dev *dev,
>         int r = -EINVAL;
>         struct vhost_iotlb_map *map;
>
> -       if (entry->v1.start > entry->v1.last || entry->asid >= dev->nas)
> +       if (entry->start > entry->last || entry->asid >= dev->nas)
>                 return -EINVAL;
>
>         asid = array_index_nospec(entry->asid, dev->nas);
> @@ -1312,18 +1312,18 @@ static int vduse_dev_iotlb_entry(struct vduse_dev *dev,
>
>         spin_lock(&dev->as[asid].domain->iotlb_lock);
>         map = vhost_iotlb_itree_first(dev->as[asid].domain->iotlb,
> -                                     entry->v1.start, entry->v1.last);
> +                                     entry->start, entry->last);
>         if (map) {
>                 if (f) {
>                         const struct vdpa_map_file *map_file;
>
>                         map_file = (struct vdpa_map_file *)map->opaque;
> -                       entry->v1.offset = map_file->offset;
> +                       entry->offset = map_file->offset;
>                         *f = get_file(map_file->file);
>                 }
> -               entry->v1.start = map->start;
> -               entry->v1.last = map->last;
> -               entry->v1.perm = map->perm;
> +               entry->start = map->start;
> +               entry->last = map->last;
> +               entry->perm = map->perm;
>                 if (capability) {
>                         *capability = 0;
>
> @@ -1363,14 +1363,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>                         break;
>
>                 ret = -EFAULT;
> -               if (cmd == VDUSE_IOTLB_GET_FD2) {
> -                       if (copy_from_user(&entry, argp, sizeof(entry)))
> -                               break;
> -               } else {
> -                       if (copy_from_user(&entry.v1, argp,
> -                                          sizeof(entry.v1)))
> -                               break;
> -               }
> +               if (copy_from_user(&entry, argp, _IOC_SIZE(cmd)))

I did not know about _IOC_SIZE and I like how it reduces the complexity, thanks!

As a proposal, maybe we can add MIN(_IOC_SIZE, sizeof(entry)) ? Not
sure if it is too much boilerplate for nothing as the compiler should
make the code identical and the uapi ioctl part should never change.
But it seems to me future changes to the code are better tied with the
MIN.

I'm ok with not including MIN() either way.

> +                       break;
>
>                 ret = -EINVAL;
>                 if (!is_mem_zero((const char *)entry.reserved,
> @@ -1385,19 +1379,13 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>                 if (!f)
>                         break;
>
> -               if (cmd == VDUSE_IOTLB_GET_FD2)
> -                       ret = copy_to_user(argp, &entry,
> -                                          sizeof(entry));
> -               else
> -                       ret = copy_to_user(argp, &entry.v1,
> -                                          sizeof(entry.v1));
> -
> +               ret = copy_to_user(argp, &entry, _IOC_SIZE(cmd));
>                 if (ret) {
>                         ret = -EFAULT;
>                         fput(f);
>                         break;
>                 }
> -               ret = receive_fd(f, NULL, perm_to_file_flags(entry.v1.perm));
> +               ret = receive_fd(f, NULL, perm_to_file_flags(entry.perm));
>                 fput(f);
>                 break;
>         }
> @@ -1603,16 +1591,16 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>                 } else if (info.asid >= dev->nas)
>                         break;
>
> -               entry.v1.start = info.start;
> -               entry.v1.last = info.last;
> +               entry.start = info.start;
> +               entry.last = info.last;
>                 entry.asid = info.asid;
>                 ret = vduse_dev_iotlb_entry(dev, &entry, NULL,
>                                             &info.capability);
>                 if (ret < 0)
>                         break;
>
> -               info.start = entry.v1.start;
> -               info.last = entry.v1.last;
> +               info.start = entry.start;
> +               info.last = entry.last;
>                 info.asid = entry.asid;
>
>                 ret = -EFAULT;
> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> index faae7718bd2e..deca8c7b9178 100644
> --- a/include/uapi/linux/vduse.h
> +++ b/include/uapi/linux/vduse.h
> @@ -299,9 +299,13 @@ struct vduse_iova_info {
>   * Structure used by VDUSE_IOTLB_GET_FD2 ioctl to find an overlapped IOVA region.
>   */
>  struct vduse_iotlb_entry_v2 {
> -       struct vduse_iotlb_entry v1;
> +       __u64 offset;
> +       __u64 start;
> +       __u64 last;
> +       __u8 perm;
> +       __u8 padding[7];
>         __u32 asid;
> -       __u32 reserved[12];
> +       __u32 reserved[11];
>  };
>
>  /*
> @@ -371,6 +375,7 @@ struct vduse_iova_range_v2 {
>         __u64 start;
>         __u64 last;
>         __u32 asid;
> +       __u32 padding;
>  };
>

Re: [PATCH 1/2] vduse: avoid adding implicit padding

[Eugenio Perez Martin]

On Mon, Feb 2, 2026 at 12:28 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Mon, Feb 2, 2026 at 11:06 AM Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > The vduse_iova_range_v2 and vduse_iotlb_entry_v2 structures are both
> > defined in a way that adds implicit padding and is incompatible between
> > i386 and x86_64 userspace because of the different structure alignment
> > requirements. Building the header with -Wpadded shows these new warnings:
> >
> > vduse.h:305:1: error: padding struct size to alignment boundary with 4 bytes [-Werror=padded]
> > vduse.h:374:1: error: padding struct size to alignment boundary with 4 bytes [-Werror=padded]
> >
> > Change the amount of padding in these two structures to align them to
> > 64 bit words and avoid those problems. Since the v1 vduse_iotlb_entry
> > already has an inconsistent size, do not attempt to reuse the structure
> > but rather list the members indiviudally, with a fixed amount of
>
> s/indiviudally/individually/ if v2
>
> > padding.
> >
>
> That's something I didn't take into account, thanks!
>
> > Fixes: 079212f6877e ("vduse: add vq group asid support")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/vdpa/vdpa_user/vduse_dev.c | 40 +++++++++++-------------------
> >  include/uapi/linux/vduse.h         |  9 +++++--
> >  2 files changed, 21 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index 73d1d517dc6c..405d59610f76 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -1301,7 +1301,7 @@ static int vduse_dev_iotlb_entry(struct vduse_dev *dev,
> >         int r = -EINVAL;
> >         struct vhost_iotlb_map *map;
> >
> > -       if (entry->v1.start > entry->v1.last || entry->asid >= dev->nas)
> > +       if (entry->start > entry->last || entry->asid >= dev->nas)
> >                 return -EINVAL;
> >
> >         asid = array_index_nospec(entry->asid, dev->nas);
> > @@ -1312,18 +1312,18 @@ static int vduse_dev_iotlb_entry(struct vduse_dev *dev,
> >
> >         spin_lock(&dev->as[asid].domain->iotlb_lock);
> >         map = vhost_iotlb_itree_first(dev->as[asid].domain->iotlb,
> > -                                     entry->v1.start, entry->v1.last);
> > +                                     entry->start, entry->last);
> >         if (map) {
> >                 if (f) {
> >                         const struct vdpa_map_file *map_file;
> >
> >                         map_file = (struct vdpa_map_file *)map->opaque;
> > -                       entry->v1.offset = map_file->offset;
> > +                       entry->offset = map_file->offset;
> >                         *f = get_file(map_file->file);
> >                 }
> > -               entry->v1.start = map->start;
> > -               entry->v1.last = map->last;
> > -               entry->v1.perm = map->perm;
> > +               entry->start = map->start;
> > +               entry->last = map->last;
> > +               entry->perm = map->perm;
> >                 if (capability) {
> >                         *capability = 0;
> >
> > @@ -1363,14 +1363,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> >                         break;
> >
> >                 ret = -EFAULT;
> > -               if (cmd == VDUSE_IOTLB_GET_FD2) {
> > -                       if (copy_from_user(&entry, argp, sizeof(entry)))
> > -                               break;
> > -               } else {
> > -                       if (copy_from_user(&entry.v1, argp,
> > -                                          sizeof(entry.v1)))
> > -                               break;
> > -               }
> > +               if (copy_from_user(&entry, argp, _IOC_SIZE(cmd)))
>
> I did not know about _IOC_SIZE and I like how it reduces the complexity, thanks!
>
> As a proposal, maybe we can add MIN(_IOC_SIZE, sizeof(entry)) ? Not
> sure if it is too much boilerplate for nothing as the compiler should
> make the code identical and the uapi ioctl part should never change.
> But it seems to me future changes to the code are better tied with the
> MIN.
>
> I'm ok with not including MIN() either way.
>
> > +                       break;
> >
> >                 ret = -EINVAL;
> >                 if (!is_mem_zero((const char *)entry.reserved,
> > @@ -1385,19 +1379,13 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> >                 if (!f)
> >                         break;
> >
> > -               if (cmd == VDUSE_IOTLB_GET_FD2)
> > -                       ret = copy_to_user(argp, &entry,
> > -                                          sizeof(entry));
> > -               else
> > -                       ret = copy_to_user(argp, &entry.v1,
> > -                                          sizeof(entry.v1));
> > -
> > +               ret = copy_to_user(argp, &entry, _IOC_SIZE(cmd));
> >                 if (ret) {
> >                         ret = -EFAULT;
> >                         fput(f);
> >                         break;
> >                 }
> > -               ret = receive_fd(f, NULL, perm_to_file_flags(entry.v1.perm));
> > +               ret = receive_fd(f, NULL, perm_to_file_flags(entry.perm));
> >                 fput(f);
> >                 break;
> >         }
> > @@ -1603,16 +1591,16 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> >                 } else if (info.asid >= dev->nas)
> >                         break;
> >
> > -               entry.v1.start = info.start;
> > -               entry.v1.last = info.last;
> > +               entry.start = info.start;
> > +               entry.last = info.last;
> >                 entry.asid = info.asid;
> >                 ret = vduse_dev_iotlb_entry(dev, &entry, NULL,
> >                                             &info.capability);
> >                 if (ret < 0)
> >                         break;
> >
> > -               info.start = entry.v1.start;
> > -               info.last = entry.v1.last;
> > +               info.start = entry.start;
> > +               info.last = entry.last;
> >                 info.asid = entry.asid;
> >
> >                 ret = -EFAULT;
> > diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> > index faae7718bd2e..deca8c7b9178 100644
> > --- a/include/uapi/linux/vduse.h
> > +++ b/include/uapi/linux/vduse.h
> > @@ -299,9 +299,13 @@ struct vduse_iova_info {
> >   * Structure used by VDUSE_IOTLB_GET_FD2 ioctl to find an overlapped IOVA region.
> >   */
> >  struct vduse_iotlb_entry_v2 {
> > -       struct vduse_iotlb_entry v1;
> > +       __u64 offset;
> > +       __u64 start;
> > +       __u64 last;
> > +       __u8 perm;
> > +       __u8 padding[7];
> >         __u32 asid;
> > -       __u32 reserved[12];
> > +       __u32 reserved[11];

(I hit "Send" too early).

We could make this padding[3] so reserved keeps being [12]. This way
the struct members keep the same alignment between the commits. Not
super important as there should not be a lot of users of this right
now, we're just introducing it.

> >  };
> >
> >  /*
> > @@ -371,6 +375,7 @@ struct vduse_iova_range_v2 {
> >         __u64 start;
> >         __u64 last;
> >         __u32 asid;
> > +       __u32 padding;
> >  };
> >

Re: [PATCH 1/2] vduse: avoid adding implicit padding

[Arnd Bergmann]

On Mon, Feb 2, 2026, at 12:50, Eugenio Perez Martin wrote:
> On Mon, Feb 2, 2026 at 12:28 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
>> >                 ret = -EFAULT;
>> > -               if (cmd == VDUSE_IOTLB_GET_FD2) {
>> > -                       if (copy_from_user(&entry, argp, sizeof(entry)))
>> > -                               break;
>> > -               } else {
>> > -                       if (copy_from_user(&entry.v1, argp,
>> > -                                          sizeof(entry.v1)))
>> > -                               break;
>> > -               }
>> > +               if (copy_from_user(&entry, argp, _IOC_SIZE(cmd)))
>>
>> I did not know about _IOC_SIZE and I like how it reduces the complexity, thanks!
>>
>> As a proposal, maybe we can add MIN(_IOC_SIZE, sizeof(entry)) ? Not
>> sure if it is too much boilerplate for nothing as the compiler should
>> make the code identical and the uapi ioctl part should never change.
>> But it seems to me future changes to the code are better tied with the
>> MIN.
>> I'm ok with not including MIN() either way.

I think it's more readable without the MIN(), but I don't mind
adding it either.

>> >   */
>> >  struct vduse_iotlb_entry_v2 {
>> > -       struct vduse_iotlb_entry v1;
>> > +       __u64 offset;
>> > +       __u64 start;
>> > +       __u64 last;
>> > +       __u8 perm;
>> > +       __u8 padding[7];
>> >         __u32 asid;
>> > -       __u32 reserved[12];
>> > +       __u32 reserved[11];
>
> (I hit "Send" too early).
>
> We could make this padding[3] so reserved keeps being [12]. This way
> the struct members keep the same alignment between the commits. Not
> super important as there should not be a lot of users of this right
> now, we're just introducing it.

I think that is too risky, as it would overlay 'asid' on top of
previously uninitialized padding fields coming from userspace
on most architectures. Since there was previously no is_mem_zero()
check for the padding, I don't think it should be reused at all.

    Arnd

Re: [PATCH 1/2] vduse: avoid adding implicit padding

[Eugenio Perez Martin]

On Mon, Feb 2, 2026 at 1:07 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Feb 2, 2026, at 12:50, Eugenio Perez Martin wrote:
> > On Mon, Feb 2, 2026 at 12:28 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
> >> >                 ret = -EFAULT;
> >> > -               if (cmd == VDUSE_IOTLB_GET_FD2) {
> >> > -                       if (copy_from_user(&entry, argp, sizeof(entry)))
> >> > -                               break;
> >> > -               } else {
> >> > -                       if (copy_from_user(&entry.v1, argp,
> >> > -                                          sizeof(entry.v1)))
> >> > -                               break;
> >> > -               }
> >> > +               if (copy_from_user(&entry, argp, _IOC_SIZE(cmd)))
> >>
> >> I did not know about _IOC_SIZE and I like how it reduces the complexity, thanks!
> >>
> >> As a proposal, maybe we can add MIN(_IOC_SIZE, sizeof(entry)) ? Not
> >> sure if it is too much boilerplate for nothing as the compiler should
> >> make the code identical and the uapi ioctl part should never change.
> >> But it seems to me future changes to the code are better tied with the
> >> MIN.
> >> I'm ok with not including MIN() either way.
>
> I think it's more readable without the MIN(), but I don't mind
> adding it either.
>

Yep, I see how MIN() adds a little bit of noise. I'm happy either way :).

> >> >   */
> >> >  struct vduse_iotlb_entry_v2 {
> >> > -       struct vduse_iotlb_entry v1;
> >> > +       __u64 offset;
> >> > +       __u64 start;
> >> > +       __u64 last;
> >> > +       __u8 perm;
> >> > +       __u8 padding[7];
> >> >         __u32 asid;
> >> > -       __u32 reserved[12];
> >> > +       __u32 reserved[11];
> >
> > (I hit "Send" too early).
> >
> > We could make this padding[3] so reserved keeps being [12]. This way
> > the struct members keep the same alignment between the commits. Not
> > super important as there should not be a lot of users of this right
> > now, we're just introducing it.
>
> I think that is too risky, as it would overlay 'asid' on top of
> previously uninitialized padding fields coming from userspace
> on most architectures. Since there was previously no is_mem_zero()
> check for the padding, I don't think it should be reused at all.
>

Ok fair point. Yes, this would need something in the code like:

if (cmd == VDUSE_IOTLB_GET_FD)
    /* Ignoring whatever came in padding as it could be uninitialized due to
     * not having this member in the struct definition
     */
    memset(entry.padding, 0, sizeof(entry.padding);
else if (!mem_is_zero(entry.padding, sizeof(entry.padding,
sizeof(entry.padding))
    /* Return error following the style of the rest of the code */
    return -EINVAL
---

So we make sure we can use the padding in the future. Would that work?