From: Tamir Duberstein 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") Signed-off-by: Tamir Duberstein Signed-off-by: Kumar Kartikeya Dwivedi --- tools/lib/bpf/libbpf.h | 11 +++++------ tools/lib/bpf/ringbuf.c | 43 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h index a8a23c135c17..1c45648fa578 100644 --- a/tools/lib/bpf/libbpf.h +++ b/tools/lib/bpf/libbpf.h @@ -1628,18 +1628,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 6e88ac209298..b7fde7d0a0fd 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; if (n == 0) return 0; @@ -284,14 +307,17 @@ static int64_t ringbuf_process_ring(struct ring *r, size_t n) * records. * * Returns number of records consumed across all registered ring buffers (or - * n, whichever is less), or negative number if any of the callbacks return - * error. + * n, whichever is less), or a negative error code on failure. */ int ring_buffer__consume_n(struct ring_buffer *rb, size_t n) { int64_t err, res = 0; int i; + err = ringbuf_validate_callbacks(rb); + if (err) + return libbpf_err(err); + for (i = 0; i < rb->ring_cnt; i++) { struct ring *ring = rb->rings[i]; @@ -309,14 +335,17 @@ int ring_buffer__consume_n(struct ring_buffer *rb, size_t n) /* Consume available ring buffer(s) data without event polling. * Returns number of records consumed across all registered ring buffers (or - * INT_MAX, whichever is less), or negative number if any of the callbacks - * return error. + * INT_MAX, whichever is less), or a negative error code on failure. */ int ring_buffer__consume(struct ring_buffer *rb) { int64_t err, res = 0; int i; + err = ringbuf_validate_callbacks(rb); + if (err) + return libbpf_err(err); + for (i = 0; i < rb->ring_cnt; i++) { struct ring *ring = rb->rings[i]; @@ -334,13 +363,17 @@ int ring_buffer__consume(struct ring_buffer *rb) /* Poll for available data and consume records, if any are available. * Returns number of records consumed (or INT_MAX, whichever is less), or - * negative number, if any of the registered callbacks returned error. + * a negative error code on failure. */ int ring_buffer__poll(struct ring_buffer *rb, int timeout_ms) { int i, cnt; int64_t err, res = 0; + err = ringbuf_validate_callbacks(rb); + if (err) + return libbpf_err(err); + cnt = epoll_wait(rb->epoll_fd, rb->events, rb->ring_cnt, timeout_ms); if (cnt < 0) return libbpf_err(-errno); -- 2.53.0