Re: [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs

[Laurent Pinchart]

On Mon, Oct 19, 2026 at 04:48:13PM +0200, Christian König wrote:
> On 10/19/26 14:44, Laurent Pinchart wrote:
> > On Mon, Oct 19, 2026 at 09:14:07AM +0200, Karl Mehltretter wrote:
> >> UVC and vivid access imported DMA-BUFs with the CPU without cache
> >> synchronization. On a Pi 400, captured data remained in the CPU cache
> >> while physical RAM was stale.
> >>
> >> Add an opt-in queue flag to bracket CPU access from buf_prepare() through
> >> buf_finish(). The allocator cannot imply CPU use because some vb2-vmalloc
> >> drivers pass buffers to hardware. Patch 1 balances finish operations in
> >> VIDIOC_REMOVE_BUFS so removing prepared buffers closes this interval.
> > 
> > This seems completely wrong.
> 
> It looks a bit questionable from 1000 miles above, but I might be wrong.
> 
> Let me iterate the rules here:
> 
> Accessing imported DMA-buf with the CPU from userspace should only be
> possible by using the dma_buf_mmap() interface and that includes the
> DMA_BUF_IOCTL_SYNC IOCTL to flush buffers dirtied by the CPU. If that
> is not done this way then that would be a *major* security issue, but
> I guess that this isn't the issue here.
> 
> What the drivers do more likely is that they use the vmap interface to
> access the DMA-buf with the CPU inside the kernel. Correct?

This patch series introduces a big penalty in the common case where a
userspace application consumes the frame with the CPU. There should be
no cache invalidation in that case.

> In this case dma_buf_begin_cpu_access() and dma_buf_end_cpu_access()
> should be called to properly frame that access.
> 
> That seems to be added in patch #2, but I absolutely can't judge if
> that is correct or not since at the same time you speak about userptr
> access and the code really looks a bit like spaghetti. 
> 
> >> This follows the per-queue flag proposed here:
> >> https://lists.openwall.net/linux-kernel/2024/06/20/236
> >>
> >> Based on Linux 7.3. Tests cover the same code on these earlier bases:
> >>
> >> - Pi 400, 7.3-rc4, all four patches: eight 640x480 YUYV frames per run,
> >>   imported CMA DMA-BUFs with a DRM device mapping:
> >>
> >>   Path                          Runs   Stale frames
> >>   UVC, vb2-vmalloc               12        0/96
> >>   vivid, vb2-vmalloc             12        0/96
> >>   vivid, vb2-dma-contig          12        0/96
> >>
> >>   All six lifecycle cases passed with both vivid allocators. Read-only
> >>   output USERPTR passed. The test added no warnings or failed CPU-access
> >>   callbacks. A control without the DRM mapping found 4/16 stale frames.
> >>
> >> - SAM9X75 (ARMv5), 6.18 backport of all four patches: the baseline
> >>   PREPARE_BUF/REMOVE_BUFS imbalance was fixed. USERPTR and six lifecycle
> >>   cases passed. All 128 vivid and 96 UVC frames matched RAM, with 100/100
> >>   begin/end calls. The baseline was already coherent.
> >>
> >> - BeagleV Ahead (RISC-V), 7.3-rc7, patches 1, 2 and 4: read-only USERPTR,
> >>   six traced lifecycle cases and 60 repeats passed with balanced callbacks
> >>   and no kernel diagnostics. A vendor 5.10 backport of patches 2 and 3
> >>   captured 96 UVC frames without stale data, with 100/100 begin/end calls.
> >>   Its baseline was already coherent.
> >>
> >> Karl Mehltretter (4):
> >>   media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS
> >>   media: videobuf2: add opt-in DMA-BUF CPU access
> >>   media: uvcvideo: synchronize imported DMA-BUFs for CPU access
> >>   media: vivid: synchronize imported DMA-BUFs for CPU access
> >>
> >>  .../media/common/videobuf2/videobuf2-core.c   | 96 ++++++++++++++++++-
> >>  .../common/videobuf2/videobuf2-dma-contig.c   |  5 +-
> >>  .../media/common/videobuf2/videobuf2-dma-sg.c |  5 +-
> >>  .../common/videobuf2/videobuf2-vmalloc.c      |  5 +-
> >>  drivers/media/test-drivers/vivid/vivid-core.c |  1 +
> >>  drivers/media/usb/uvc/uvc_queue.c             |  1 +
> >>  include/media/videobuf2-core.h                | 12 ++-
> >>  7 files changed, 116 insertions(+), 9 deletions(-)
> >>
> >>
> >> base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222

-- 
Regards,

Laurent Pinchart