Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <1599233679.5231.4.camel@HansenPartnership.com>
Subject: Re: [PATCH] dma-direct: zero out DMA_ATTR_NO_KERNEL_MAPPING buf
From:   James Bottomley <James.Bottomley@HansenPartnership.com>
To:     Hillf Danton <hdanton@sina.com>, Christoph Hellwig <hch@lst.de>
Cc:     linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org,
        Kees Cook <keescook@chromium.org>,
        Matthew Wilcox <willy@infradead.org>,
        Marek Szyprowski <m.szyprowski@samsung.com>
Date:   Fri, 04 Sep 2020 08:34:39 -0700
In-Reply-To: <20200904152550.17964-1-hdanton@sina.com>
References: <20200904152550.17964-1-hdanton@sina.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@chromium.org>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: Hillf Danton <hdanton@sina.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