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:   Wed, 5 Apr 2023 07:32:06 +0200
From:   Petr Tesařík <petr@...arici.cz>
To:     Dexuan Cui <decui@...rosoft.com>
Cc:     Dexuan-Linux Cui <dexuan.linux@...il.com>,
        Petr Tesarik <petrtesarik@...weicloud.com>,
        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>,
        "Michael Kelley (LINUX)" <mikelley@...rosoft.com>,
        Tianyu Lan <Tianyu.Lan@...rosoft.com>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>
Subject: Re: [PATCH v1 2/2] swiotlb: Fix slot alignment checks

On Wed, 5 Apr 2023 05:11:42 +0000
Dexuan Cui <decui@...rosoft.com> wrote:

> > From: Petr Tesařík <petr@...arici.cz>
> > Sent: Tuesday, April 4, 2023 9:40 PM  
> > > > ...
> > > > 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, everythidiff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 5b919ef832b6..8d87cb69769b 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -639,8 +639,8 @@ static int swiotlb_do_find_slots(struct device *dev, int area_index,
 	 * allocations.
 	 */
 	if (alloc_size >= PAGE_SIZE)
-		iotlb_align_mask &= PAGE_MASK;
-	iotlb_align_mask &= alloc_align_mask;
+		iotlb_align_mask |= ~PAGE_MASK;
+	iotlb_align_mask |= alloc_align_mask;
 
 	/*
 	 * For mappings with an alignment requirement don't bother looping to
ng works fine.  
> > >
> > > The log is pasted below. Looks like the SCSI driver hv_storvsc fails to
> > > detect the disk capacity:  
> > 
> > The first thing I can imagine is that there are in fact no (free) slots
> > in the SWIOTLB which match the alignment constraints, so the map
> > operation fails. However, this would result in a "swiotlb buffer is
> > full" message in the log, and I can see no such message in the log
> > excerpt you have posted.
> > 
> > Please, can you check if there are any "swiotlb" messages preceding the
> > first error message?
> > 
> > Petr T  
> 
> There is no "swiotlb buffer is full" error.
> 
> The hv_storvsc driver (drivers/scsi/storvsc_drv.c) calls scsi_dma_map(),
> which doesn't return -ENOMEM when the failure happens.

I see...

Argh, you're right. This is a braino. The alignment mask is in fact an
INVERTED mask, i.e. it masks off bits that are not relevant for the
alignment. The more strict alignment needed the more bits must be set,
so the individual alignment constraints must be combined with an OR
instead of an AND.

Can you apply the following change and check if it fixes the issue?

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 5b919ef832b6..8d87cb69769b 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -639,8 +639,8 @@ static int swiotlb_do_find_slots(struct device *dev, int area_index,
 	 * allocations.
 	 */
 	if (alloc_size >= PAGE_SIZE)
-		iotlb_align_mask &= PAGE_MASK;
-	iotlb_align_mask &= alloc_align_mask;
+		iotlb_align_mask |= ~PAGE_MASK;
+	iotlb_align_mask |= alloc_align_mask;
 
 	/*
 	 * For mappings with an alignment requirement don't bother looping to


Petr T