Re: [PATCH bpf v2 2/8] libbpf: ringbuf: Prevent NULL callback crash

[Eduard Zingerman <eddyz87@gmail.com>]

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 <tamird@kernel.org>
> > ---
> >  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.

[...]