linux-kernel - Re: MT76x2U crashes XHCI driver on AMD Ryzen system

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]

Message-ID: <20190219110851.GA23223@redhat.com>
Date:   Tue, 19 Feb 2019 12:08:52 +0100
From:   Stanislaw Gruszka <sgruszka@...hat.com>
To:     Robin Murphy <robin.murphy@....com>
Cc:     Lorenzo Bianconi <lorenzo.bianconi@...hat.com>,
        Samuel Sieb <samuel@...b.net>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        linux-kernel@...r.kernel.org, iommu@...ts.linux-foundation.org,
        Rosen Penev <rosenp@...il.com>,
        Alexander Duyck <alexander.h.duyck@...ux.intel.com>
Subject: Re: MT76x2U crashes XHCI driver on AMD Ryzen system

On Mon, Feb 18, 2019 at 05:01:59PM +0000, Robin Murphy wrote:
> On 18/02/2019 14:37, Stanislaw Gruszka wrote:
> [...]
> >Another issue is that dma_map_sg() & dma_map_page() may require some
> >constraints. I'm not sure about that and I want to clarify that with
> >CCed mm maintainers. I think DMA drivers may expect sg->offset < PAGE_SIZE
> >for both dma_map_sg() and dma_map_page(). Additionally dma_map_page()
> >maight expect that offset & length specify buffer within one page.
> 
> Luckily, this came up a while back[1] and we seemed to reach a
> consensus that sg->offset >= PAGE_SIZE for dma_map_sg() was weird
> but valid. IIRC it was only the Intel IOMMU code which failed to
> handle that case appropriately (and which I fixed) - the AMD IOMMU
> code always looked like it should be OK, but I'm not sure I've ever
> seen definitive test results (and I don't have hardware to do so
> myself).

Funny that we have problems on AMD IOMMU and not with Intel IOMMU.

> For dma_map_page(), length >= PAGE_SIZE should be perfectly valid
> and handled correctly. The offset >= PAGE_SIZE case is a bit harder
> to justify, but at the same time has less scope for the DMA API
> backend to get it wrong, so either way is likely to be OK in
> practice (in particular the AMD IOMMU code looks like it won't have
> a problem, since its map_page() implementation converts page and
> offset to a plain physical address before doing anything else).

Thanks for clarify this. So my patch which do:

-		page = virt_to_head_page(data);
+		page = virt_to_page(data);
 		offset = data - page_address(page);
 		sg_set_page(&urb->sg[i], page, sglen, offset);

should not be necessary as IOMMU driver do exactly the same internally.

Are there any alignment requirement for offset for dma_map_{page,sg} ?
It will work with let say sg->offset=113 or we have make sure it is
aligned to some boundary. If so, what boundary ?

Stanislaw