linux-kernel - Re: [PATCH v3] vfio: align capability structures

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230817123338.764e8c83.alex.williamson@redhat.com>
Date:   Thu, 17 Aug 2023 12:33:38 -0600
From:   Alex Williamson <alex.williamson@...hat.com>
To:     Jason Gunthorpe <jgg@...dia.com>,
        Stefan Hajnoczi <stefanha@...hat.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] vfio: align capability structures

On Thu, 17 Aug 2023 15:01:30 -0300
Jason Gunthorpe <jgg@...dia.com> wrote:

> On Wed, Aug 09, 2023 at 04:31:44PM -0400, Stefan Hajnoczi wrote:
> > The VFIO_DEVICE_GET_INFO, VFIO_DEVICE_GET_REGION_INFO, and
> > VFIO_IOMMU_GET_INFO ioctls fill in an info struct followed by capability
> > structs:
> > 
> >   +------+---------+---------+-----+
> >   | info | caps[0] | caps[1] | ... |
> >   +------+---------+---------+-----+
> > 
> > Both the info and capability struct sizes are not always multiples of
> > sizeof(u64), leaving u64 fields in later capability structs misaligned.
> > 
> > Userspace applications currently need to handle misalignment manually in
> > order to support CPU architectures and programming languages with strict
> > alignment requirements.
> > 
> > Make life easier for userspace by ensuring alignment in the kernel. This
> > is done by padding info struct definitions and by copying out zeroes
> > after capability structs that are not aligned.
> > 
> > The new layout is as follows:
> > 
> >   +------+---------+---+---------+-----+
> >   | info | caps[0] | 0 | caps[1] | ... |
> >   +------+---------+---+---------+-----+
> > 
> > In this example caps[0] has a size that is not multiples of sizeof(u64),
> > so zero padding is added to align the subsequent structure.
> > 
> > Adding zero padding between structs does not break the uapi. The memory
> > layout is specified by the info.cap_offset and caps[i].next fields
> > filled in by the kernel. Applications use these field values to locate
> > structs and are therefore unaffected by the addition of zero padding.
> > 
> > Note that code that copies out info structs with padding is updated to
> > always zero the struct and copy out as many bytes as userspace
> > requested. This makes the code shorter and avoids potential information
> > leaks by ensuring padding is initialized.
> > 
> > Originally-by: Alex Williamson <alex.williamson@...hat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@...hat.com>
> > ---
> > v3:
> > - Also align capability structs in drivers/iommu/iommufd/vfio_compat.c
> >   [Jason]
> > 
> >  include/uapi/linux/vfio.h           |  2 ++
> >  drivers/iommu/iommufd/vfio_compat.c |  2 ++
> >  drivers/vfio/pci/vfio_pci_core.c    | 11 ++---------
> >  drivers/vfio/vfio_iommu_type1.c     | 11 ++---------
> >  drivers/vfio/vfio_main.c            |  6 ++++++
> >  5 files changed, 14 insertions(+), 18 deletions(-)  
> 
> Acked-by: Jason Gunthorpe <jgg@...dia.com>

Thanks!

Applied to vfio next branch for v6.6.  Thanks,

Alex