On Fri, Jul 10, 2026 at 9:18 PM 寿柏能 <[email protected]> wrote: > > Hi T.J., > > Thanks for the review. > > You are right, the export tracing was inadvertently lost when splitting > dma_buf_fd() apart. I've fixed it in v3 by introducing a new > dma_buf_fd_install() helper in dma-buf.c that wraps the DMA_BUF_TRACE > call together with fd_install(), and using that in dma-heap.c instead > of a bare fd_install(). The trace event fires after copy_to_user() > succeeds, just before the fd becomes visible to userspace — the same > observable moment as before. > > > Baineng
Thanks Baineng, that LGTM. Please have my RB: Reviewed-by: T.J. Mercier <[email protected]> You'll probably want to resend this with a "[PATCH v3]" subject though. Originally I wasn't sure if we really needed dma_buf_fd_install exported, but I looked around and see that fastrpc_dmabuf_alloc (drivers/misc/fastrpc.c), gntdev_ioctl_dmabuf_exp_from_refs (drivers/xen/gntdev-dmabuf.c), and I think video_usercopy (drivers/media/v4l2-core/v4l2-ioctl.c) also suffer from this same problem. Want to take a stab at those too? :) -T.J. > > T.J. Mercier <[email protected]> 于2026年7月11日周六 04:20写道： >> >> On Fri, Jul 10, 2026 at 4:06 AM <[email protected]> wrote: >> > >> > Thank you for your contribution! Sashiko AI review found 1 potential >> > issue(s) to consider: >> > - [Medium] Bypassing the standard dma_buf_fd() API skips the emission of >> > the trace_dma_buf_fd tracepoint. >> > -- >> > >> > --- Patch [1]: [PATCH v2] dma-buf: dma-heap: don't publish fd before >> > copy_to_user() succeeds --- >> > commit 869904722955c74d0c6e8c974d3c64c6befabac6 >> > Author: Baineng Shou <[email protected]> >> > >> > dma-buf: dma-heap: don't publish fd before copy_to_user() succeeds >> > >> > This commit restructures DMA_HEAP_IOCTL_ALLOC to delay publishing the >> > file descriptor until after copy_to_user() succeeds. It open-codes the >> > fd allocation and installation steps instead of using dma_buf_fd() to >> > prevent an fd leak on copy_to_user() failure. >> > >> > Link: >> > https://lore.kernel.org/dri-devel/[email protected]/ >> > >> > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c >> > > index 3937dd41bb0f6..983dee947c851 100644 >> > > --- a/drivers/dma-buf/dma-heap.c >> > > +++ b/drivers/dma-buf/dma-heap.c >> > [ ... ] >> > > @@ -175,15 +162,28 @@ static long dma_heap_ioctl(struct file *file, >> > > unsigned int ucmd, >> > > >> > > switch (kcmd) { >> > > case DMA_HEAP_IOCTL_ALLOC: >> > > - ret = dma_heap_ioctl_allocate(file, kdata); >> > > + dmabuf = dma_heap_ioctl_allocate(file, kdata); >> > > + >> > > + if (IS_ERR(dmabuf)) { >> > > + ret = PTR_ERR(dmabuf); >> > > + break; >> > > + } >> > > + >> > > + fd = ((struct dma_heap_allocation_data *)kdata)->fd; >> > > + if (copy_to_user((void __user *)arg, kdata, out_size) != >> > > 0) { >> > > + put_unused_fd(fd); >> > > + dma_buf_put(dmabuf); >> > > + ret = -EFAULT; >> > > + } else { >> > > + fd_install(fd, dmabuf->file); >> > >> > [Severity: Medium] >> > By open-coding the fd reservation and installation here instead of using >> > dma_buf_fd(), does this skip the emission of the trace_dma_buf_fd >> > tracepoint? >> > >> > If so, observability tools, BPF scripts, and perf tracing relying on >> > trace_dma_buf_fd to map dmabufs to file descriptors will silently miss all >> > allocations originating from the dma-heap subsystem. >> >> Hi Xiang Gao, >> >> It looks like this change in its current form breaks the export >> tracing that you introduced. It looks fixable by adding a call to >> DMA_BUF_TRACE in after copy_to_user in dma_heap_ioctl. However I'm >> wondering if you're aware of the dmabuf iterator that traverses the >> dmabuf_list? I've backported it to 5.10, so it's available on all our >> kernels: https://r.android.com/q/Iaeda328f7693acc1506269c12cac5e7955e5c091 >> Someone else from Xiaomi even helped with the 6.12 backport. So I >> guess I'm wondering if the dmabuf iterator would work for you instead >> of the tracepoints. >> >> Thanks, >> T.J. >> >> > > + } >> > > + >> > > break; >> > > default: >> > > ret = -ENOTTY; >> > > goto err; >> > > } >> > >> > -- >> > Sashiko AI review · >> > https://sashiko.dev/#/patchset/[email protected]?part=1
