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

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
>

Reply via email to