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]

Date:   Wed, 8 Nov 2023 15:45:49 +0100
From:   Petr Tesarik <petr.tesarik1@...wei-partners.com>
To:     Halil Pasic <pasic@...ux.ibm.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>
Subject: Re: Memory corruption with CONFIG_SWIOTLB_DYNAMIC=y

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...

Petr T