linux-kernel - Re: [PATCH] dma-direct: zero out 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>] [day] [month] [year] [list]

Message-ID: <1599233679.5231.4.camel@HansenPartnership.com>
Date:   Fri, 04 Sep 2020 08:34:39 -0700
From:   James Bottomley <James.Bottomley@...senPartnership.com>
To:     Hillf Danton <hdanton@...a.com>, Christoph Hellwig <hch@....de>
Cc:     linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Kees Cook <keescook@...omium.org>,
        Matthew Wilcox <willy@...radead.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: [PATCH] dma-direct: zero out DMA_ATTR_NO_KERNEL_MAPPING buf

On Fri, 2020-09-04 at 23:25 +0800, Hillf Danton wrote:
> The DMA buffer allocated is always cleared in DMA core and this is
> making DMA_ATTR_NO_KERNEL_MAPPING non-special.
> 
> Fixes: d98849aff879 ("dma-direct: handle DMA_ATTR_NO_KERNEL_MAPPING
> in common code")
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Matthew Wilcox <willy@...radead.org>
> Cc: Marek Szyprowski <m.szyprowski@...sung.com>
> Cc: James Bottomley <James.Bottomley@...senPartnership.com>
> Signed-off-by: Hillf Danton <hdanton@...a.com>
> ---
> 
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -178,9 +178,17 @@ void *dma_direct_alloc_pages(struct devi
>  
>  	if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
>  	    !force_dma_unencrypted(dev)) {
> +		int i;
> +
>  		/* remove any dirty cache lines on the kernel alias
> */
>  		if (!PageHighMem(page))
>  			arch_dma_prep_coherent(page, size);
> +
> +		for (i = 0; i < size/PAGE_SIZE; i++) {
> +			ret = kmap_atomic(page + i);
> +			memset(ret, 0, PAGE_SIZE);
> +			kunmap_atomic(ret);

This is massively expensive on PARISC and likely other VIPT/VIVT
architectures.  What's the reason for clearing it?  This could also be
really inefficient even on PIPT architectures if the memory is device
remote.

If we really have to do this, it should likely be done in the arch or
driver hooks because there are potentially more efficient ways we can
do this knowing how the architecture behaves.

James