linux-kernel - Re: [PATCH 4/4] comedi: allocate DMA coherent buffer as individual pages

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] [day] [month] [year] [list]

Message-ID: <3cabb8d7-0b1f-4c95-a3d0-cc0d957354a1@mev.co.uk>
Date: Tue, 29 Apr 2025 16:59:36 +0100
From: Ian Abbott <abbotti@....co.uk>
To: Marek Szyprowski <m.szyprowski@...sung.com>,
 Christoph Hellwig <hch@....de>
Cc: linux-kernel@...r.kernel.org,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 H Hartley Sweeten <hsweeten@...ionengravers.com>,
 Robin Murphy <robin.murphy@....com>, iommu@...ts.linux.dev
Subject: Re: [PATCH 4/4] comedi: allocate DMA coherent buffer as individual
 pages

On 28/04/2025 16:33, Marek Szyprowski wrote:
> On 28.04.2025 14:56, Christoph Hellwig wrote:
>> On Tue, Apr 15, 2025 at 12:35:59PM +0100, Ian Abbott wrote:
>>> +			vma->vm_start = start;
>>> +			vma->vm_end = start + PAGE_SIZE;
>>> +			retval = dma_mmap_coherent(bm->dma_hw_dev, vma,
>>> +						   buf->virt_addr,
>>> +						   buf->dma_addr, PAGE_SIZE);
>> I'm not  fan of the vm_start/vm_end manipulation, but I've seen it in
>> other places.  In a perfect world we'd have a dma_mmap_coherent_offset
>> or similar helper that encapsulates it, and then maybe later replace
>> that hack with passing on the offset.
> 
> Indeed the dma_mmap_*() makes too many assumptions about the vma. The
> case You mentioned is probably in drivers/infiniband/hw/hfi1/file_ops.c
> but I also see that the vma->vm_pgoff is being adjusted before most
> dma_mmap_*() calls, which proves that the current API is somehow
> limited. It would be great to fix this too while touching the
> dma_mmap_attrs() API.

Drivers would probably have to continue manipulating vma->vm_pgoff 
anyway if they use its value in a special way, like 
drivers/infiniband/hw/hfil/file_ops.c or drivers/uio/uio.c.  The 
dma_mmap_*() calls already use vma->vm_pgoff as an offset into the VMA 
area, so I think all the new API would need is a parameter to restrict 
the number of pages being mapped, or something similar.

The new API doesn't necessarily have to be reflected all the way down to 
the dma_mmap_direct(), iommu_dma_mmap(), and ops->mmap() functions, as 
the new dma_mmap_*() function could modify vma->vm_end temporarily in 
order to restrict the number of pages being mapped by the lower-level 
functions.

-- 
-=( Ian Abbott <abbotti@....co.uk> || MEV Ltd. is a company  )=-
-=( registered in England & Wales.  Regd. number: 02862268.  )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-