| 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
| ||
|
Date: Mon, 18 Mar 2024 13:36:16 +0100
From: Petr Tesařík <petr@...arici.cz>
To: Michael Kelley <mhklinux@...look.com>
Cc: Will Deacon <will@...nel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "kernel-team@...roid.com"
<kernel-team@...roid.com>, "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
Christoph Hellwig <hch@....de>, Marek Szyprowski
<m.szyprowski@...sung.com>, Robin Murphy <robin.murphy@....com>, Petr
Tesarik <petr.tesarik1@...wei-partners.com>, Dexuan Cui
<decui@...rosoft.com>, Nicolin Chen <nicolinc@...dia.com>
Subject: Re: [PATCH v6 1/6] swiotlb: Fix double-allocation of slots due to
broken alignment handling
On Mon, 18 Mar 2024 03:39:07 +0000
Michael Kelley <mhklinux@...look.com> wrote:
> From: Will Deacon <will@...nel.org> Sent: Friday, March 8, 2024 7:28 AM
> >
> >
> > Fix the problem by treating the allocation alignment separately to any
> > additional alignment requirements from the device, using the maximum
> > of the two as the stride to search the buffer slots and taking care
> > to ensure a minimum of page-alignment for buffers larger than a page.
> >
> > This also resolves swiotlb allocation failures occuring due to the
> > inclusion of ~PAGE_MASK in 'iotlb_align_mask' for large allocations and
> > resulting in alignment requirements exceeding swiotlb_max_mapping_size().
> >
> > Fixes: bbb73a103fbb ("swiotlb: fix a braino in the alignment check fix")
> > Fixes: 0eee5ae10256 ("swiotlb: fix slot alignment checks")
> > Cc: Christoph Hellwig <hch@....de>
> > Cc: Marek Szyprowski <m.szyprowski@...sung.com>
> > Cc: Robin Murphy <robin.murphy@....com>
> > Cc: Dexuan Cui <decui@...rosoft.com>
> > Reviewed-by: Michael Kelley <mhklinux@...look.com>
> > Reviewed-by: Petr Tesarik <petr.tesarik1@...wei-partners.com>
> > Tested-by: Nicolin Chen <nicolinc@...dia.com>
> > Signed-off-by: Will Deacon <will@...nel.org>
> > ---
> > kernel/dma/swiotlb.c | 28 +++++++++++++++-------------
> > 1 file changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> > index b079a9a8e087..2ec2cc81f1a2 100644
> > --- a/kernel/dma/swiotlb.c
> > +++ b/kernel/dma/swiotlb.c
> > @@ -982,7 +982,7 @@ static int swiotlb_search_pool_area(struct device
> > *dev, struct io_tlb_pool *pool
> > phys_to_dma_unencrypted(dev, pool->start) & boundary_mask;
> > unsigned long max_slots = get_max_slots(boundary_mask);
> > unsigned int iotlb_align_mask =
> > - dma_get_min_align_mask(dev) | alloc_align_mask;
> > + dma_get_min_align_mask(dev) & ~(IO_TLB_SIZE - 1);
> > unsigned int nslots = nr_slots(alloc_size), stride;
> > unsigned int offset = swiotlb_align_offset(dev, orig_addr);
> > unsigned int index, slots_checked, count = 0, i;
> > @@ -993,19 +993,18 @@ static int swiotlb_search_pool_area(struct device *dev, struct io_tlb_pool *pool
> > BUG_ON(!nslots);
> > BUG_ON(area_index >= pool->nareas);
> >
> > + /*
> > + * For mappings with an alignment requirement don't bother looping to
> > + * unaligned slots once we found an aligned one.
> > + */
> > + stride = get_max_slots(max(alloc_align_mask, iotlb_align_mask));
> > +
> > /*
> > * For allocations of PAGE_SIZE or larger only look for page aligned
> > * allocations.
> > */
> > if (alloc_size >= PAGE_SIZE)
> > - iotlb_align_mask |= ~PAGE_MASK;
> > - iotlb_align_mask &= ~(IO_TLB_SIZE - 1);
> > -
> > - /*
> > - * For mappings with an alignment requirement don't bother looping to
> > - * unaligned slots once we found an aligned one.
> > - */
> > - stride = (iotlb_align_mask >> IO_TLB_SHIFT) + 1;
> > + stride = umax(stride, PAGE_SHIFT - IO_TLB_SHIFT + 1);
> >
> > spin_lock_irqsave(&area->lock, flags);
> > if (unlikely(nslots > pool->area_nslabs - area->used))
> > @@ -1015,11 +1014,14 @@ static int swiotlb_search_pool_area(struct device *dev, struct io_tlb_pool *pool
> > index = area->index;
> >
> > for (slots_checked = 0; slots_checked < pool->area_nslabs; ) {
> > - slot_index = slot_base + index;
> > + phys_addr_t tlb_addr;
> >
> > - if (orig_addr &&
> > - (slot_addr(tbl_dma_addr, slot_index) &
> > - iotlb_align_mask) != (orig_addr & iotlb_align_mask)) {
> > + slot_index = slot_base + index;
> > + tlb_addr = slot_addr(tbl_dma_addr, slot_index);
> > +
> > + if ((tlb_addr & alloc_align_mask) ||
> > + (orig_addr && (tlb_addr & iotlb_align_mask) !=
> > + (orig_addr & iotlb_align_mask))) {
> > index = wrap_area_index(pool, index + 1);
> > slots_checked++;
> > continue;
> > --
>
> Question for IOMMU folks: alloc_align_mask is set only in
> iommu_dma_map_page(), using the IOMMU granule size.
> Can the granule ever be larger than PAGE_SIZE?
I don't feel as part of the IOMMU folks, but since I have spent one and
a half aeons looking around DMA-related code, let me answer anyway.
No, this is not possible, see here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/iova.c?h=v6.8#n61
HTH
Petr T
> If so,
> swiotlb_search_pool_area() can fail to find slots even when
> the swiotlb is empty.
>
> The failure happens when alloc_align_mask is larger than
> PAGE_SIZE and the alloc_size is the swiotlb max of 256 Kbytes
> (or even a bit smaller in some cases). The swiotlb memory
> pool is allocated in swiotlb_memblock_alloc() with PAGE_SIZE
> alignment. On x86/x64, if alloc_align_mask is 8191 and the
> pool start address is something like XXXX1000, slot 0 won't
> satisfy alloc_align_mask. Slot 1 satisfies alloc_align_mask,
> but has a size of 127 slots and can't fulfill a 256 Kbyte request.
> The problem repeats through the entire swiotlb and the
> allocation fails.
>
> Updating swiotlb_memblock_alloc() to use an alignment of
> IO_TLB_SIZE * IO_TLB_SEGSIZE (i.e., 256 Kbytes) solves the
> problem for all viable configurations.
>
> Michael
>
Powered by blists - more mailing lists