[PATCH] bpf: make dmabuf iterator stay drained after NULL

[PATCH] bpf: make dmabuf iterator stay drained after NULL

[Kang Chen]

bpf_iter_dmabuf_next() uses kit->dmabuf == NULL to mean "iteration has
not started".  dma_buf_iter_next() also returns NULL at the end of the
list, so a next() call after exhaustion restarts from the list head.

The verifier relies on the iterator contract that once next() returns
NULL it keeps returning NULL: process_iter_next_call() marks the
iterator DRAINED and treats the result of later next() calls as the
constant zero.  A restarted dmabuf iteration therefore hands the BPF
program a kernel pointer while the verifier believes R0 == 0.

Use an invalid pointer value as the drained sentinel so the iterator
state keeps its one-pointer ABI, and skip dma_buf_put() for it.

Fixes: 6eab7ac7c5ee ("bpf: Add open coded dmabuf iterator")
Signed-off-by: Kang Chen <k.chen@smail.nju.edu.cn>
---
 kernel/bpf/dmabuf_iter.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/dmabuf_iter.c b/kernel/bpf/dmabuf_iter.c
index cd500248abd9..a2b7c38f9ae6 100644
--- a/kernel/bpf/dmabuf_iter.c
+++ b/kernel/bpf/dmabuf_iter.c
@@ -156,6 +156,13 @@ struct bpf_iter_dmabuf_kern {
 	struct dma_buf *dmabuf;
 } __aligned(8);
 
+/*
+ * The iterator state is one pointer wide because struct bpf_iter_dmabuf is
+ * part of the generated vmlinux.h ABI.  Use an invalid pointer value as the
+ * "drained" sentinel instead of growing the state.
+ */
+#define DMABUF_ITER_DONE	((struct dma_buf *)1L)
+
 __bpf_kfunc_start_defs();
 
 __bpf_kfunc int bpf_iter_dmabuf_new(struct bpf_iter_dmabuf *it)
@@ -173,11 +180,25 @@ __bpf_kfunc struct dma_buf *bpf_iter_dmabuf_next(struct bpf_iter_dmabuf *it)
 {
 	struct bpf_iter_dmabuf_kern *kit = (void *)it;
 
+	/*
+	 * The verifier assumes that an iterator which returned NULL keeps
+	 * returning NULL (see process_iter_next_call()).  Without this check
+	 * the NULL left by the last dma_buf_iter_next() would be interpreted
+	 * as "not started" and the iteration would restart from the list head.
+	 */
+	if (kit->dmabuf == DMABUF_ITER_DONE)
+		return NULL;
+
 	if (kit->dmabuf)
 		kit->dmabuf = dma_buf_iter_next(kit->dmabuf);
 	else
 		kit->dmabuf = dma_buf_iter_begin();
 
+	if (!kit->dmabuf) {
+		kit->dmabuf = DMABUF_ITER_DONE;
+		return NULL;
+	}
+
 	return kit->dmabuf;
 }
 
@@ -185,7 +206,7 @@ __bpf_kfunc void bpf_iter_dmabuf_destroy(struct bpf_iter_dmabuf *it)
 {
 	struct bpf_iter_dmabuf_kern *kit = (void *)it;
 
-	if (kit->dmabuf)
+	if (kit->dmabuf && kit->dmabuf != DMABUF_ITER_DONE)
 		dma_buf_put(kit->dmabuf);
 }
 
-- 
2.34.1

Re: [PATCH] bpf: make dmabuf iterator stay drained after NULL

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2299 bytes --]

> bpf: make dmabuf iterator stay drained after NULL

TL;DR: Nit only: the new comment in bpf_iter_dmabuf_next() describes the
pre-patch restart behavior, not the code it sits on; the review suggests
rewording it to explain why the sentinel exists.

> diff --git a/kernel/bpf/dmabuf_iter.c b/kernel/bpf/dmabuf_iter.c
> --- a/kernel/bpf/dmabuf_iter.c
> +++ b/kernel/bpf/dmabuf_iter.c

[ ... ]

