linux-kernel - Re: [PATCH v1 2/2] swiotlb: Fix slot alignment checks

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:   Tue, 4 Apr 2023 12:55:29 -0700
From:   Dexuan-Linux Cui <dexuan.linux@...il.com>
To:     Petr Tesarik <petrtesarik@...weicloud.com>
Cc:     Christoph Hellwig <hch@....de>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Robin Murphy <robin.murphy@....com>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        Jianxiong Gao <jxgao@...gle.com>,
        David Stevens <stevensd@...omium.org>,
        Joerg Roedel <jroedel@...e.de>,
        "open list:DMA MAPPING HELPERS" <iommu@...ts.linux.dev>,
        open list <linux-kernel@...r.kernel.org>,
        Roberto Sassu <roberto.sassu@...wei.com>, petr@...arici.cz,
        Michael Kelley <mikelley@...rosoft.com>,
        Dexuan Cui <decui@...rosoft.com>, Tianyu.Lan@...rosoft.com,
        linux-hyperv@...r.kernel.org
Subject: Re: [PATCH v1 2/2] swiotlb: Fix slot alignment checks

On Tue, Mar 21, 2023 at 1:37 AM Petr Tesarik
<petrtesarik@...weicloud.com> wrote:
>
> From: Petr Tesarik <petr.tesarik.ext@...wei.com>
>
> Explicit alignment and page alignment are used only to calculate
> the stride, not when checking actual slot physical address.
>
> Originally, only page alignment was implemented, and that worked,
> because the whole SWIOTLB is allocated on a page boundary, so
> aligning the start index was sufficient to ensure a page-aligned
> slot.
>
> When Christoph Hellwig added support for min_align_mask, the index
> could be incremented in the search loop, potentially finding an
> unaligned slot if minimum device alignment is between IO_TLB_SIZE
> and PAGE_SIZE. The bug could go unnoticed, because the slot size
> is 2 KiB, and the most common page size is 4 KiB, so there is no
> alignment value in between.
>
> IIUC the intention has been to find a slot that conforms to all
> alignment constraints: device minimum alignment, an explicit
> alignment (given as function parameter) and optionally page
> alignment (if allocation size is >= PAGE_SIZE). The most
> restrictive mask can be trivially computed with logical AND. The
> rest can stay.
>
> Fixes: 1f221a0d0dbf ("swiotlb: respect min_align_mask")
> Fixes: e81e99bacc9f ("swiotlb: Support aligned swiotlb buffers")
> Signed-off-by: Petr Tesarik <petr.tesarik.ext@...wei.com>
> ---
>  kernel/dma/swiotlb.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 3856e2b524b4..5b919ef832b6 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -634,22 +634,26 @@ static int swiotlb_do_find_slots(struct device *dev, int area_index,
>         BUG_ON(!nslots);
>         BUG_ON(area_index >= mem->nareas);
>
> +       /*
> +        * 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 &= alloc_align_mask;
> +
>         /*
>          * For mappings with an alignment requirement don't bother looping to
> -        * unaligned slots once we found an aligned one.  For allocations of
> -        * PAGE_SIZE or larger only look for page aligned allocations.
> +        * unaligned slots once we found an aligned one.
>          */
>         stride = (iotlb_align_mask >> IO_TLB_SHIFT) + 1;
> -       if (alloc_size >= PAGE_SIZE)
> -               stride = max(stride, stride << (PAGE_SHIFT - IO_TLB_SHIFT));
> -       stride = max(stride, (alloc_align_mask >> IO_TLB_SHIFT) + 1);
>
>         spin_lock_irqsave(&area->lock, flags);
>         if (unlikely(nslots > mem->area_nslabs - area->used))
>                 goto not_found;
>
>         slot_base = area_index * mem->area_nslabs;
> -       index = wrap_area_index(mem, ALIGN(area->index, stride));
> +       index = area->index;
>
>         for (slots_checked = 0; slots_checked < mem->area_nslabs; ) {
>                 slot_index = slot_base + index;
> --
> 2.39.2
>

Hi Petr, this patch has gone into the mainline:
0eee5ae10256 ("swiotlb: fix slot alignment checks")

Somehow it breaks Linux VMs on Hyper-V: a regular VM with
swiotlb=force or a confidential VM (which uses swiotlb) fails to boot.
If I revert this patch, everything works fine.

Cc'd Tianyu/Michael and the Hyper-V list.

Thanks,
Dexuan