From 3115d6a40e37384fcda3a9e338fe883c9475b888 Mon Sep 17 00:00:00 2001
From: Billy McFall <22157057+Billy99@users.noreply.github.com>
Date: Mon, 12 Aug 2024 17:39:21 -0400
Subject: [PATCH] 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 <22157057+Billy99@users.noreply.github.com>
---
 bpfman/src/errors.rs        |  2 ++
 bpfman/src/multiprog/mod.rs | 18 ++++++++++++++----
 bpfman/src/multiprog/tc.rs  | 17 ++++++++++-------
 bpfman/src/multiprog/xdp.rs | 17 ++++++++++-------
 xtask/public-api/bpfman.txt |  1 +
 5 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/bpfman/src/errors.rs b/bpfman/src/errors.rs
index afbf08af5..870eb7062 100644
--- 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
index 68a32fd3a..20cb70162 100644
--- 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
index 8e662eb10..70a3ff2d4 100644
--- 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
index f1a09f070..0d600bfe7 100644
--- 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
index 63a571c80..0386c5913 100644
--- 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)