> @@ -173,11 +180,25 @@ __bpf_kfunc struct dma_buf *bpf_iter_dmabuf_next(struct bpf_iter_dmabuf *it)
>  {
>  	struct bpf_iter_dmabuf_kern *kit = (void *)it;
>  
> +	/*
> +	 * The verifier assumes that an iterator which returned NULL keeps
> +	 * returning NULL (see process_iter_next_call()).  Without this check
> +	 * the NULL left by the last dma_buf_iter_next() would be interpreted
> +	 * as "not started" and the iteration would restart from the list head.
> +	 */
> +	if (kit->dmabuf == DMABUF_ITER_DONE)
> +		return NULL;
> +
>  	if (kit->dmabuf)
>  		kit->dmabuf = dma_buf_iter_next(kit->dmabuf);
>  	else
>  		kit->dmabuf = dma_buf_iter_begin();
>  
> +	if (!kit->dmabuf) {
> +		kit->dmabuf = DMABUF_ITER_DONE;
> +		return NULL;
> +	}
> +

This isn't a bug, but is the second sentence of the comment above
describing the pre-patch behavior rather than the code it sits on?

With this patch no NULL is ever left in kit->dmabuf, since the block
above replaces it with DMABUF_ITER_DONE:

    if (!kit->dmabuf) {
            kit->dmabuf = DMABUF_ITER_DONE;
            return NULL;
    }

If only the "if (kit->dmabuf == DMABUF_ITER_DONE)" check were removed, the
iteration would not restart from the list head.  The sentinel would take
the "if (kit->dmabuf)" branch and be passed to
dma_buf_iter_next((struct dma_buf *)1).

The restart described is the bug being fixed, which is already in the
changelog.  Could the comment instead say why the sentinel exists, for
example: once begin()/next() returns NULL, park the iterator at
DMABUF_ITER_DONE so it is not mistaken for a fresh (NULL) iterator, and
keep returning NULL as process_iter_next_call() requires?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/42982427559

Re: [PATCH] bpf: make dmabuf iterator stay drained after NULL

[Christian König]

Adding T.J. to comment.

Regards,
Christian.

On 11/10/26 13:51, Kang Chen wrote:
> bpf_iter_dmabuf_next() uses kit->dmabuf == NULL to mean "iteration has
> not started".  dma_buf_iter_next() also returns NULL at the end of the
> list, so a next() call after exhaustion restarts from the list head.
> 
> The verifier relies on the iterator contract that once next() returns
> NULL it keeps returning NULL: process_iter_next_call() marks the
> iterator DRAINED and treats the result of later next() calls as the
> constant zero.  A restarted dmabuf iteration therefore hands the BPF
> program a kernel pointer while the verifier believes R0 == 0.
> 
> Use an invalid pointer value as the drained sentinel so the iterator
> state keeps its one-pointer ABI, and skip dma_buf_put() for it.
> 
> Fixes: 6eab7ac7c5ee ("bpf: Add open coded dmabuf iterator")
> Signed-off-by: Kang Chen <k.chen@smail.nju.edu.cn>
> ---
>  kernel/bpf/dmabuf_iter.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/dmabuf_iter.c b/kernel/bpf/dmabuf_iter.c
> index cd500248abd9..a2b7c38f9ae6 100644
> --- a/kernel/bpf/dmabuf_iter.c
> +++ b/kernel/bpf/dmabuf_iter.c
> @@ -156,6 +156,13 @@ struct bpf_iter_dmabuf_kern {
>         struct dma_buf *dmabuf;
>  } __aligned(8);
> 
> +/*
> + * The iterator state is one pointer wide because struct bpf_iter_dmabuf is
> + * part of the generated vmlinux.h ABI.  Use an invalid pointer value as the
> + * "drained" sentinel instead of growing the state.
> + */
> +#define DMABUF_ITER_DONE       ((struct dma_buf *)1L)
> +
>  __bpf_kfunc_start_defs();
> 
>  __bpf_kfunc int bpf_iter_dmabuf_new(struct bpf_iter_dmabuf *it)
> @@ -173,11 +180,25 @@ __bpf_kfunc struct dma_buf *bpf_iter_dmabuf_next(struct bpf_iter_dmabuf *it)
>  {
>         struct bpf_iter_dmabuf_kern *kit = (void *)it;
> 
> +       /*
> +        * The verifier assumes that an iterator which returned NULL keeps
> +        * returning NULL (see process_iter_next_call()).  Without this check
> +        * the NULL left by the last dma_buf_iter_next() would be interpreted
> +        * as "not started" and the iteration would restart from the list head.
> +        */
> +       if (kit->dmabuf == DMABUF_ITER_DONE)
> +               return NULL;
> +
>         if (kit->dmabuf)
>                 kit->dmabuf = dma_buf_iter_next(kit->dmabuf);
>         else
>                 kit->dmabuf = dma_buf_iter_begin();
> 
> +       if (!kit->dmabuf) {
> +               kit->dmabuf = DMABUF_ITER_DONE;
> +               return NULL;
> +       }
> +
>         return kit->dmabuf;
>  }
> 
> @@ -185,7 +206,7 @@ __bpf_kfunc void bpf_iter_dmabuf_destroy(struct bpf_iter_dmabuf *it)
>  {
>         struct bpf_iter_dmabuf_kern *kit = (void *)it;
> 
> -       if (kit->dmabuf)
> +       if (kit->dmabuf && kit->dmabuf != DMABUF_ITER_DONE)
>                 dma_buf_put(kit->dmabuf);
>  }
> 
> --
> 2.34.1
> 

Re: [PATCH] bpf: make dmabuf iterator stay drained after NULL

[T.J. Mercier]

On Wed, Nov 11, 2026 at 12:18 AM Christian König
<christian.koenig@amd.com> wrote:
>
> Adding T.J. to comment.
>
> Regards,
> Christian.

Thanks I've had to aggressively filter emails, so for the most part I
only see stuff where I'm on To: (and I still can't read them all). I
would have expected get_maintainer.pl to add me here.



