Skip to content

Commit aa060c0

Browse files
d-e-s-odanielocfb
authored andcommitted
Fix potential memory leaks in RingBufferBuilder::build()
When we build up a RingBuffer object via the RingBufferBuilder::build() function, we may leak memory on error paths, because of the usage of raw pointers. Fix the issue by: 1) stop converting boxes into raw pointers to begin with, which should be fine given that libbpf does not actually dereference the sample callback pointer in ring_buffer__new() or ring_buffer__add(). 2) properly freeing the ring buffer if we return early because we failed to successfully add a callback. Closes: #862 Signed-off-by: Daniel Müller <[email protected]>
1 parent 7362311 commit aa060c0

File tree

2 files changed

+15
-5
lines changed

2 files changed

+15
-5
lines changed

libbpf-rs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ Unreleased
2222
- Adjusted `OpenMap::set_inner_map_fd` to return `Result`
2323
- Adjusted `ProgramInput::context_in` field to be a mutable reference
2424
- Made inner `query::Tag` contents publicly accessible
25+
- Fixed potential memory leak in `RingBufferBuilder::build`
2526
- Removed `Display` implementation of various `enum` types
2627

2728

libbpf-rs/src/ringbuf.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::fmt::Debug;
33
use std::fmt::Formatter;
44
use std::fmt::Result as FmtResult;
55
use std::ops::Deref as _;
6+
use std::ops::DerefMut as _;
67
use std::os::raw::c_ulong;
78
use std::os::unix::prelude::AsRawFd;
89
use std::os::unix::prelude::BorrowedFd;
@@ -89,40 +90,48 @@ impl<'slf, 'cb: 'slf> RingBufferBuilder<'slf, 'cb> {
8990
let c_sample_cb: libbpf_sys::ring_buffer_sample_fn = Some(Self::call_sample_cb);
9091

9192
for (fd, callback) in self.fd_callbacks {
92-
let sample_cb_ptr = Box::into_raw(Box::new(callback));
93+
let mut sample_cb = Box::new(callback);
9394
match rb_ptr {
9495
None => {
9596
// Allocate a new ringbuf manager and add a ringbuf to it
97+
// SAFETY: All pointers are valid or rightly NULL.
98+
// The object referenced by `sample_cb` is
99+
// not modified by `libbpf`
96100
let ptr = unsafe {
97101
libbpf_sys::ring_buffer__new(
98102
fd.as_raw_fd(),
99103
c_sample_cb,
100-
sample_cb_ptr as *mut _,
104+
sample_cb.deref_mut() as *mut _ as *mut _,
101105
null_mut(),
102106
)
103107
};
104108
let ptr = validate_bpf_ret(ptr).context("failed to create new ring buffer")?;
105109
rb_ptr = Some(ptr)
106110
}
107-
Some(ptr) => {
111+
Some(mut ptr) => {
108112
// Add a ringbuf to the existing ringbuf manager
113+
// SAFETY: All pointers are valid or rightly NULL.
114+
// The object referenced by `sample_cb` is
115+
// not modified by `libbpf`
109116
let err = unsafe {
110117
libbpf_sys::ring_buffer__add(
111118
ptr.as_ptr(),
112119
fd.as_raw_fd(),
113120
c_sample_cb,
114-
sample_cb_ptr as *mut _,
121+
sample_cb.deref_mut() as *mut _ as *mut _,
115122
)
116123
};
117124

118125
// Handle errors
119126
if err != 0 {
127+
// SAFETY: The pointer is valid.
128+
let () = unsafe { libbpf_sys::ring_buffer__free(ptr.as_mut()) };
120129
return Err(Error::from_raw_os_error(err));
121130
}
122131
}
123132
}
124133

125-
unsafe { cbs.push(Box::from_raw(sample_cb_ptr)) };
134+
let () = cbs.push(sample_cb);
126135
}
127136

128137
match rb_ptr {

0 commit comments

Comments
 (0)