linux-kernel - Re: [PATCH 2/4] bpf: Add dmabuf iterator

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAADnVQ+AesNdq_LB+MWxLnHbU08Zrn-8VgwY4+0PKuk7PmRd+w@mail.gmail.com>
Date: Tue, 22 Apr 2025 16:00:50 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: "T.J. Mercier" <tjmercier@...gle.com>
Cc: Song Liu <song@...nel.org>, Sumit Semwal <sumit.semwal@...aro.org>, 
	Christian König <christian.koenig@....com>, 
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Shuah Khan <skhan@...uxfoundation.org>, LKML <linux-kernel@...r.kernel.org>, 
	linux-media@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	linaro-mm-sig@...ts.linaro.org, 
	"open list:DOCUMENTATION" <linux-doc@...r.kernel.org>, bpf <bpf@...r.kernel.org>, 
	"open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>, android-mm@...gle.com, simona@...ll.ch, 
	Jonathan Corbet <corbet@....net>, Eduard <eddyz87@...il.com>, 
	Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, 
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, Jiri Olsa <jolsa@...nel.org>, 
	Mykola Lysenko <mykolal@...com>
Subject: Re: [PATCH 2/4] bpf: Add dmabuf iterator

On Tue, Apr 22, 2025 at 12:57 PM T.J. Mercier <tjmercier@...gle.com> wrote:
>
> On Mon, Apr 21, 2025 at 4:39 PM Alexei Starovoitov
> <alexei.starovoitov@...il.com> wrote:
> >
> > On Mon, Apr 21, 2025 at 1:40 PM T.J. Mercier <tjmercier@...gle.com> wrote:
> > >
> > > > > new file mode 100644
> > > > > index 000000000000..b4b8be1d6aa4
> > > > > --- /dev/null
> > > > > +++ b/kernel/bpf/dmabuf_iter.c
> > > >
> > > > Maybe we should add this file to drivers/dma-buf. I would like to
> > > > hear other folks thoughts on this.
> > >
> > > This is fine with me, and would save us the extra
> > > CONFIG_DMA_SHARED_BUFFER check that's currently needed in
> > > kernel/bpf/Makefile but would require checking CONFIG_BPF instead.
> > > Sumit / Christian any objections to moving the dmabuf bpf iterator
> > > implementation into drivers/dma-buf?
> >
> > The driver directory would need to 'depends on BPF_SYSCALL'.
> > Are you sure you want this?
> > imo kernel/bpf/ is fine for this.
>
> I don't have a strong preference so either way is fine with me. The
> main difference I see is maintainership.
>
> > You also probably want
> > .feature                = BPF_ITER_RESCHED
> > in bpf_dmabuf_reg_info.
>
> Thank you, this looks like a good idea.
>
> > Also have you considered open coded iterator for dmabufs?
> > Would it help with the interface to user space?
>
> I read through the open coded iterator patches, and it looks like they
> would be slightly more efficient by avoiding seq_file overhead. As far
> as the interface to userspace, for the purpose of replacing what's
> currently exposed by CONFIG_DMABUF_SYSFS_STATS I don't think there is
> a difference. However it looks like if I were to try to replace all of
> our userspace analysis of dmabufs with a single bpf program then an
> open coded iterator would make that much easier. I had not considered
> attempting that.
>
> One problem I see with open coded iterators is that support is much
> more recent (2023 vs 2020). We support longterm stable kernels (back
> to 5.4 currently but probably 5.10 by the time this would be used), so
> it seems like it would be harder to backport the kernel support for an
> open-coded iterator that far since it only goes back as far as 6.6
> now. Actually it doesn't look like it is possible while also
> maintaining the stable ABI we provide to device vendors. Which means
> we couldn't get rid of the dmabuf sysfs stats userspace dependency
> until 6.1 EOL in Dec. 2027. :\ So I'm in favor of a traditional bpf
> iterator here for now.

Fair enough, but please implement both and backport only
the old style pinned iterator.
The code will be mostly shared between them.
bpf_iter_dmabuf_new/_next will be more flexible with more
options to return data to user space. Like android can invent
their own binary format. Pack into it in a bpf prog, send to
bpf ringbuf and unmarshal efficiently in user space.
Instead of being limited to text output that pinned iterators
are supposed to do usually.
You can do binary with bpf_seq_write() too, but it's rare.

Also please provide full bpf prog that you'll use in production
in a selftest instead of trivial:
+SEC("iter/dmabuf")
+int dmabuf_collector(struct bpf_iter__dmabuf *ctx)

just to make sure it's tested end to end and future changes
won't break it.

pw-bot: cr