On Tue, Aug 15, 2023 at 03:23:50PM +0000, David Laight wrote: > From: Stefan Hajnoczi > > Sent: 09 August 2023 22:03 > > > > 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 > > 32-bit userspace on a 64-bit kernel breakage. > > Isn't the hole likely to cause an information leak? > Forcing it to be there doesn't make any difference. > I'd add an explicit pad as well. Yes, Kevin had a similar comment about this text. What I meant was that it's safest to have a single memory layout across all architectures (with explicit padding) so that there are no surprises. I'm going to remove the statement about information leaks because it's confusing. > > It is a shame there isn't an __attribute__(()) to error padded structures. > > > > > This patch increases the struct size on x32 but this is safe because of > > the struct's argsz field. The kernel may grow the struct as long as it > > still supports smaller argsz values from userspace (e.g. applications > > compiled against older kernel headers). > > Doesn't changing the offset of later fields break compatibility? > The size field (probably) only lets you extend the structure. Yes, that would break compatibility but I don't see any changes in this patch series that modifies the offsets of later fields. Have I missed something? > Oh, for sanity do min(variable, constant). Can you elaborate? Thanks, Stefan
Attachment:
signature.asc
Description: PGP signature
