linux-kernel - Re: [PATCH v1 01/13] genirq/msi: Store the IOMMU IOVA directly in msi_desc instead of iommu

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250213135752.GY3754072@nvidia.com>
Date: Thu, 13 Feb 2025 09:57:52 -0400
From: Jason Gunthorpe <jgg@...dia.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Nicolin Chen <nicolinc@...dia.com>, kevin.tian@...el.com,
	maz@...nel.org, joro@...tes.org, will@...nel.org,
	robin.murphy@....com, shuah@...nel.org, iommu@...ts.linux.dev,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-kselftest@...r.kernel.org, eric.auger@...hat.com,
	baolu.lu@...ux.intel.com, yi.l.liu@...el.com, yury.norov@...il.com,
	jacob.pan@...ux.microsoft.com, patches@...ts.linux.dev
Subject: Re: [PATCH v1 01/13] genirq/msi: Store the IOMMU IOVA directly in
 msi_desc instead of iommu_cookie

On Thu, Feb 13, 2025 at 12:54:15PM +0100, Thomas Gleixner wrote:
> So this change log really fails to follow the basic structure:
> 
>    The context, the problem and the solution

The IOMMU translation for MSI message addresses is a two step
process, seperated in time:

 1) iommu_dma_prepare_msi(): A cookie pointer containing the IOVA
    address is stored in the MSI descriptor, when a MSI interrupt is
    allocated.

 2) iommu_dma_compose_msi_msg(): The compose callback uses this
    cookkie pointer to compute the translated message address.

This has an inherent lifetime problem for the pointer stored in the
cookie. It must remain valid between the two steps. There is no
locking at the irq layer that helps protect the liftime. Today this
only works under the assumption that the iommu domain is not changed
while MSI interrupts are being programmed. This is true for normal DMA
API users within the kernel as the iommu domain is attached before the
driver is probed and cannot be changed while a driver is attached.

Classic VFIO type1 also prevented changing the iommu domain while VFIO
was running as it does not support changing the "container" after
starting up.

However, iommufd has improved this and we can change the iommu domain
during VFIO operation. This allows userspace to directly race
VFIO_DEVICE_ATTACH_IOMMUFD_PT (which calls iommu_attach_group()) and
VFIO_DEVICE_SET_IRQS (which calls into iommu_dma_compose_msi_msg()).

This causes both the cookie pointer and the unlocked call to
iommu_get_domain_for_dev() on the MSI translation path to become a
potential UAF.

Fix the MSI cookie UAF by removing the cookie pointer. The translated
message address is already known during iommu_dma_prepare_msi() and
can not change. It can simply be stored as an integer in the MSI
descriptor.

A following patch will correct the iommu_get_domain_for_dev() UAF
using the IOMMU group mutex.

Ok?

Nicolin - lets change the patch structure a little bit can you adjust
this patch to leave iommu_dma_compose_msi_msg() in dma-iommu.c and the
next patch will be all about renaming and moving it to the MSI core
code instead? Easier to explain

Thanks,
Jason