linux-kernel - Re: [PATCH v4 1/5] iommu/s390: Fix duplicate domain attachments

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: <Yz7Dzayee74Mu4NH@nvidia.com>
Date:   Thu, 6 Oct 2022 09:02:21 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Niklas Schnelle <schnelle@...ux.ibm.com>
Cc:     Matthew Rosato <mjrosato@...ux.ibm.com>,
        Pierre Morel <pmorel@...ux.ibm.com>, iommu@...ts.linux.dev,
        linux-s390@...r.kernel.org, borntraeger@...ux.ibm.com,
        hca@...ux.ibm.com, gor@...ux.ibm.com,
        gerald.schaefer@...ux.ibm.com, agordeev@...ux.ibm.com,
        svens@...ux.ibm.com, joro@...tes.org, will@...nel.org,
        robin.murphy@....com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 1/5] iommu/s390: Fix duplicate domain attachments

On Thu, Oct 06, 2022 at 01:52:44PM +0200, Niklas Schnelle wrote:

> One option I see would be to ignore the error return from
> zpci_register_ioat() if it indicates case 2b. Then we would still add
> the device to the IOMMU's devices list and return success despite
> knowing that the device is inaccessible (DMA and MMIO blocked).
> 
> Then the recovery/reset code will register the new domain once the
> device comes out of the error state. At least from an IOMMU API point
> of view that would make the attachment always succeed for all
> zpci_register_ioat() error cases that aren't programming bugs and can
> conceivably be recovered from.

This is what I was thinking..

> If you agree I would propose adding this as a robustness improvement as
> part of my upcoming series of IOMMU improvements needed for the DMA API
> conversion. As stated above before the DMA API conversion any error
> that would cause zpci_register_ioat() to fail while the IOMMU API is
> being used will need a "power cycle" anyway so postponing this doesn't
> hurt.

Yes, I think this series is fine as is

Patch 4 mostly deletes all these error cases, and the one hunk that is left:

+	if (domain->geometry.aperture_start > zdev->end_dma ||
+	    domain->geometry.aperture_end < zdev->start_dma)
+		return -EINVAL;

Is misplaced. If a device cannot be supported by the IOMMU, which is
what that is really saying since it only s390 creates one aperture
size, then it should fail to probe, not fail at attach.

So I'd change the above to a WARN_ON() for future safety and add a
similar test to probe and then all that is left is the
zpci_register_ioat() which you have a plan for.

Jason