On Tue, Aug 08, 2023 at 08:29:25PM -0300, Jason Gunthorpe wrote: > On Thu, Aug 03, 2023 at 03:18:23PM -0600, Alex Williamson wrote: > > > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c > > index 902f06e52c48..2d074cbd371d 100644 > > --- a/drivers/vfio/vfio_main.c > > +++ b/drivers/vfio/vfio_main.c > > @@ -1362,6 +1362,8 @@ struct vfio_info_cap_header *vfio_info_cap_add(struct vfio_info_cap *caps, > > void *buf; > > struct vfio_info_cap_header *header, *tmp; > > > > + size = ALIGN(size, sizeof(u64)); > > + > > buf = krealloc(caps->buf, caps->size + size, GFP_KERNEL); > > if (!buf) { > > kfree(caps->buf); > > @@ -1395,6 +1397,8 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset) > > struct vfio_info_cap_header *tmp; > > void *buf = (void *)caps->buf; > > > > + WARN_ON(!IS_ALIGNED(offset, sizeof(u64))); > > + > > for (tmp = buf; tmp->next; tmp = buf + tmp->next - offset) > > tmp->next += offset; > > } > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > > index fa06e3eb4955..fd2761841ffe 100644 > > --- a/include/uapi/linux/vfio.h > > +++ b/include/uapi/linux/vfio.h > > @@ -217,6 +217,7 @@ struct vfio_device_info { > > __u32 num_regions; /* Max region index + 1 */ > > __u32 num_irqs; /* Max IRQ index + 1 */ > > __u32 cap_offset; /* Offset within info struct of first cap */ > > + __u32 pad; /* Size must be aligned for caps */ > > }; > > #define VFIO_DEVICE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 7) > > > > @@ -1444,6 +1445,7 @@ struct vfio_iommu_type1_info { > > #define VFIO_IOMMU_INFO_CAPS (1 << 1) /* Info supports caps */ > > __u64 iova_pgsizes; /* Bitmap of supported page sizes */ > > __u32 cap_offset; /* Offset within info struct of first cap */ > > + __u32 pad; /* Size must be aligned for caps */ > > }; > > IMHO this is partially being caused by not using __aligned_u64 for the > other __u64's in the same struct.. > > Both of these structs have u64s in them and many arches will > automatically add the above padding. __aligned_u64 will force the > reset to do it, and then making padding explicit as you have done will > make it really true. > > This is a subtle x64/x32 compatability issue also. It is probably best > just to do the change across the whole header file. I will send a separate series that switches the struct definitions to __aligned_u64. > Please also include the matching hunk for iommufd: > > --- a/drivers/iommu/iommufd/vfio_compat.c > +++ b/drivers/iommu/iommufd/vfio_compat.c > @@ -483,6 +483,8 @@ static int iommufd_vfio_iommu_get_info(struct iommufd_ctx *ictx, > rc = cap_size; > goto out_put; > } > + cap_size = ALIGN(cap_size, sizeof(u64)); > + > if (last_cap && info.argsz >= total_cap_size && > put_user(total_cap_size, &last_cap->next)) { > rc = -EFAULT; Okay, will fix. Stefan
Attachment:
signature.asc
Description: PGP signature