linux-kernel - Re: Regression due to d98849aff879 (dma-direct: handle DMA_ATTR_NO_KERNEL

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: <1565191457.2323.41.camel@pengutronix.de>
Date:   Wed, 07 Aug 2019 17:24:17 +0200
From:   Lucas Stach <l.stach@...gutronix.de>
To:     Christoph Hellwig <hch@....de>
Cc:     "Lendacky, Thomas" <Thomas.Lendacky@....com>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Thiago Jung Bauermann <bauerman@...ux.ibm.com>,
        Halil Pasic <pasic@...ux.ibm.com>
Subject: Re: Regression due to d98849aff879 (dma-direct: handle
 DMA_ATTR_NO_KERNEL_MAPPING in common code)

Am Dienstag, den 06.08.2019, 17:44 +0200 schrieb Christoph Hellwig:
> On Tue, Aug 06, 2019 at 04:06:58PM +0200, Lucas Stach wrote:
> > 
> > dma_direct_free_pages() then needs the same check, as otherwise the cpu
> > address is treated as a cookie instead of a real address and the
> > encryption needs to be re-enabled.
> 
> Ok, lets try this one instead:
> 
> --
> From 3a7aa9fe38a5eae5d879831b4f8c1032e735a0b6 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@....de>
> Date: Tue, 6 Aug 2019 14:33:23 +0300
> Subject: dma-direct: fix DMA_ATTR_NO_KERNEL_MAPPING
> 
> The new DMA_ATTR_NO_KERNEL_MAPPING needs to actually assign
> a dma_addr to work.  Also skip it if the architecture needs
> forced decryption handling, as that needs a kernel virtual
> address.
> 
> Fixes: d98849aff879 (dma-direct: handle DMA_ATTR_NO_KERNEL_MAPPING in common code)
> Signed-off-by: Christoph Hellwig <hch@....de>
> ---
>  kernel/dma/direct.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 59bdceea3737..4c211c87a719 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -130,11 +130,13 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size,
>  	if (!page)
>  		return NULL;
>  
> -	if (attrs & DMA_ATTR_NO_KERNEL_MAPPING) {
> +	if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
> +	    !force_dma_unencrypted(dev)) {
>  		/* remove any dirty cache lines on the kernel alias */
>  		if (!PageHighMem(page))
>  			arch_dma_prep_coherent(page, size);
>  		/* return the page pointer as the opaque cookie */
> +		*dma_handle = phys_to_dma(dev, page_to_phys(page));

I would suggest to place this line above the comment, as the comment
only really applies to the return value. Other than this nitpick, this
matches my understanding of the required changes, so:

Reviewed-by: Lucas Stach <l.stach@...gutronix.de>


>  		return page;
>  	}
>  
> @@ -178,7 +180,8 @@ void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr,
>  {
>  	unsigned int page_order = get_order(size);
>  
> -	if (attrs & DMA_ATTR_NO_KERNEL_MAPPING) {
> +	if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
> +	    !force_dma_unencrypted(dev)) {
>  		/* cpu_addr is a struct page cookie, not a kernel address */
>  		__dma_direct_free_pages(dev, size, cpu_addr);
>  		return;