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

Message-ID: <20250428125648.GC27794@lst.de>
Date: Mon, 28 Apr 2025 14:56:48 +0200
From: Christoph Hellwig <hch@....de>
To: Ian Abbott <abbotti@....co.uk>
Cc: linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	H Hartley Sweeten <hsweeten@...ionengravers.com>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Robin Murphy <robin.murphy@....com>, iommu@...ts.linux.dev
Subject: Re: [PATCH 4/4] comedi: allocate DMA coherent buffer as individual
 pages

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.

> +			if (retval)
> +				break;
> +
> +			start += PAGE_SIZE;
> +		}
> +		vma->vm_start = vm_start;
> +		vma->vm_end = vm_end;
>  	} else {
>  		for (i = 0; i < n_pages; ++i) {
>  			unsigned long pfn;
> @@ -2407,19 +2421,18 @@ static int comedi_mmap(struct file *file, struct vm_area_struct *vma)
>  
>  			start += PAGE_SIZE;
>  		}
> +	}
>  
>  #ifdef CONFIG_MMU
> -		/*
> -		 * Leaving behind a partial mapping of a buffer we're about to
> -		 * drop is unsafe, see remap_pfn_range_notrack().
> -		 * We need to zap the range here ourselves instead of relying
> -		 * on the automatic zapping in remap_pfn_range() because we call
> -		 * remap_pfn_range() in a loop.
> -		 */
> -		if (retval)
> -			zap_vma_ptes(vma, vma->vm_start, size);
> +	/*
> +	 * Leaving behind a partial mapping of a buffer we're about to drop is
> +	 * unsafe, see remap_pfn_range_notrack().  We need to zap the range
> +	 * here ourselves instead of relying on the automatic zapping in
> +	 * remap_pfn_range() because we call remap_pfn_range() in a loop.
> +	 */
> +	if (retval)
> +		zap_vma_ptes(vma, vma->vm_start, size);
>  #endif
> -	}
>  
>  	if (retval == 0) {
>  		vma->vm_ops = &comedi_vm_ops;
> -- 
> 2.47.2
---end quoted text---