libbpf / libbpf-rs

Minimal and opinionated eBPF tooling for the Rust ecosystem
Other
731 stars 129 forks source link

Memory leak on error path in `RingBufferBuilder::build` #862

Closed AnthonyMikh closed 1 month ago

AnthonyMikh commented 1 month ago

RingBufferBuilder::build loops over saved callbacks and converts them into raw pointers via Box::into_raw(Box::new(callback)) before handing them over to libbpf. On each iteration after adding a callback it is reconstructed with Box::from_raw and pushed into a vector which is passed into returned RingBuffer for proper cleanup.

However callback reconstruction only happens after calling ring_buffer__new or ring_buffer__add. Both functions can return an error. If it happens, RingBufferBuilder::build returns early and does not call Box::from_raw on callback pointer, thus leaking memory.

Also in case of an error ring_buffer__free is not called, so memory for ring buffer leaks as well.

danielocfb commented 1 month ago

Thanks for the report. Will you work on a fix @AnthonyMikh ?

AnthonyMikh commented 1 month ago

I will probably, it does not seem to be so hard. I am not sure though if the whole RingBufferBuilder is worth keeping. What advantage does it have over adding callback addition methods to RingBuffer directly?

danielocfb commented 1 month ago

I suspect one would want to prevent addition of callbacks to a ring buffer that is already in use. Having a build step that creates another object would help with that and prevent accidental wrong usage.

AnthonyMikh commented 1 month ago

I suspect one would want to prevent addition of callbacks to a ring buffer that is already in use

Yeah, makes sense