> On 11/10/26 13:51, Kang Chen wrote:
> > bpf_iter_dmabuf_next() uses kit->dmabuf == NULL to mean "iteration has
> > not started".  dma_buf_iter_next() also returns NULL at the end of the
> > list, so a next() call after exhaustion restarts from the list head.
> >
> > The verifier relies on the iterator contract that once next() returns
> > NULL it keeps returning NULL: process_iter_next_call() marks the
> > iterator DRAINED and treats the result of later next() calls as the
> > constant zero.  A restarted dmabuf iteration therefore hands the BPF
> > program a kernel pointer while the verifier believes R0 == 0.
> >
> > Use an invalid pointer value as the drained sentinel so the iterator
> > state keeps its one-pointer ABI, and skip dma_buf_put() for it.
> >
> > Fixes: 6eab7ac7c5ee ("bpf: Add open coded dmabuf iterator")
> > Signed-off-by: Kang Chen <k.chen@smail.nju.edu.cn>
> > ---
> >  kernel/bpf/dmabuf_iter.c | 23 ++++++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/dmabuf_iter.c b/kernel/bpf/dmabuf_iter.c
> > index cd500248abd9..a2b7c38f9ae6 100644
> > --- a/kernel/bpf/dmabuf_iter.c
> > +++ b/kernel/bpf/dmabuf_iter.c
> > @@ -156,6 +156,13 @@ struct bpf_iter_dmabuf_kern {
> >         struct dma_buf *dmabuf;
> >  } __aligned(8);
> >
> > +/*
> > + * The iterator state is one pointer wide because struct bpf_iter_dmabuf is
> > + * part of the generated vmlinux.h ABI.  Use an invalid pointer value as the
> > + * "drained" sentinel instead of growing the state.
> > + */
> > +#define DMABUF_ITER_DONE       ((struct dma_buf *)1L)
> > +
> >  __bpf_kfunc_start_defs();
> >
> >  __bpf_kfunc int bpf_iter_dmabuf_new(struct bpf_iter_dmabuf *it)
> > @@ -173,11 +180,25 @@ __bpf_kfunc struct dma_buf *bpf_iter_dmabuf_next(struct bpf_iter_dmabuf *it)
> >  {
> >         struct bpf_iter_dmabuf_kern *kit = (void *)it;
> >
> > +       /*
> > +        * The verifier assumes that an iterator which returned NULL keeps
> > +        * returning NULL (see process_iter_next_call()).  Without this check
> > +        * the NULL left by the last dma_buf_iter_next() would be interpreted
> > +        * as "not started" and the iteration would restart from the list head.
> > +        */
> > +       if (kit->dmabuf == DMABUF_ITER_DONE)
> > +               return NULL;
> > +
> >         if (kit->dmabuf)
> >                 kit->dmabuf = dma_buf_iter_next(kit->dmabuf);
> >         else
> >                 kit->dmabuf = dma_buf_iter_begin();
> >
> > +       if (!kit->dmabuf) {
> > +               kit->dmabuf = DMABUF_ITER_DONE;
> > +               return NULL;
> > +       }
> > +
> >         return kit->dmabuf;
> >  }
> >
> > @@ -185,7 +206,7 @@ __bpf_kfunc void bpf_iter_dmabuf_destroy(struct bpf_iter_dmabuf *it)
> >  {
> >         struct bpf_iter_dmabuf_kern *kit = (void *)it;
> >
> > -       if (kit->dmabuf)
> > +       if (kit->dmabuf && kit->dmabuf != DMABUF_ITER_DONE)
> >                 dma_buf_put(kit->dmabuf);
> >  }
> >
> > --
> > 2.34.1
> >
>

