Loosen RingBuffer lifetime requirements

With commit 0068735 ("Fix Map leaking file descriptors when not
managed by Object") we tightened the lifetime requirements surrounding
RingBuffer and RingBufferBuilder objects. Specifically, the new contract
was that the Map being passed to the RingBufferBuilder had to outlive
said object as well as any constructed RingBuffer.
In a conversation with Andrii Nakryiko <[email protected]>, he confirmed
that libbpf's API contract is grab a reference of all file descriptors
added to the ring buffer via ring_buffer__add().
As such, with this change we loosen lifetime requirements surround
RingBuffer and RingBufferBuilder accordingly.

Signed-off-by: Daniel Müller <[email protected]>

Author: d-e-s-o

libbpf-rs/src/ringbuf.rs

@@ -49,11 +49,11 @@ impl Debug for RingBufferCallback<'_> {
 /// between [`Program`][crate::Program]s and userspace. As of Linux 5.8, the
 /// `ringbuf` map is now preferred over the `perf buffer`.
 #[derive(Debug, Default)]
-pub struct RingBufferBuilder<'a> {
-    fd_callbacks: Vec<(BorrowedFd<'a>, RingBufferCallback<'a>)>,
+pub struct RingBufferBuilder<'slf, 'cb> {
+    fd_callbacks: Vec<(BorrowedFd<'slf>, RingBufferCallback<'cb>)>,
 }
 
-impl<'a> RingBufferBuilder<'a> {
+impl<'slf, 'cb: 'slf> RingBufferBuilder<'slf, 'cb> {
     /// Create a new `RingBufferBuilder` object.
     pub fn new() -> Self {
         RingBufferBuilder {
@@ -69,9 +69,9 @@ impl<'a> RingBufferBuilder<'a> {
     ///
     /// The callback provides a raw byte slice. You may find libraries such as
     /// [`plain`](https://crates.io/crates/plain) helpful.
-    pub fn add<NewF>(&mut self, map: &'a MapHandle, callback: NewF) -> Result<&mut Self>
+    pub fn add<NewF>(&mut self, map: &'slf MapHandle, callback: NewF) -> Result<&mut Self>
     where
-        NewF: FnMut(&[u8]) -> i32 + 'a,
+        NewF: FnMut(&[u8]) -> i32 + 'cb,
     {
         if map.map_type() != MapType::RingBuf {
             return Err(Error::InvalidInput("Must use a RingBuf map".into()));
@@ -82,7 +82,7 @@ impl<'a> RingBufferBuilder<'a> {
     }
 
     /// Build a new [`RingBuffer`]. Must have added at least one ringbuf.
-    pub fn build(self) -> Result<RingBuffer<'a>> {
+    pub fn build(self) -> Result<RingBuffer<'cb>> {
         let mut cbs = vec![];
         let mut ptr: Option<NonNull<libbpf_sys::ring_buffer>> = None;
         let c_sample_cb: libbpf_sys::ring_buffer_sample_fn = Some(Self::call_sample_cb);
@@ -145,10 +145,10 @@ impl<'a> RingBufferBuilder<'a> {
 /// between [`Program`][crate::Program]s and userspace. As of Linux 5.8, the
 /// `ringbuf` map is now preferred over the `perf buffer`.
 #[derive(Debug)]
-pub struct RingBuffer<'a> {
+pub struct RingBuffer<'cb> {
     ptr: NonNull<libbpf_sys::ring_buffer>,
     #[allow(clippy::vec_box)]
-    _cbs: Vec<Box<RingBufferCallback<'a>>>,
+    _cbs: Vec<Box<RingBufferCallback<'cb>>>,
 }
 
 impl RingBuffer<'_> {
@@ -199,7 +199,7 @@ mod test {
 
     /// Check that `RingBuffer` is `Send`.
     #[test]
-    fn perfbuffer_is_send() {
+    fn ringbuffer_is_send() {
         fn test<T>()
         where
             T: Send,

libbpf-rs/tests/test.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::let_unit_value)]
+
 use std::collections::HashSet;
 use std::ffi::c_void;
 use std::fs;
@@ -891,6 +893,57 @@ fn test_sudo_object_ringbuf_closure() {
     assert_eq!(v2, 2);
 }
 
+/// Check that `RingBuffer` works correctly even if the map file descriptors
+/// provided during construction are closed. This test validates that `libbpf`'s
+/// refcount behavior is correctly reflected in our `RingBuffer` lifetimes.
+#[test]
+fn test_sudo_object_ringbuf_with_closed_map() {
+    bump_rlimit_mlock();
+
+    fn test(poll_fn: impl FnOnce(&libbpf_rs::RingBuffer)) {
+        let mut value = 0i32;
+
+        {
+            let mut obj = get_test_object("tracepoint.bpf.o");
+            let prog = obj
+                .prog_mut("handle__tracepoint")
+                .expect("Failed to find program");
+
+            let _link = prog
+                .attach_tracepoint("syscalls", "sys_enter_getpid")
+                .expect("Failed to attach prog");
+
+            let map = obj.map("ringbuf").expect("Failed to get ringbuf map");
+
+            let callback = |data: &[u8]| {
+                plain::copy_from_bytes(&mut value, data).expect("Wrong size");
+                0
+            };
+
+            let mut builder = libbpf_rs::RingBufferBuilder::new();
+            builder.add(map, callback).expect("Failed to add ringbuf");
+            let ringbuf = builder.build().expect("Failed to build");
+
+            drop(obj);
+
+            // Trigger the tracepoint. At this point `map` along with the containing
+            // `obj` have been destroyed.
+            let _pid = unsafe { libc::getpid() };
+            let () = poll_fn(&ringbuf);
+        }
+
+        // If we see a 1 here the ring buffer was still working as expected.
+        assert_eq!(value, 1);
+    }
+
+    test(|ringbuf| ringbuf.consume().expect("Failed to consume ringbuf"));
+    test(|ringbuf| {
+        ringbuf
+            .poll(Duration::from_secs(5))
+            .expect("Failed to poll ringbuf")
+    });
+}
+
 #[test]
 fn test_sudo_object_task_iter() {
     bump_rlimit_mlock();