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

Message-ID: <2488828cfdd54cd15b2fecbbc021aaf69228f0c3.camel@linux.ibm.com>
Date:   Thu, 06 Oct 2022 15:01:59 +0200
From:   Niklas Schnelle <schnelle@...ux.ibm.com>
To:     Jason Gunthorpe <jgg@...dia.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, 2022-10-06 at 09:02 -0300, Jason Gunthorpe wrote:
> 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
>  

Sounds good will do a v5 anyway to add the map_pages()/unmap_pages().