Re: Memory corruption with CONFIG_SWIOTLB_DYNAMIC=y

From: Halil Pasic
Date: Wed Nov 08 2023 - 09:33:24 EST


On Wed, 8 Nov 2023 12:04:12 +0100
Petr Tesarik <petr.tesarik1@xxxxxxxxxxxxxxxxxxx> 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". But it should take care of
the corruption, I agree with that. I hope to provide review for that
patch by the end of the day.

Regards,
Halil