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

["Christian König" <christian.koenig@amd.com>]

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
>