On Fri, 2026-06-19 at 15:54 +0200, Jiri Olsa wrote:
> On Thu, Jun 18, 2026 at 08:26:40PM -0400, Tamir Duberstein wrote:
> > ring_buffer__new() and ring_buffer__add() allow a NULL sample
> > callback. When callback-based consumption reaches such a ring, it calls
> > through the NULL function pointer and crashes.
> >
> > Check every ring before manager polling or consumption so a missing
> > callback returns -EINVAL before the manager waits or consumes another
> > ring. Check the selected ring before direct per-ring consumption. Perform
> > both checks before honoring a zero record bound so invalid callback use
> > always reports the error.
> >
> > Fixes: bf99c936f947 ("libbpf: Add BPF ring buffer support")
> > Assisted-by: Codex:gpt-5.5
> > Signed-off-by: Tamir Duberstein <[email protected]>
> > ---
> > tools/lib/bpf/libbpf.h | 11 ++-
> > tools/lib/bpf/ringbuf.c | 43 +++++++++--
> > tools/testing/selftests/bpf/prog_tests/ringbuf.c | 93
> > ++++++++++++++++++++++++
> > 3 files changed, 136 insertions(+), 11 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index bba4e8464396..9ba6b9ad3498 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -1526,18 +1526,17 @@ LIBBPF_API int ring__map_fd(const struct ring *r);
> > *
> > * @param r A ringbuffer object.
> > * @return The number of records consumed (or INT_MAX, whichever is less),
> > or
> > - * a negative number if any of the callbacks return an error.
> > + * a negative error code on failure.
> > */
> > LIBBPF_API int ring__consume(struct ring *r);
> >
> > /**
> > - * @brief **ring__consume_n()** consumes up to a requested amount of items
> > from
> > - * a ringbuffer without event polling.
> > + * @brief **ring__consume_n()** consumes up to a requested number of
> > records
> > + * from a ringbuffer without event polling.
> > *
> > * @param r A ringbuffer object.
> > - * @param n Maximum amount of items to consume.
> > - * @return The number of items consumed, or a negative number if any of the
> > - * callbacks return an error.
> > + * @param n Maximum number of records to consume.
> > + * @return The number of records consumed, or a negative error code on
> > failure.
> > */
> > LIBBPF_API int ring__consume_n(struct ring *r, size_t n);
> >
> > diff --git a/tools/lib/bpf/ringbuf.c b/tools/lib/bpf/ringbuf.c
> > index f2bb619d5a75..0dae4d95d309 100644
> > --- a/tools/lib/bpf/ringbuf.c
> > +++ b/tools/lib/bpf/ringbuf.c
> > @@ -231,6 +231,26 @@ static inline int roundup_len(__u32 len)
> > return (len + 7) / 8 * 8;
> > }
> >
> > +static int ringbuf_validate(const struct ring *r)
> > +{
> > + if (unlikely(!r->sample_cb))
> > + return -EINVAL;
> > + return 0;
> > +}
> > +
> > +static int ringbuf_validate_callbacks(const struct ring_buffer *rb)
> > +{
> > + int i, err;
> > +
> > + for (i = 0; i < rb->ring_cnt; i++) {
> > + err = ringbuf_validate(rb->rings[i]);
> > + if (err)
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int64_t ringbuf_process_ring(struct ring *r, size_t n)
> > {
> > int *len_ptr, len, err;
> > @@ -240,6 +260,9 @@ static int64_t ringbuf_process_ring(struct ring *r,
> > size_t n)
> > bool got_new_data;
> > void *sample;
> >
> > + err = ringbuf_validate(r);
> > + if (err)
> > + return err;
>
> as Emil noted in first version, this seems like overkill, could you
> just check sample_cb != NULL before it's actualy called?
>
> in [1] you mentioned you plan to add some feature that won't use sample_cb,
> it'd be easier to review/suggest the solution with more details on that
>
> jirka
As-is it appears that the simplest thing to do is to error out in
ring_buffer__new() and ring_buffer__add() when callback is NULL or
when overwrite map flag is present.
I checked our internal usage for then __new/__add functions,
and it seems there are no cases when NULL is passed as a callback.
[...]
