On Mon, Feb 2, 2026 at 11:06 AM Arnd Bergmann <[email protected]> wrote:
>
> From: Arnd Bergmann <[email protected]>
>
> 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 <[email protected]>
> ---
> 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;
> };
>