[PATCH bpf] selftests/bpf: Test that a drained dmabuf iterator keeps returning NULL

[T.J. Mercier]

The BPF iterator docs in bpf_iterators.rst specify that the next
function of an open-coded iterator should keep returning NULL after it
has been returned once.

Add a subtest that checks this for the dmabuf iterator. It drains an
iterator, calls bpf_iter_dmabuf_next() once more and checks that the
result is NULL. The dma-bufs the test creates keep the list from being
empty, so an iterator that starts over would return one of them.

The existing open_coded subtest uses bpf_for_each(), which stops at the
first NULL and never calls next() on a drained iterator.

Signed-off-by: T.J. Mercier <tjmercier@google.com>
---
This depends on Kang Chen's fix and fails without it:
https://lore.kernel.org/bpf/20261110125106.2935677-1-k.chen@smail.nju.edu.cn/
Kang, feel free to carry it as 2/2 in your v2.

 .../selftests/bpf/prog_tests/dmabuf_iter.c    | 19 +++++++++++++++++++
 .../testing/selftests/bpf/progs/dmabuf_iter.c | 19 +++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c b/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
index 1a5dc1adae73..c1f81b5dccdc 100644
--- a/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c
@@ -265,6 +265,23 @@ static void subtest_dmabuf_iter_check_open_coded(struct dmabuf_iter *skel, int m
 	} while (bpf_map_get_next_key(map_fd, key, key));
 }
 
+static void subtest_dmabuf_iter_check_next_after_drain(struct dmabuf_iter *skel)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	int err, fd;
+
+	fd = bpf_program__fd(skel->progs.iter_dmabuf_next_after_drain);
+
+	err = bpf_prog_test_run_opts(fd, &topts);
+	if (!ASSERT_OK(err, "test_run_opts err"))
+		return;
+	if (!ASSERT_OK(topts.retval, "test_run_opts retval"))
+		return;
+
+	/* The test buffers exist, so a restarted iteration would find one */
+	ASSERT_EQ(skel->data->next_after_drain, 0, "next_after_drain");
+}
+
 void test_dmabuf_iter(void)
 {
 	struct dmabuf_iter *skel = NULL;
@@ -315,6 +332,8 @@ void test_dmabuf_iter(void)
 	}
 	if (test__start_subtest("open_coded"))
 		subtest_dmabuf_iter_check_open_coded(skel, map_fd);
+	if (test__start_subtest("next_after_drain"))
+		subtest_dmabuf_iter_check_next_after_drain(skel);
 
 destroy:
 	destroy_test_buffers();
diff --git a/tools/testing/selftests/bpf/progs/dmabuf_iter.c b/tools/testing/selftests/bpf/progs/dmabuf_iter.c
index 9cbb7442646e..d5421b75c39c 100644
--- a/tools/testing/selftests/bpf/progs/dmabuf_iter.c
+++ b/tools/testing/selftests/bpf/progs/dmabuf_iter.c
@@ -99,3 +99,22 @@ int iter_dmabuf_for_each(const void *ctx)
 
 	return 0;
 }
+
+/* Result of a bpf_iter_dmabuf_next() call after the iterator was drained */
+__u64 next_after_drain = -1;
+
+SEC("syscall")
+int iter_dmabuf_next_after_drain(const void *ctx)
+{
+	struct bpf_iter_dmabuf it;
+
+	bpf_iter_dmabuf_new(&it);
+	while (bpf_iter_dmabuf_next(&it))
+		;
+
+	/* Store the raw value; the verifier would remove a branch on it */
+	next_after_drain = (__u64)bpf_iter_dmabuf_next(&it);
+	bpf_iter_dmabuf_destroy(&it);
+
+	return 0;
+}
-- 
2.56.0.567.gdb80ae8cd5-goog

Re: [PATCH] bpf: make dmabuf iterator stay drained after NULL

[T.J. Mercier]

