linux-kernel - Re: Memory corruption with CONFIG_SWIOTLB

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: <20231110102245.6ac971b9.pasic@linux.ibm.com>
Date:   Fri, 10 Nov 2023 10:22:45 +0100
From:   Halil Pasic <pasic@...ux.ibm.com>
To:     Petr Tesarik <petr.tesarik1@...wei-partners.com>
Cc:     Petr Tesařík <petr@...arici.cz>,
        Niklas Schnelle <schnelle@...ux.ibm.com>,
        Christoph Hellwig <hch@....de>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Robin Murphy <robin.murphy@....com>,
        Ross Lagerwall <ross.lagerwall@...rix.com>,
        linux-pci <linux-pci@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <iommu@...ts.linux.dev>,
        Matthew Rosato <mjrosato@...ux.ibm.com>,
        Halil Pasic <pasic@...ux.ibm.com>
Subject: Re: Memory corruption with CONFIG_SWIOTLB_DYNAMIC=y

On Wed, 8 Nov 2023 15:45:49 +0100
Petr Tesarik <petr.tesarik1@...wei-partners.com> wrote:

> On 11/8/2023 3:32 PM, Halil Pasic wrote:
> > On Wed, 8 Nov 2023 12:04:12 +0100
> > Petr Tesarik <petr.tesarik1@...wei-partners.com> wrote:
> > [..]  
> >>>
> >>> For the sake of simplicity let us assume we only have the min_align_mask
> >>> requirement. Then I believe the worst case is that we need 
> >>> (orig_addr & min_align_mask & PAGE_MASK)  + (min_align_mask & ~PAGE_MASK)
> >>> extra space to fit.
> >>>
> >>> Depending on how the semantics pan out one may be able to replace
> >>> min_align_mask with combined_mask.
> >>>
> >>> Is your point that for large combined_mask values 
> >>> _get_free_pages(GFP_NOWAIT | __GFP_NOWARN, required_order) is not
> >>> likely to complete successfully?    
> >>
> >> Yes, that's the reason. OTOH it's probably worth a try. The point is
> >> that mapping a DMA buffer is allowed to fail, so callers should be
> >> prepared anyway.
> >>
> >> And for the case you reported initially, I don't think there is any need
> >> to preserve bit 11 (0x800) from the original buffer's physical address,
> >> which is enough to fix it. See also my other email earlier today.  
> > 
> > Hm. Do you mean "[PATCH 1/1] swiotlb: fix out-of-bounds TLB allocations
> > with CONFIG_SWIOTLB_DYNAMIC" or a different one?
> > 
> > I only see "[PATCH 1/1] swiotlb: fix out-of-bounds TLB allocations
> > with CONFIG_SWIOTLB_DYNAMIC" but I don't think that one takes
> > care of "I don't think there is any need to preserve bit 11 (0x800)
> > from the original buffer's physical address".  
> 
> Yes, I mean only this patch. I want to fix memory corruption fast, while
> waiting for more feedback on my understanding of the alignment masks.
> What I'm trying to say is that your specific use case may not even need
> a bigger allocation if the page alignment should be interpreted differently.
> 
> Again, thank you for your in-depth inspection, because it's not totally
> clear how the various alignment parameters should be interpreted. It's
> difficult to write correct code then...

I fully understand. Thanks for tackling this. We decided to go with a bug
report and not with a fix because of the very same reasons: lack of
clarity on how certain things are supposed to work. Let us see how
the discussion develops. :)

Regards,
Halil

> 
> Petr T