linux-kernel - Re: [PATCH v2 1/3] iommu: Replace detach

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]

Date:   Fri, 26 Aug 2022 12:34:45 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Lu Baolu <baolu.lu@...ux.intel.com>
Cc:     Joerg Roedel <joro@...tes.org>,
        Christoph Hellwig <hch@...radead.org>,
        Kevin Tian <kevin.tian@...el.com>,
        Ashok Raj <ashok.raj@...el.com>, Will Deacon <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        Jean-Philippe Brucker <jean-philippe@...aro.com>,
        Eric Auger <eric.auger@...hat.com>,
        Liu Yi L <yi.l.liu@...el.com>,
        Jacob jun Pan <jacob.jun.pan@...el.com>,
        iommu@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] iommu: Replace detach_dev with real blocking
 domains

On Fri, Aug 26, 2022 at 08:30:12PM +0800, Lu Baolu wrote:
> From iommu core's point of view, detaching a domain from a device is
> equal to attaching the group's blocking domain to the device. This
> repalces all detach_dev callbacks in the IOMMU drivers with a real
> blocking domain and handles detaching domain through it.

There is a subtly here, where detach_dev() is not *always* going to a
blocking domain. Recall we made this mistaken when building the owner
series and it is why the blocking domain was introduced.

The challenge with this work is to be sure that the code you assigned
to the blocking domain doesn't actually set an identity domain or
set things to use the platform DMA ops.

> +static int blocking_domain_attach_dev(struct iommu_domain *_domain,
> +				      struct device *dev)
> +{
> +	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
> +
> +	s390_iommu_detach_device(domain, dev);
> +
> +	return 0;
> +}

For instance, I know that this is returning the group back to the
platform DMA ops.

To really make progress on this I would suggest

1) Remove detach_dev from drivers that don't use it. This is any
   driver that provides a default domain, if a default domain is set
   then we always use attach_dev(default_domain) and never detach_dev

2) Of those drivers in #1 audit and try to determine if their
   detach_dev is doing blocking domain behavior and if yes give them a
   real blocking domain. Otherwise just delete the code.

3) The remaining drivers are not using default_domain. Rename
   'detach_dev' to iommu_ops->set_platform_dma(dev) to make it clear
   that what it is doing is returning control back to the platform DMA
   ops.

I would do one patch per driver on the blocking domain conversions and
get acks from the driver owners. Do as many as can be acked, delete
the rest.

Make it clear to driver owners checking that blocking domain must halt
all DMA, and it must not be a passthrough or identity behavior.

When last I looked several of the drivers looked like they were
setting an identity domain on the detach_dev - eg back to a boot time
behavior where the iommu is bypassed. It is really important these not
be converted to blocking :) Arguably if they can be found they should
be converted to identity domains instead.

Jason