On Tue, Nov 10, 2026 at 4:57 AM Kang Chen <k.chen@smail.nju.edu.cn> wrote:
>
> bpf_iter_dmabuf_next() uses kit->dmabuf == NULL to mean "iteration has
> not started".  dma_buf_iter_next() also returns NULL at the end of the
> list, so a next() call after exhaustion restarts from the list head.
>
> The verifier relies on the iterator contract that once next() returns
> NULL it keeps returning NULL: process_iter_next_call() marks the
> iterator DRAINED and treats the result of later next() calls as the
> constant zero.  A restarted dmabuf iteration therefore hands the BPF
> program a kernel pointer while the verifier believes R0 == 0.
>
> Use an invalid pointer value as the drained sentinel so the iterator
> state keeps its one-pointer ABI, and skip dma_buf_put() for it.
>
> Fixes: 6eab7ac7c5ee ("bpf: Add open coded dmabuf iterator")
> Signed-off-by: Kang Chen <k.chen@smail.nju.edu.cn>

Thanks for this fix. The cgroup (bpf_iter_css_next) and scx_dsq
(bpf_iter_scx_dsq_next) iterators look like they also have this
problem.

> ---
>  kernel/bpf/dmabuf_iter.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/dmabuf_iter.c b/kernel/bpf/dmabuf_iter.c
> index cd500248abd9..a2b7c38f9ae6 100644
> --- a/kernel/bpf/dmabuf_iter.c
> +++ b/kernel/bpf/dmabuf_iter.c
> @@ -156,6 +156,13 @@ struct bpf_iter_dmabuf_kern {
>         struct dma_buf *dmabuf;
>  } __aligned(8);
>
> +/*
> + * The iterator state is one pointer wide because struct bpf_iter_dmabuf is
> + * part of the generated vmlinux.h ABI.  Use an invalid pointer value as the
> + * "drained" sentinel instead of growing the state.
> + */

This comment explains why this patch uses a sentinel instead of
growing the state, but that doesn't help anyone reading this code in
the future. This belongs in the commit message, not a code comment.
The real constraint is that the size of bpf_iter_dmabuf is
user-visible API, so it can't grow without breaking backwards
compatibility.

> +#define DMABUF_ITER_DONE       ((struct dma_buf *)1L)

I would prefer this to be DMABUF_ITER_START to mirror the kmem_cache
iterator. Then the second if (!kit->dmabuf) block in
bpf_iter_dmabuf_next() below could also be removed.

> +
>  __bpf_kfunc_start_defs();
>
>  __bpf_kfunc int bpf_iter_dmabuf_new(struct bpf_iter_dmabuf *it)
> @@ -173,11 +180,25 @@ __bpf_kfunc struct dma_buf *bpf_iter_dmabuf_next(struct bpf_iter_dmabuf *it)
>  {
>         struct bpf_iter_dmabuf_kern *kit = (void *)it;
>
> +       /*
> +        * The verifier assumes that an iterator which returned NULL keeps
> +        * returning NULL (see process_iter_next_call()).  Without this check
> +        * the NULL left by the last dma_buf_iter_next() would be interpreted
> +        * as "not started" and the iteration would restart from the list head.
> +        */

Same here, this is commit message material. If you want to add a code
comment, referencing the rule from bpf_iterators.rst should be enough:

/* Once NULL is returned, subsequent next calls should keep returning NULL. */

> +       if (kit->dmabuf == DMABUF_ITER_DONE)
> +               return NULL;
> +
>         if (kit->dmabuf)
>                 kit->dmabuf = dma_buf_iter_next(kit->dmabuf);
>         else
>                 kit->dmabuf = dma_buf_iter_begin();
>
> +       if (!kit->dmabuf) {
> +               kit->dmabuf = DMABUF_ITER_DONE;
> +               return NULL;
> +       }
> +
>         return kit->dmabuf;
>  }
>
> @@ -185,7 +206,7 @@ __bpf_kfunc void bpf_iter_dmabuf_destroy(struct bpf_iter_dmabuf *it)
>  {
>         struct bpf_iter_dmabuf_kern *kit = (void *)it;
>
> -       if (kit->dmabuf)
> +       if (kit->dmabuf && kit->dmabuf != DMABUF_ITER_DONE)
>                 dma_buf_put(kit->dmabuf);
>  }
>
> --
> 2.34.1

I think we should also add a test for this. I've sent one as a reply
to this thread. It fails without your fix and passes with it. Feel
free to add it as 2/2 in v2.