Browse Source

Fixed a bug where the FUSE daemon was not returning the correct
amount of data to the kernel when handling read requests.

Matthew Carr 1 year ago
parent
commit
a6f535669a
3 changed files with 46 additions and 33 deletions
  1. 5 5
      crates/btfuse/src/fuse_fs.rs
  2. 40 27
      crates/btfuse/src/main.rs
  3. 1 1
      crates/btlib/src/crypto.rs

+ 5 - 5
crates/btfuse/src/fuse_fs.rs

@@ -322,9 +322,9 @@ mod private {
         ) -> IoResult<usize> {
             block_on(async move {
                 let path = self.path_from_luid(ctx.uid);
-                let total_size: usize = size.try_into().display_err()?;
-                let mut total_written = 0;
-                while total_written < total_size {
+                let mut size: usize = size.try_into().display_err()?;
+                let start_size = size;
+                while size > 0 {
                     let msg = Read {
                         inode,
                         handle,
@@ -337,11 +337,11 @@ mod private {
                         break;
                     }
                     w.write_all(&guard)?;
-                    total_written += slice.len();
+                    size -= slice.len();
                     let len: u64 = slice.len().try_into().display_err()?;
                     offset += len;
                 }
-                Ok(total_written)
+                Ok(start_size - size)
             })
         }
 

+ 40 - 27
crates/btfuse/src/main.rs

@@ -81,13 +81,9 @@ async fn remote_provider<C: 'static + Creds + Send + Sync>(
     Ok(client)
 }
 
-async fn run_daemon(
-    config: Config,
-    mounted_signal: Option<oneshot::Sender<()>>,
-    stop_signal: Option<oneshot::Receiver<()>>,
-) {
+async fn run_daemon(config: Config, mounted_signal: Option<oneshot::Sender<()>>) {
     let node_creds = Arc::new(
-        node_creds(config.tpm_state_file, &config.tabrmd).expect("failed to get node creds")
+        node_creds(config.tpm_state_file, &config.tabrmd).expect("failed to get node creds"),
     );
     let fallback_path = {
         let writecap = node_creds
@@ -129,14 +125,7 @@ async fn run_daemon(
     }
     .expect("failed to create FUSE daemon");
 
-    if let Some(stop_signal) = stop_signal {
-        tokio::select! {
-            _  = daemon.finished() => (),
-            _ = stop_signal => (),
-        };
-    } else {
-        daemon.finished().await;
-    }
+    daemon.finished().await;
 }
 
 #[tokio::main]
@@ -151,7 +140,7 @@ async fn main() {
         .with_tabrmd(from_envvar(ENVVARS.tabrmd).unwrap())
         .with_mnt_options(from_envvar(ENVVARS.mnt_options).unwrap());
     let config = builder.build();
-    run_daemon(config, None, None).await;
+    run_daemon(config, None).await;
 }
 
 #[cfg(test)]
@@ -165,7 +154,7 @@ mod test {
     use std::{
         ffi::{OsStr, OsString},
         fs::Permissions,
-        io::SeekFrom,
+        io::{BufRead, SeekFrom},
         net::{IpAddr, Ipv6Addr},
         num::NonZeroUsize,
         os::unix::fs::PermissionsExt,
@@ -195,11 +184,39 @@ mod test {
         env_logger::Builder::from_default_env().btformat().init();
     }
 
+    /// Reads `/etc/mtab` to determine if `mnt_path` is mounted. Returns true if it is and false
+    /// otherwise.
+    fn mounted(mnt_path: &str) -> bool {
+        let file = std::fs::OpenOptions::new()
+            .read(true)
+            .write(false)
+            .create(false)
+            .open("/etc/mtab")
+            .unwrap();
+        let mut reader = std::io::BufReader::new(file);
+        let mut line = String::with_capacity(64);
+        loop {
+            line.clear();
+            let read = reader.read_line(&mut line).unwrap();
+            if 0 == read {
+                break;
+            }
+            let path = line.split(' ').skip(1).next().unwrap();
+            if path == mnt_path {
+                return true;
+            }
+        }
+        false
+    }
+
     /// Unmounts the file system at the given path.
     fn unmount<P: AsRef<Path>>(mnt_path: P) {
+        let mnt_path = mnt_path.as_ref();
+        if !mounted(mnt_path.to_str().unwrap()) {
+            return;
+        }
         const PROG: &str = "fusermount";
         let mnt_path = mnt_path
-            .as_ref()
             .as_os_str()
             .to_str()
             .expect("failed to convert mnt_path to `str`");
@@ -228,7 +245,7 @@ mod test {
         config: Config,
         handle: Option<JoinHandle<()>>,
         node_principal: OsString,
-        stop_tx: Option<oneshot::Sender<()>>,
+        stop_flag: Option<()>,
         // Note that the drop order of these fields is significant.
         _receiver: Option<R>,
         _cred_store: TpmCredStore,
@@ -247,7 +264,6 @@ mod test {
     async fn new(remote: bool) -> TestCase<impl Receiver> {
         let tmp = TempDir::new("btfuse").unwrap();
         let (mounted_tx, mounted_rx) = oneshot::channel();
-        let (stop_tx, stop_rx) = oneshot::channel();
         let (swtpm, cred_store) = swtpm();
         let builder = Config::builder()
             .with_threads(Some(NonZeroUsize::new(1).unwrap()))
@@ -273,9 +289,7 @@ mod test {
         };
         let config_clone = config.clone();
         let handle = tokio::spawn(async move {
-            let mnt_dir = config_clone.mnt_dir.clone();
-            run_daemon(config_clone, Some(mounted_tx), Some(stop_rx)).await;
-            unmount(mnt_dir);
+            run_daemon(config_clone, Some(mounted_tx)).await;
         });
         if let Some(timeout) = TIMEOUT {
             tokio::time::timeout(timeout, mounted_rx)
@@ -291,7 +305,7 @@ mod test {
             config,
             handle: Some(handle),
             node_principal,
-            stop_tx: Some(stop_tx),
+            stop_flag: Some(()),
             _receiver: receiver,
             _temp_dir: tmp,
             _swtpm: swtpm,
@@ -325,10 +339,8 @@ mod test {
 
         /// Signals to the daemon that it must stop.
         fn signal_stop(&mut self) {
-            if let Some(stop_tx) = self.stop_tx.take() {
-                if let Err(_) = stop_tx.send(()) {
-                    log::error!("failed to send the TestCase stop signal");
-                }
+            if let Some(_) = self.stop_flag.take() {
+                unmount(&self.config.mnt_dir)
             }
         }
 
@@ -728,6 +740,7 @@ mod test {
         fill(&mut buf, 1);
         assert_eq!(buf, actual);
 
+        drop(file);
         case.stop().await;
     }
 

+ 1 - 1
crates/btlib/src/crypto.rs

@@ -1993,7 +1993,7 @@ pub struct SignWrite<T, Op> {
 
 impl<T, Op: SignOp> SignWrite<T, Op> {
     pub fn new(inner: T, op: Op) -> Self {
-        SignWrite { inner, op, }
+        SignWrite { inner, op }
     }
 
     pub fn finish_into(mut self, buf: &mut [u8]) -> Result<(usize, T)> {