public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] [v2] vduse: avoid adding implicit padding
@ 2026-02-02 22:48 Arnd Bergmann
  2026-02-02 22:48 ` [PATCH 2/2] [v2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO Arnd Bergmann
  0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2026-02-02 22:48 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jason Wang, Eugenio Pérez
  Cc: Arnd Bergmann, Xuan Zhuo, Xie Yongji, virtualization,
	linux-kernel

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>
---
v2:
  no changes since v1
---
 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] [v2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO
  2026-02-02 22:48 [PATCH 1/2] [v2] vduse: avoid adding implicit padding Arnd Bergmann
@ 2026-02-02 22:48 ` Arnd Bergmann
  2026-02-03  7:15   ` Eugenio Perez Martin
                     ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Arnd Bergmann @ 2026-02-02 22:48 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jason Wang, Xie Yongji
  Cc: Arnd Bergmann, Xuan Zhuo, Eugenio Pérez, virtualization,
	linux-kernel

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 a proper .compat_ioctl handler for x86 that reads the structures
with the smaller padding before calling the internal handlers.

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>
---
The code is directly copied from the native ioctl handler, but I
did not test this with actual x86-32 userspace, so please review
carefully.
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 123 ++++++++++++++++++++++++++++-
 1 file changed, 122 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 405d59610f76..3ada167ac260 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1618,6 +1618,127 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 	return ret;
 }
 
+/*
+ * 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)
+
+static long vduse_dev_compat_ioctl(struct file *file, unsigned int cmd,
+				   unsigned long arg)
+{
+	struct vduse_dev *dev = file->private_data;
+	void __user *argp = (void __user *)arg;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_X86_64))
+		return vduse_dev_ioctl(file, cmd,
+				       (unsigned long)compat_ptr(arg));
+
+	if (unlikely(dev->broken))
+		return -EPERM;
+
+	switch (cmd) {
+	case COMPAT_VDUSE_IOTLB_GET_FD: {
+		struct vduse_iotlb_entry_v2 entry = {0};
+		struct file *f = NULL;
+
+		ret = -EFAULT;
+		if (copy_from_user(&entry, argp, _IOC_SIZE(cmd)))
+			break;
+
+		ret = vduse_dev_iotlb_entry(dev, &entry, &f, NULL);
+		if (ret)
+			break;
+
+		ret = -EINVAL;
+		if (!f)
+			break;
+
+		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.perm));
+		fput(f);
+		break;
+	}
+	case COMPAT_VDUSE_VQ_GET_INFO: {
+		struct vduse_vq_info vq_info = {};
+		struct vduse_virtqueue *vq;
+		u32 index;
+
+		ret = -EFAULT;
+		if (copy_from_user(&vq_info, argp,
+				   sizeof(struct compat_vduse_vq_info)))
+			break;
+
+		ret = -EINVAL;
+		if (vq_info.index >= dev->vq_num)
+			break;
+
+		index = array_index_nospec(vq_info.index, dev->vq_num);
+		vq = dev->vqs[index];
+		vq_info.desc_addr = vq->desc_addr;
+		vq_info.driver_addr = vq->driver_addr;
+		vq_info.device_addr = vq->device_addr;
+		vq_info.num = vq->num;
+
+		if (dev->driver_features & BIT_ULL(VIRTIO_F_RING_PACKED)) {
+			vq_info.packed.last_avail_counter =
+				vq->state.packed.last_avail_counter;
+			vq_info.packed.last_avail_idx =
+				vq->state.packed.last_avail_idx;
+			vq_info.packed.last_used_counter =
+				vq->state.packed.last_used_counter;
+			vq_info.packed.last_used_idx =
+				vq->state.packed.last_used_idx;
+		} else
+			vq_info.split.avail_index =
+				vq->state.split.avail_index;
+
+		vq_info.ready = vq->ready;
+
+		ret = -EFAULT;
+		if (copy_to_user(argp, &vq_info,
+		    sizeof(struct compat_vduse_vq_info)))
+			break;
+
+		ret = 0;
+		break;
+	}
+	default:
+		ret = -ENOIOCTLCMD;
+		break;
+	}
+
+	return vduse_dev_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
+}
+
 static int vduse_dev_release(struct inode *inode, struct file *file)
 {
 	struct vduse_dev *dev = file->private_data;
@@ -1678,7 +1799,7 @@ static const struct file_operations vduse_dev_fops = {
 	.write_iter	= vduse_dev_write_iter,
 	.poll		= vduse_dev_poll,
 	.unlocked_ioctl	= vduse_dev_ioctl,
-	.compat_ioctl	= compat_ptr_ioctl,
+	.compat_ioctl	= PTR_IF(IS_ENABLED(CONFIG_COMPAT), vduse_dev_compat_ioctl),
 	.llseek		= noop_llseek,
 };
 
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] [v2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO
  2026-02-02 22:48 ` [PATCH 2/2] [v2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO Arnd Bergmann
@ 2026-02-03  7:15   ` Eugenio Perez Martin
  2026-02-03 10:00   ` Michael S. Tsirkin
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Eugenio Perez Martin @ 2026-02-03  7:15 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Michael S. Tsirkin, Jason Wang, Xie Yongji, Arnd Bergmann,
	Xuan Zhuo, virtualization, linux-kernel

On Mon, Feb 2, 2026 at 11:49 PM 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 a proper .compat_ioctl handler for x86 that reads the structures
> with the smaller padding before calling the internal handlers.
>
> Fixes: ad146355bfad ("vduse: Support querying information of IOVA regions")
> Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")

I can work on merging the new code of vduse_dev_compat_ioctl and
vduse_dev_ioctl on top. If this fix makes it correct,

Acked-by: Eugenio Pérez <eperezma@redhat.com>

Thanks!

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> The code is directly copied from the native ioctl handler, but I
> did not test this with actual x86-32 userspace, so please review
> carefully.
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 123 ++++++++++++++++++++++++++++-
>  1 file changed, 122 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 405d59610f76..3ada167ac260 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1618,6 +1618,127 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>         return ret;
>  }
>
> +/*
> + * 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)
> +
> +static long vduse_dev_compat_ioctl(struct file *file, unsigned int cmd,
> +                                  unsigned long arg)
> +{
> +       struct vduse_dev *dev = file->private_data;
> +       void __user *argp = (void __user *)arg;
> +       int ret;
> +
> +       if (!IS_ENABLED(CONFIG_X86_64))
> +               return vduse_dev_ioctl(file, cmd,
> +                                      (unsigned long)compat_ptr(arg));
> +
> +       if (unlikely(dev->broken))
> +               return -EPERM;
> +
> +       switch (cmd) {
> +       case COMPAT_VDUSE_IOTLB_GET_FD: {
> +               struct vduse_iotlb_entry_v2 entry = {0};
> +               struct file *f = NULL;
> +
> +               ret = -EFAULT;
> +               if (copy_from_user(&entry, argp, _IOC_SIZE(cmd)))
> +                       break;
> +
> +               ret = vduse_dev_iotlb_entry(dev, &entry, &f, NULL);
> +               if (ret)
> +                       break;
> +
> +               ret = -EINVAL;
> +               if (!f)
> +                       break;
> +
> +               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.perm));
> +               fput(f);
> +               break;
> +       }
> +       case COMPAT_VDUSE_VQ_GET_INFO: {
> +               struct vduse_vq_info vq_info = {};
> +               struct vduse_virtqueue *vq;
> +               u32 index;
> +
> +               ret = -EFAULT;
> +               if (copy_from_user(&vq_info, argp,
> +                                  sizeof(struct compat_vduse_vq_info)))
> +                       break;
> +
> +               ret = -EINVAL;
> +               if (vq_info.index >= dev->vq_num)
> +                       break;
> +
> +               index = array_index_nospec(vq_info.index, dev->vq_num);
> +               vq = dev->vqs[index];
> +               vq_info.desc_addr = vq->desc_addr;
> +               vq_info.driver_addr = vq->driver_addr;
> +               vq_info.device_addr = vq->device_addr;
> +               vq_info.num = vq->num;
> +
> +               if (dev->driver_features & BIT_ULL(VIRTIO_F_RING_PACKED)) {
> +                       vq_info.packed.last_avail_counter =
> +                               vq->state.packed.last_avail_counter;
> +                       vq_info.packed.last_avail_idx =
> +                               vq->state.packed.last_avail_idx;
> +                       vq_info.packed.last_used_counter =
> +                               vq->state.packed.last_used_counter;
> +                       vq_info.packed.last_used_idx =
> +                               vq->state.packed.last_used_idx;
> +               } else
> +                       vq_info.split.avail_index =
> +                               vq->state.split.avail_index;
> +
> +               vq_info.ready = vq->ready;
> +
> +               ret = -EFAULT;
> +               if (copy_to_user(argp, &vq_info,
> +                   sizeof(struct compat_vduse_vq_info)))
> +                       break;
> +
> +               ret = 0;
> +               break;
> +       }
> +       default:
> +               ret = -ENOIOCTLCMD;
> +               break;
> +       }
> +
> +       return vduse_dev_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
> +}
> +
>  static int vduse_dev_release(struct inode *inode, struct file *file)
>  {
>         struct vduse_dev *dev = file->private_data;
> @@ -1678,7 +1799,7 @@ static const struct file_operations vduse_dev_fops = {
>         .write_iter     = vduse_dev_write_iter,
>         .poll           = vduse_dev_poll,
>         .unlocked_ioctl = vduse_dev_ioctl,
> -       .compat_ioctl   = compat_ptr_ioctl,
> +       .compat_ioctl   = PTR_IF(IS_ENABLED(CONFIG_COMPAT), vduse_dev_compat_ioctl),
>         .llseek         = noop_llseek,
>  };
>
> --
> 2.39.5
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] [v2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO
  2026-02-02 22:48 ` [PATCH 2/2] [v2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO Arnd Bergmann
  2026-02-03  7:15   ` Eugenio Perez Martin
@ 2026-02-03 10:00   ` Michael S. Tsirkin
  2026-02-03 10:22   ` Michael S. Tsirkin
  2026-02-13 10:04   ` Michael S. Tsirkin
  3 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2026-02-03 10:00 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jason Wang, Xie Yongji, Arnd Bergmann, Xuan Zhuo,
	Eugenio Pérez, virtualization, linux-kernel

On Mon, Feb 02, 2026 at 11:48:08PM +0100, Arnd Bergmann 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 a proper .compat_ioctl handler for x86 that reads the structures
> with the smaller padding before calling the internal handlers.
> 
> 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>
> ---
> The code is directly copied from the native ioctl handler, but I
> did not test this with actual x86-32 userspace, so please review
> carefully.
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 123 ++++++++++++++++++++++++++++-
>  1 file changed, 122 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 405d59610f76..3ada167ac260 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1618,6 +1618,127 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>  	return ret;
>  }
>  
> +/*
> + * 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)
> +
> +static long vduse_dev_compat_ioctl(struct file *file, unsigned int cmd,
> +				   unsigned long arg)


I think this needs an ifdef, otherwise gcc will warn about
an unused static function.

> +{
> +	struct vduse_dev *dev = file->private_data;
> +	void __user *argp = (void __user *)arg;
> +	int ret;
> +
> +	if (!IS_ENABLED(CONFIG_X86_64))
> +		return vduse_dev_ioctl(file, cmd,
> +				       (unsigned long)compat_ptr(arg));
> +
> +	if (unlikely(dev->broken))
> +		return -EPERM;
> +
> +	switch (cmd) {
> +	case COMPAT_VDUSE_IOTLB_GET_FD: {
> +		struct vduse_iotlb_entry_v2 entry = {0};
> +		struct file *f = NULL;
> +
> +		ret = -EFAULT;
> +		if (copy_from_user(&entry, argp, _IOC_SIZE(cmd)))
> +			break;
> +
> +		ret = vduse_dev_iotlb_entry(dev, &entry, &f, NULL);
> +		if (ret)
> +			break;
> +
> +		ret = -EINVAL;
> +		if (!f)
> +			break;
> +
> +		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.perm));
> +		fput(f);
> +		break;
> +	}
> +	case COMPAT_VDUSE_VQ_GET_INFO: {
> +		struct vduse_vq_info vq_info = {};
> +		struct vduse_virtqueue *vq;
> +		u32 index;
> +
> +		ret = -EFAULT;
> +		if (copy_from_user(&vq_info, argp,
> +				   sizeof(struct compat_vduse_vq_info)))
> +			break;
> +
> +		ret = -EINVAL;
> +		if (vq_info.index >= dev->vq_num)
> +			break;
> +
> +		index = array_index_nospec(vq_info.index, dev->vq_num);
> +		vq = dev->vqs[index];
> +		vq_info.desc_addr = vq->desc_addr;
> +		vq_info.driver_addr = vq->driver_addr;
> +		vq_info.device_addr = vq->device_addr;
> +		vq_info.num = vq->num;
> +
> +		if (dev->driver_features & BIT_ULL(VIRTIO_F_RING_PACKED)) {
> +			vq_info.packed.last_avail_counter =
> +				vq->state.packed.last_avail_counter;
> +			vq_info.packed.last_avail_idx =
> +				vq->state.packed.last_avail_idx;
> +			vq_info.packed.last_used_counter =
> +				vq->state.packed.last_used_counter;
> +			vq_info.packed.last_used_idx =
> +				vq->state.packed.last_used_idx;
> +		} else
> +			vq_info.split.avail_index =
> +				vq->state.split.avail_index;
> +
> +		vq_info.ready = vq->ready;
> +
> +		ret = -EFAULT;
> +		if (copy_to_user(argp, &vq_info,
> +		    sizeof(struct compat_vduse_vq_info)))
> +			break;
> +
> +		ret = 0;
> +		break;
> +	}
> +	default:
> +		ret = -ENOIOCTLCMD;
> +		break;
> +	}
> +
> +	return vduse_dev_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
> +}
> +
>  static int vduse_dev_release(struct inode *inode, struct file *file)
>  {
>  	struct vduse_dev *dev = file->private_data;
> @@ -1678,7 +1799,7 @@ static const struct file_operations vduse_dev_fops = {
>  	.write_iter	= vduse_dev_write_iter,
>  	.poll		= vduse_dev_poll,
>  	.unlocked_ioctl	= vduse_dev_ioctl,
> -	.compat_ioctl	= compat_ptr_ioctl,
> +	.compat_ioctl	= PTR_IF(IS_ENABLED(CONFIG_COMPAT), vduse_dev_compat_ioctl),
>  	.llseek		= noop_llseek,
>  };
>  
> -- 
> 2.39.5


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] [v2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO
  2026-02-02 22:48 ` [PATCH 2/2] [v2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO Arnd Bergmann
  2026-02-03  7:15   ` Eugenio Perez Martin
  2026-02-03 10:00   ` Michael S. Tsirkin
@ 2026-02-03 10:22   ` Michael S. Tsirkin
  2026-02-03 10:39     ` Arnd Bergmann
  2026-02-13 10:04   ` Michael S. Tsirkin
  3 siblings, 1 reply; 8+ messages in thread
From: Michael S. Tsirkin @ 2026-02-03 10:22 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jason Wang, Xie Yongji, Arnd Bergmann, Xuan Zhuo,
	Eugenio Pérez, virtualization, linux-kernel

On Mon, Feb 02, 2026 at 11:48:08PM +0100, Arnd Bergmann 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 a proper .compat_ioctl handler for x86 that reads the structures
> with the smaller padding before calling the internal handlers.
> 
> 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>
> ---
> The code is directly copied from the native ioctl handler, but I
> did not test this with actual x86-32 userspace, so please review
> carefully.

More importantly, I'm not applying this until it's tested)

> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 123 ++++++++++++++++++++++++++++-
>  1 file changed, 122 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 405d59610f76..3ada167ac260 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1618,6 +1618,127 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>  	return ret;
>  }
>  
> +/*

ifndef CONFIG_COMPAT around the structs will make it clearer
they are only for this purpose.

> + * i386 has different alignment constraints than x86_64,

why i386 specifically? many architectures have CONFIG_COMPAT
and it looks like all of them will have the issue.

> + * 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];

Was surprised to learn __alignof__ can be used to size
arrays. This is the first use of this capability in the kernel.

I think the point of all this is that compat_vduse_iotlb_entry
will be 4 byte aligned now? Very well. But why do we bother
with specifying the hidden padding? compilers adds exactly
this amount anyway. Just plan compat_u64 will do the trick.

> +};



> +#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;

it's a global variable? I'm not aware of this trick. What is this doing?

> +#define COMPAT_VDUSE_VQ_GET_INFO	_IOWR(VDUSE_BASE, 0x15, struct compat_vduse_vq_info)
> +
> +static long vduse_dev_compat_ioctl(struct file *file, unsigned int cmd,
> +				   unsigned long arg)
> +{
> +	struct vduse_dev *dev = file->private_data;
> +	void __user *argp = (void __user *)arg;
> +	int ret;
> +
> +	if (!IS_ENABLED(CONFIG_X86_64))
> +		return vduse_dev_ioctl(file, cmd,
> +				       (unsigned long)compat_ptr(arg));
> +
> +	if (unlikely(dev->broken))
> +		return -EPERM;
> +
> +	switch (cmd) {
> +	case COMPAT_VDUSE_IOTLB_GET_FD: {
> +		struct vduse_iotlb_entry_v2 entry = {0};
> +		struct file *f = NULL;
> +
> +		ret = -EFAULT;
> +		if (copy_from_user(&entry, argp, _IOC_SIZE(cmd)))
> +			break;
> +
> +		ret = vduse_dev_iotlb_entry(dev, &entry, &f, NULL);
> +		if (ret)
> +			break;
> +
> +		ret = -EINVAL;
> +		if (!f)
> +			break;
> +
> +		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.perm));
> +		fput(f);
> +		break;
> +	}
> +	case COMPAT_VDUSE_VQ_GET_INFO: {
> +		struct vduse_vq_info vq_info = {};
> +		struct vduse_virtqueue *vq;
> +		u32 index;
> +
> +		ret = -EFAULT;
> +		if (copy_from_user(&vq_info, argp,
> +				   sizeof(struct compat_vduse_vq_info)))
> +			break;
> +
> +		ret = -EINVAL;
> +		if (vq_info.index >= dev->vq_num)
> +			break;
> +
> +		index = array_index_nospec(vq_info.index, dev->vq_num);
> +		vq = dev->vqs[index];
> +		vq_info.desc_addr = vq->desc_addr;
> +		vq_info.driver_addr = vq->driver_addr;
> +		vq_info.device_addr = vq->device_addr;
> +		vq_info.num = vq->num;
> +
> +		if (dev->driver_features & BIT_ULL(VIRTIO_F_RING_PACKED)) {
> +			vq_info.packed.last_avail_counter =
> +				vq->state.packed.last_avail_counter;
> +			vq_info.packed.last_avail_idx =
> +				vq->state.packed.last_avail_idx;
> +			vq_info.packed.last_used_counter =
> +				vq->state.packed.last_used_counter;
> +			vq_info.packed.last_used_idx =
> +				vq->state.packed.last_used_idx;
> +		} else
> +			vq_info.split.avail_index =
> +				vq->state.split.avail_index;
> +
> +		vq_info.ready = vq->ready;
> +
> +		ret = -EFAULT;
> +		if (copy_to_user(argp, &vq_info,
> +		    sizeof(struct compat_vduse_vq_info)))
> +			break;
> +
> +		ret = 0;
> +		break;
> +	}
> +	default:
> +		ret = -ENOIOCTLCMD;
> +		break;
> +	}
> +
> +	return vduse_dev_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
> +}
> +
>  static int vduse_dev_release(struct inode *inode, struct file *file)
>  {
>  	struct vduse_dev *dev = file->private_data;
> @@ -1678,7 +1799,7 @@ static const struct file_operations vduse_dev_fops = {
>  	.write_iter	= vduse_dev_write_iter,
>  	.poll		= vduse_dev_poll,
>  	.unlocked_ioctl	= vduse_dev_ioctl,
> -	.compat_ioctl	= compat_ptr_ioctl,
> +	.compat_ioctl	= PTR_IF(IS_ENABLED(CONFIG_COMPAT), vduse_dev_compat_ioctl),

Too funky IMHO. Everyone uses ifdef around this, let's do the same.


>  	.llseek		= noop_llseek,
>  };
>  
> -- 
> 2.39.5


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] [v2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO
  2026-02-03 10:22   ` Michael S. Tsirkin
@ 2026-02-03 10:39     ` Arnd Bergmann
  2026-02-03 10:47       ` Michael S. Tsirkin
  0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2026-02-03 10:39 UTC (permalink / raw)
  To: Michael S. Tsirkin, Arnd Bergmann
  Cc: Jason Wang, Xie Yongji, Xuan Zhuo, Eugenio Pérez,
	virtualization, linux-kernel

On Tue, Feb 3, 2026, at 11:22, Michael S. Tsirkin wrote:
> On Mon, Feb 02, 2026 at 11:48:08PM +0100, Arnd Bergmann 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 a proper .compat_ioctl handler for x86 that reads the structures
>> with the smaller padding before calling the internal handlers.
>> 
>> 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>
>> ---
>> The code is directly copied from the native ioctl handler, but I
>> did not test this with actual x86-32 userspace, so please review
>> carefully.
>
> More importantly, I'm not applying this until it's tested)

Sure

> ifndef CONFIG_COMPAT around the structs will make it clearer
> they are only for this purpose.
>
>> + * i386 has different alignment constraints than x86_64,
>
> why i386 specifically? many architectures have CONFIG_COMPAT
> and it looks like all of them will have the issue.

No, the weird alignment rules are only on arc, csky, m68k,
microblaze, nios2, openrisc, sh and x86-32. Out of those,
x86 is hte only one that currently has a 64-bit version
(arc and micrblaze 64-bit support never made it upstream,
sh64 was removed since there were no products).

All the other architectures with compat support (arm,
powerpc, mips, sparc, riscv) have the same alignment rules
for 32-bit and 64-bit builds and align all integers naturally.

>> + * 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];
>
> Was surprised to learn __alignof__ can be used to size
> arrays. This is the first use of this capability in the kernel.
>
> I think the point of all this is that compat_vduse_iotlb_entry
> will be 4 byte aligned now? Very well. But why do we bother
> with specifying the hidden padding? compilers adds exactly
> this amount anyway. Just plan compat_u64 will do the trick.

Right, I could remove the padding field here, since this is
just used to document the size of the otherwise implied
padding.

The patch I used to find the issue originally adds explicit
padding to all uapi structures with implied padding, so I
did the smae thing here.

>> +#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;
>
> it's a global variable? I'm not aware of this trick. What is this doing?

My mistake, that should not have been here.

>> @@ -1678,7 +1799,7 @@ static const struct file_operations vduse_dev_fops = {
>>  	.write_iter	= vduse_dev_write_iter,
>>  	.poll		= vduse_dev_poll,
>>  	.unlocked_ioctl	= vduse_dev_ioctl,
>> -	.compat_ioctl	= compat_ptr_ioctl,
>> +	.compat_ioctl	= PTR_IF(IS_ENABLED(CONFIG_COMPAT), vduse_dev_compat_ioctl),
>
> Too funky IMHO. Everyone uses ifdef around this, let's do the same.

Sure. I only used this because you asked for fewer #ifdefs in
my v1 patch. If I use an #ifdef around this one, I also have
to add one around the function definition. In that case, I'd
probably change it back to the x86 check there, and use

#if defined(CONFIG_X86_64) && defined(CONFIG_COMPAT)
      .compat_ioctl = vduse_dev_compat_ioctl,
#else
      .compat_ioctl = compat_ptr_ioctl.
#endif

      Arnd

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] [v2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO
  2026-02-03 10:39     ` Arnd Bergmann
@ 2026-02-03 10:47       ` Michael S. Tsirkin
  0 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2026-02-03 10:47 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Arnd Bergmann, Jason Wang, Xie Yongji, Xuan Zhuo,
	Eugenio Pérez, virtualization, linux-kernel

On Tue, Feb 03, 2026 at 11:39:43AM +0100, Arnd Bergmann wrote:
> On Tue, Feb 3, 2026, at 11:22, Michael S. Tsirkin wrote:
> > On Mon, Feb 02, 2026 at 11:48:08PM +0100, Arnd Bergmann 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 a proper .compat_ioctl handler for x86 that reads the structures
> >> with the smaller padding before calling the internal handlers.
> >> 
> >> 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>
> >> ---
> >> The code is directly copied from the native ioctl handler, but I
> >> did not test this with actual x86-32 userspace, so please review
> >> carefully.
> >
> > More importantly, I'm not applying this until it's tested)
> 
> Sure
> 
> > ifndef CONFIG_COMPAT around the structs will make it clearer
> > they are only for this purpose.
> >
> >> + * i386 has different alignment constraints than x86_64,
> >
> > why i386 specifically? many architectures have CONFIG_COMPAT
> > and it looks like all of them will have the issue.
> 
> No, the weird alignment rules are only on arc, csky, m68k,
> microblaze, nios2, openrisc, sh and x86-32. Out of those,
> x86 is hte only one that currently has a 64-bit version
> (arc and micrblaze 64-bit support never made it upstream,
> sh64 was removed since there were no products).
> 
> All the other architectures with compat support (arm,
> powerpc, mips, sparc, riscv) have the same alignment rules
> for 32-bit and 64-bit builds and align all integers naturally.


Oh interesting. But the code is compiled for
and generates useless code for all CONFIG_COMPAT right now.

The ifdef you need is COMPAT_FOR_U64_ALIGNMENT then, I think.





> >> + * 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];
> >
> > Was surprised to learn __alignof__ can be used to size
> > arrays. This is the first use of this capability in the kernel.
> >
> > I think the point of all this is that compat_vduse_iotlb_entry
> > will be 4 byte aligned now? Very well. But why do we bother
> > with specifying the hidden padding? compilers adds exactly
> > this amount anyway. Just plan compat_u64 will do the trick.
> 
> Right, I could remove the padding field here, since this is
> just used to document the size of the otherwise implied
> padding.
> 
> The patch I used to find the issue originally adds explicit
> padding to all uapi structures with implied padding, so I
> did the smae thing here.
> 
> >> +#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;
> >
> > it's a global variable? I'm not aware of this trick. What is this doing?
> 
> My mistake, that should not have been here.
> 
> >> @@ -1678,7 +1799,7 @@ static const struct file_operations vduse_dev_fops = {
> >>  	.write_iter	= vduse_dev_write_iter,
> >>  	.poll		= vduse_dev_poll,
> >>  	.unlocked_ioctl	= vduse_dev_ioctl,
> >> -	.compat_ioctl	= compat_ptr_ioctl,
> >> +	.compat_ioctl	= PTR_IF(IS_ENABLED(CONFIG_COMPAT), vduse_dev_compat_ioctl),
> >
> > Too funky IMHO. Everyone uses ifdef around this, let's do the same.
> 
> Sure. I only used this because you asked for fewer #ifdefs in
> my v1 patch.

It's less the amount of ifdefs more them being placed strategically.
sorry about being unclear.

> If I use an #ifdef around this one, I also have
> to add one around the function definition.

and the structs, preferably.

> In that case, I'd
> probably change it back to the x86 check there, and use
> 
> #if defined(CONFIG_X86_64) && defined(CONFIG_COMPAT)
>       .compat_ioctl = vduse_dev_compat_ioctl,
> #else
>       .compat_ioctl = compat_ptr_ioctl.
> #endif
> 
>       Arnd


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] [v2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO
  2026-02-02 22:48 ` [PATCH 2/2] [v2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO Arnd Bergmann
                     ` (2 preceding siblings ...)
  2026-02-03 10:22   ` Michael S. Tsirkin
@ 2026-02-13 10:04   ` Michael S. Tsirkin
  3 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2026-02-13 10:04 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jason Wang, Xie Yongji, Arnd Bergmann, Xuan Zhuo,
	Eugenio Pérez, virtualization, linux-kernel

On Mon, Feb 02, 2026 at 11:48:08PM +0100, Arnd Bergmann 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 a proper .compat_ioctl handler for x86 that reads the structures
> with the smaller padding before calling the internal handlers.
> 
> 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>


So patch 1 is in going upstream but I am still waiting for v3 of patch
1.

> ---
> The code is directly copied from the native ioctl handler, but I
> did not test this with actual x86-32 userspace, so please review
> carefully.
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 123 ++++++++++++++++++++++++++++-
>  1 file changed, 122 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 405d59610f76..3ada167ac260 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1618,6 +1618,127 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>  	return ret;
>  }
>  
> +/*
> + * 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)
> +
> +static long vduse_dev_compat_ioctl(struct file *file, unsigned int cmd,
> +				   unsigned long arg)
> +{
> +	struct vduse_dev *dev = file->private_data;
> +	void __user *argp = (void __user *)arg;
> +	int ret;
> +
> +	if (!IS_ENABLED(CONFIG_X86_64))
> +		return vduse_dev_ioctl(file, cmd,
> +				       (unsigned long)compat_ptr(arg));
> +
> +	if (unlikely(dev->broken))
> +		return -EPERM;
> +
> +	switch (cmd) {
> +	case COMPAT_VDUSE_IOTLB_GET_FD: {
> +		struct vduse_iotlb_entry_v2 entry = {0};
> +		struct file *f = NULL;
> +
> +		ret = -EFAULT;
> +		if (copy_from_user(&entry, argp, _IOC_SIZE(cmd)))
> +			break;
> +
> +		ret = vduse_dev_iotlb_entry(dev, &entry, &f, NULL);
> +		if (ret)
> +			break;
> +
> +		ret = -EINVAL;
> +		if (!f)
> +			break;
> +
> +		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.perm));
> +		fput(f);
> +		break;
> +	}
> +	case COMPAT_VDUSE_VQ_GET_INFO: {
> +		struct vduse_vq_info vq_info = {};
> +		struct vduse_virtqueue *vq;
> +		u32 index;
> +
> +		ret = -EFAULT;
> +		if (copy_from_user(&vq_info, argp,
> +				   sizeof(struct compat_vduse_vq_info)))
> +			break;
> +
> +		ret = -EINVAL;
> +		if (vq_info.index >= dev->vq_num)
> +			break;
> +
> +		index = array_index_nospec(vq_info.index, dev->vq_num);
> +		vq = dev->vqs[index];
> +		vq_info.desc_addr = vq->desc_addr;
> +		vq_info.driver_addr = vq->driver_addr;
> +		vq_info.device_addr = vq->device_addr;
> +		vq_info.num = vq->num;
> +
> +		if (dev->driver_features & BIT_ULL(VIRTIO_F_RING_PACKED)) {
> +			vq_info.packed.last_avail_counter =
> +				vq->state.packed.last_avail_counter;
> +			vq_info.packed.last_avail_idx =
> +				vq->state.packed.last_avail_idx;
> +			vq_info.packed.last_used_counter =
> +				vq->state.packed.last_used_counter;
> +			vq_info.packed.last_used_idx =
> +				vq->state.packed.last_used_idx;
> +		} else
> +			vq_info.split.avail_index =
> +				vq->state.split.avail_index;
> +
> +		vq_info.ready = vq->ready;
> +
> +		ret = -EFAULT;
> +		if (copy_to_user(argp, &vq_info,
> +		    sizeof(struct compat_vduse_vq_info)))
> +			break;
> +
> +		ret = 0;
> +		break;
> +	}
> +	default:
> +		ret = -ENOIOCTLCMD;
> +		break;
> +	}
> +
> +	return vduse_dev_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
> +}
> +
>  static int vduse_dev_release(struct inode *inode, struct file *file)
>  {
>  	struct vduse_dev *dev = file->private_data;
> @@ -1678,7 +1799,7 @@ static const struct file_operations vduse_dev_fops = {
>  	.write_iter	= vduse_dev_write_iter,
>  	.poll		= vduse_dev_poll,
>  	.unlocked_ioctl	= vduse_dev_ioctl,
> -	.compat_ioctl	= compat_ptr_ioctl,
> +	.compat_ioctl	= PTR_IF(IS_ENABLED(CONFIG_COMPAT), vduse_dev_compat_ioctl),
>  	.llseek		= noop_llseek,
>  };
>  
> -- 
> 2.39.5


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-02-13 10:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-02 22:48 [PATCH 1/2] [v2] vduse: avoid adding implicit padding Arnd Bergmann
2026-02-02 22:48 ` [PATCH 2/2] [v2] vduse: fix compat handling for VDUSE_IOTLB_GET_FD/VDUSE_VQ_GET_INFO Arnd Bergmann
2026-02-03  7:15   ` Eugenio Perez Martin
2026-02-03 10:00   ` Michael S. Tsirkin
2026-02-03 10:22   ` Michael S. Tsirkin
2026-02-03 10:39     ` Arnd Bergmann
2026-02-03 10:47       ` Michael S. Tsirkin
2026-02-13 10:04   ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox