bpfman: Failed dispatcher load corrupts DB

If the TC or XDP dispatcher fails to load, subsequent load commands fail with a panic in bpfman. To fix, removed an unwrap(), add a dispatcher delete in the Dispatcher.new() function when an error occurs, and mapped some errors to new BpfmanError for dispatcher failures instead of just returning lower level error. Signed-off-by: Billy McFall <[email protected]>
Billy99 · Aug 13, 2024 · 3115d6a · 3115d6a
1 parent f379196
commit 3115d6a
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 18 deletions.
diff --git a/bpfman/src/errors.rs b/bpfman/src/errors.rs
@@ -69,6 +69,8 @@ pub enum BpfmanError {
     BtfError(#[from] aya::BtfError),
     #[error("Failed to acquire database lock, please try again later")]
     DatabaseLockError,
+    #[error("dispatcher failed to load. {0}")]
+    DispatcherLoadError(String),
 }
 
 #[derive(Error, Debug)]

diff --git a/bpfman/src/multiprog/mod.rs b/bpfman/src/multiprog/mod.rs
@@ -54,8 +54,13 @@ impl Dispatcher {
                 let mut x =
                     XdpDispatcher::new(root_db, xdp_mode, if_index, if_name.to_string(), revision)?;
 
-                x.load(root_db, programs, old_dispatcher, image_manager)
-                    .await?;
+                if let Err(res) = x
+                    .load(root_db, programs, old_dispatcher, image_manager)
+                    .await
+                {
+                    let _ = x.delete(root_db, true);
+                    return Err(res);
+                }
                 Dispatcher::Xdp(x)
             }
             ProgramType::Tc => {
@@ -67,8 +72,13 @@ impl Dispatcher {
                     revision,
                 )?;
 
-                t.load(root_db, programs, old_dispatcher, image_manager)
-                    .await?;
+                if let Err(res) = t
+                    .load(root_db, programs, old_dispatcher, image_manager)
+                    .await
+                {
+                    let _ = t.delete(root_db, true);
+                    return Err(res);
+                }
                 Dispatcher::Tc(t)
             }
             _ => return Err(BpfmanError::DispatcherNotRequired),

diff --git a/bpfman/src/multiprog/tc.rs b/bpfman/src/multiprog/tc.rs
@@ -150,14 +150,17 @@ impl TcDispatcher {
 
         let mut loader = EbpfLoader::new()
             .set_global("CONFIG", &config, true)
-            .load(&program_bytes)?;
+            .load(&program_bytes)
+            .map_err(|e| BpfmanError::DispatcherLoadError(format!("{e}")))?;
 
-        let dispatcher: &mut SchedClassifier = loader
-            .program_mut(TC_DISPATCHER_PROGRAM_NAME)
-            .unwrap()
-            .try_into()?;
-
-        dispatcher.load()?;
+        if let Some(program) = loader.program_mut(TC_DISPATCHER_PROGRAM_NAME) {
+            let dispatcher: &mut SchedClassifier = program.try_into()?;
+            dispatcher.load()?;
+        } else {
+            return Err(BpfmanError::DispatcherLoadError(
+                "invalid BPF function name".to_string(),
+            ));
+        }
 
         let base = match direction {
             Ingress => RTDIR_FS_TC_INGRESS,

diff --git a/bpfman/src/multiprog/xdp.rs b/bpfman/src/multiprog/xdp.rs
@@ -146,14 +146,17 @@ impl XdpDispatcher {
 
         let mut loader = EbpfLoader::new()
             .set_global("conf", &config, true)
-            .load(&program_bytes)?;
+            .load(&program_bytes)
+            .map_err(|e| BpfmanError::DispatcherLoadError(format!("{e}")))?;
 
-        let dispatcher: &mut Xdp = loader
-            .program_mut(XDP_DISPATCHER_PROGRAM_NAME)
-            .unwrap()
-            .try_into()?;
-
-        dispatcher.load()?;
+        if let Some(program) = loader.program_mut(XDP_DISPATCHER_PROGRAM_NAME) {
+            let dispatcher: &mut Xdp = program.try_into()?;
+            dispatcher.load()?;
+        } else {
+            return Err(BpfmanError::DispatcherLoadError(
+                "invalid BPF function name".to_string(),
+            ));
+        }
 
         let path = format!("{RTDIR_FS_XDP}/dispatcher_{if_index}_{revision}");
         fs::create_dir_all(path).unwrap();

diff --git a/xtask/public-api/bpfman.txt b/xtask/public-api/bpfman.txt
@@ -16,6 +16,7 @@ pub bpfman::errors::BpfmanError::ContainerAttachError::container_pid: i32
 pub bpfman::errors::BpfmanError::ContainerAttachError::program_type: alloc::string::String
 pub bpfman::errors::BpfmanError::DatabaseError(alloc::string::String, alloc::string::String)
 pub bpfman::errors::BpfmanError::DatabaseLockError
+pub bpfman::errors::BpfmanError::DispatcherLoadError(alloc::string::String)
 pub bpfman::errors::BpfmanError::DispatcherNotRequired
 pub bpfman::errors::BpfmanError::Error(alloc::string::String)
 pub bpfman::errors::BpfmanError::InternalError(alloc::string::String)