On Mon, Feb 2, 2026 at 12:28 PM Eugenio Perez Martin
<[email protected]> wrote:
>
> 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];
(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;
> > };
> >
