linux-kernel - Re: [PATCH] MA-21654 Use dma_alloc_pages in vb2_dma_sg_alloc

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, 26 Sep 2023 08:50:49 +0200
From:   Christoph Hellwig <hch@....de>
To:     Tomasz Figa <tfiga@...omium.org>
Cc:     Fang Hui <hui.fang@....com>, Christoph Hellwig <hch@....de>,
        Robin Murphy <robin.murphy@....com>, m.szyprowski@...sung.com,
        mchehab@...nel.org, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org, anle.pan@....com, xuegang.liu@....com
Subject: Re: [PATCH] MA-21654 Use dma_alloc_pages in
 vb2_dma_sg_alloc_compacted

On Wed, Sep 20, 2023 at 04:41:08PM +0900, Tomasz Figa wrote:
> On Thu, Sep 14, 2023 at 4:41 PM Fang Hui <hui.fang@....com> wrote:
> >
> > On system with "CONFIG_ZONE_DMA32=y", if the allocated physical address is
> 
> First of all, thanks a lot for the patch! Please check my review comments below.
> 
> Is CONFIG_ZONE_DMA32 really the factor that triggers the problem? My
> understanding was that the problem was that the hardware has 32-bit
> DMA, but the system has physical memory at addresses beyond the first
> 4G.

You should NEVER disable CONFIG_ZONE_DMA32 for a system that has
memory > 4GB.  I've made this point repeatedly, but the ARM64 maintainers
insist on making it configurable instead of selecting it like most other
64-bit architetures that aren't guaranteed to always use a IOMMU.

We need to stop that.

> Hmm, when I was proposing dma_alloc_pages(), I missed that it returns
> a DMA handle. That on its own can be handled by saving the returned
> handles somewhere in struct vb2_dma_sg_buf, but there is a bigger
> problem - the function would actually create a mapping if the DMA
> device requires some mapping management (e.g. is behind an IOMMU),
> which is undesirable, because we create the mapping ourselves below
> anyway...
> 
> @Christoph Hellwig @Robin Murphy  I need your thoughts on this as
> well. Would it make sense to have a variant of dma_alloc_pages() that
> only allocates the pages, but doesn't perform the mapping? (Or a flag
> that tells the implementation to skip creating a mapping.)

dma_map_pages needs to map the pages as part of finding out that the
allocation actually works.  So skipping it can't really be done.

So why do you want to create your own mapping anyway?