Hi James,
On 05.09.2020 17:50, James Bottomley wrote:
> [resend with correct linux-arch address]
> On Sat, 2020-09-05 at 15:35 +0800, Hillf Danton wrote:
>> On Fri, 04 Sep 2020 08:34:39 -0700 James Bottomley wrote:
>>> 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@xxxxxxxxxxxx>
>>>> Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
>>>> Cc: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
>>>> Cc: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
>>>> Signed-off-by: Hillf Danton <hdanton@xxxxxxxx>
>>>> ---
>>>>
>>>> --- 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);
>> Hi James
>>> This is massively expensive on PARISC and likely other VIPT/VIVT
>>> architectures.
>> Correct.
>>
>>> What's the reason for clearing it? This could also be
>> /* we always manually zero the memory once we are done: */
>> gfp &= ~__GFP_ZERO;
>> gfp |= dma_direct_optimal_gfp_mask(dev, dev->coherent_dma_mask,
>> &phys_limit);
> That's not a reason ... that comment was put in for coherent mappings.
> What is the reason we should incur all this expense for clearing pages
> which aren't unmapped in the kernel, because we can update the comment?
> The usual rationale for kernel mapped pages is security, because they
> may leak information but unmapped pages shouldn't have this problem.
Any dma_alloc_attrs() buffer might be mmaped to userspace, so the
security reason is still valid. Possible lack if kernel mapping was only
a hint that driver doesn't need it, so it might be skipped on some
architectures, where creating it requires significant resources (i.e.
vmalloc area).
>>> 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.
>> I'm open to any vintage ideas in your mind wrt clearing dma buf e.g
>> on platforms like PARISC. Or feel free to offload me the work if it
>> makes sense to you who are rich of PARISC knowledge.
> OK, I've cc'd linux-arch because this is a problem for more than just
> parisc. However, not having to do it is the best solution ... sort of
> the doctor, doctor it hurts when I do this answer.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
