On Thu, Aug 10, 2023 at 03:22:56AM +0000, Tian, Kevin wrote: > > From: Stefan Hajnoczi <stefanha@xxxxxxxxxx> > > Sent: Thursday, August 10, 2023 5:03 AM > > > > The memory layout of struct vfio_device_gfx_plane_info is > > architecture-dependent due to a u64 field and a struct size that is not > > a multiple of 8 bytes: > > - On x86_64 the struct size is padded to a multiple of 8 bytes. > > - On x32 the struct size is only a multiple of 4 bytes, not 8. > > - Other architectures may vary. > > > > Use __aligned_u64 to make memory layout consistent. This reduces the > > chance of holes that result in an information leak and the chance that > > I didn't quite get this. The leak example [1] from your earlier fix is really > not caused by the use of __u64. Instead it's a counter example that on > x32 there is no hole with 4byte alignment for __u64. > > I'd remove the hole part and just keep the compat reason. > > [1] https://lore.kernel.org/lkml/20230801103114.757d7992.alex.williamson@xxxxxxxxxx/T/ Okay. > > > @@ -1392,6 +1392,8 @@ static long intel_vgpu_ioctl(struct vfio_device > > *vfio_dev, unsigned int cmd, > > if (dmabuf.argsz < minsz) > > return -EINVAL; > > > > + minsz = min(minsz, sizeof(dmabuf)); > > + > > Is there a case where minsz could be greater than sizeof(dmabuf)? Thanks for spotting this, it's a bug in the patch. It should be min(dmabuf.argsz, sizeof(dmabuf)). Stefan
Attachment:
signature.asc
Description: PGP signature
