Re: [PATCH] dma-buf: system_heap: Reject private mappings in system_heap_mmap()

["Christian König" <christian.koenig@amd.com>]

On 10/31/26 02:44, T.J. Mercier wrote:
> On Fri, Oct 30, 2026 at 1:44 PM Chris Wiseman <chriswiseman@google.com> wrote:
>>
>> When commit 381295758792 ("dma-buf: system_heap: Rework system heap to
>> use sgtables instead of pagelists") stopped using heap_helpers.c and
>> added a custom system_heap_mmap(), the check rejecting private mappings
>> was dropped.
>>
>> Other exporters and buffer allocators reject private or copy-on-write
>> mappings in their mmap handlers: cma_heap_mmap() and mmap_udmabuf()
>> check that VM_SHARED or VM_MAYSHARE is set, and drm_gem_shmem_mmap()
>> and ttm_bo_mmap_obj() reject vma_is_cow_mapping(vma).
>>
>> However, system_heap_mmap() omits this check and calls remap_pfn_range()
>> even when userspace requests MAP_PRIVATE with PROT_WRITE. As a result,
>> subsequent CPU writes to a MAP_PRIVATE mapping of a system heap buffer
>> fault into do_wp_page() -> wp_page_copy(), silently allocating private
>> anonymous copy-on-write pages whose writes never reach the underlying
>> system heap buffer pages.
>>
>> Reject non-shared mappings in system_heap_mmap() with -EINVAL to match
>> cma_heap_mmap() and mmap_udmabuf().
>>
>> Fixes: 381295758792 ("dma-buf: system_heap: Rework system heap to use sgtables instead of pagelists")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Chris Wiseman <chriswiseman@google.com>
> 
> Reviewed-by: T.J. Mercier <tjmercier@google.com>
> 
> I wonder if we should have this in dma_buf_mmap_internal() though. Any
> thoughts Sumit or Christian?

I do remember that this was once already discussed with Simona and a couple of others, but I don't remember what the outcome was.

On the other hand I can't think of a reason why anybody would want a private mapping with DMA-buf copy on write clearly won't work. Maybe for backward compatibility with broken userspace, not for a real use case.

I would say let's push this patch to drm-misc-fixes and apply another patch to drm-misc-next to enforce this for all DMA-bufs and see if anybody complains that this breaks userspace.

Should I push it to drm-misc-fixes or want somebody else take that?

Regards,
Christian.