[pbs-devel] [PATCH proxmox-backup 4/4] fix #5853: client: pxar: exclude stale files on metadata read

[Christian Ebner <c.ebner@proxmox.com>]

Skip and warn the user for files which returned a stale file handle
error while reading the metadata associated to that file.

Instead of returning with an error when getting the metadata, return
a boolean flag signaling if a stale file handle has been encountered.

Link to issue in bugtracker:
https://bugzilla.proxmox.com/show_bug.cgi?id=5853

Link to thread in community forum:
https://forum.proxmox.com/threads/156822/

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-client/src/pxar/create.rs | 100 ++++++++++++++++++++++------------
 1 file changed, 66 insertions(+), 34 deletions(-)

diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs
index 2a844922c..85be00db4 100644
--- a/pbs-client/src/pxar/create.rs
+++ b/pbs-client/src/pxar/create.rs
@@ -228,7 +228,7 @@ where
     let mut fs_feature_flags = Flags::from_magic(fs_magic);
 
     let stat = nix::sys::stat::fstat(source_dir.as_raw_fd())?;
-    let metadata = get_metadata(
+    let (metadata, stale_fd) = get_metadata(
         source_dir.as_raw_fd(),
         &stat,
         feature_flags & fs_feature_flags,
@@ -744,7 +744,7 @@ impl Archiver {
             return Ok(());
         }
 
-        let metadata = get_metadata(
+        let (metadata, stale_fd) = get_metadata(
             fd.as_raw_fd(),
             stat,
             self.flags(),
@@ -753,6 +753,11 @@ impl Archiver {
             self.skip_e2big_xattr,
         )?;
 
+        if stale_fd {
+            log::warn!("Stale filehandle encountered, skip {:?}", self.path);
+            return Ok(());
+        }
+
         if self.previous_payload_index.is_none() {
             return self
                 .add_entry_to_archive(encoder, &mut None, c_file_name, stat, fd, &metadata, None)
@@ -1301,7 +1306,14 @@ impl Archiver {
         file_name: &Path,
         metadata: &Metadata,
     ) -> Result<(), Error> {
-        let dest = nix::fcntl::readlinkat(fd.as_raw_fd(), &b""[..])?;
+        let dest = match nix::fcntl::readlinkat(fd.as_raw_fd(), &b""[..]) {
+            Ok(dest) => dest,
+            Err(Errno::ESTALE) => {
+                log::warn!("Stale file handle encountered, skip {file_name:?}");
+                return Ok(());
+            }
+            Err(err) => return Err(err.into()),
+        };
         encoder.add_symlink(metadata, file_name, dest).await?;
         Ok(())
     }
@@ -1397,9 +1409,10 @@ fn get_metadata(
     fs_magic: i64,
     fs_feature_flags: &mut Flags,
     skip_e2big_xattr: bool,
-) -> Result<Metadata, Error> {
+) -> Result<(Metadata, bool), Error> {
     // required for some of these
     let proc_path = Path::new("/proc/self/fd/").join(fd.to_string());
+    let mut stale_fd = false;
 
     let mut meta = Metadata {
         stat: pxar::Stat {
@@ -1412,18 +1425,27 @@ fn get_metadata(
         ..Default::default()
     };
 
-    get_xattr_fcaps_acl(
+    if get_xattr_fcaps_acl(
         &mut meta,
         fd,
         &proc_path,
         flags,
         fs_feature_flags,
         skip_e2big_xattr,
-    )?;
-    get_chattr(&mut meta, fd)?;
+    )? {
+        stale_fd = true;
+        log::warn!("Stale filehandle, xattrs incomplete");
+    }
+    if get_chattr(&mut meta, fd)? {
+        stale_fd = true;
+        log::warn!("Stale filehandle, chattr incomplete");
+    }
     get_fat_attr(&mut meta, fd, fs_magic)?;
-    get_quota_project_id(&mut meta, fd, flags, fs_magic)?;
-    Ok(meta)
+    if get_quota_project_id(&mut meta, fd, flags, fs_magic)? {
+        stale_fd = true;
+        log::warn!("Stale filehandle, quota  project id incomplete");
+    }
+    Ok((meta, stale_fd))
 }
 
 fn get_fcaps(
@@ -1431,22 +1453,23 @@ fn get_fcaps(
     fd: RawFd,
     flags: Flags,
     fs_feature_flags: &mut Flags,
-) -> Result<(), Error> {
+) -> Result<bool, Error> {
     if !flags.contains(Flags::WITH_FCAPS) {
-        return Ok(());
+        return Ok(false);
     }
 
     match xattr::fgetxattr(fd, xattr::XATTR_NAME_FCAPS) {
         Ok(data) => {
             meta.fcaps = Some(pxar::format::FCaps { data });
-            Ok(())
+            Ok(false)
         }
-        Err(Errno::ENODATA) => Ok(()),
+        Err(Errno::ENODATA) => Ok(false),
         Err(Errno::EOPNOTSUPP) => {
             fs_feature_flags.remove(Flags::WITH_FCAPS);
-            Ok(())
+            Ok(false)
         }
-        Err(Errno::EBADF) => Ok(()), // symlinks
+        Err(Errno::EBADF) => Ok(false), // symlinks
+        Err(Errno::ESTALE) => Ok(true),
         Err(err) => Err(err).context("failed to read file capabilities"),
     }
 }
@@ -1458,32 +1481,35 @@ fn get_xattr_fcaps_acl(
     flags: Flags,
     fs_feature_flags: &mut Flags,
     skip_e2big_xattr: bool,
-) -> Result<(), Error> {
+) -> Result<bool, Error> {
     if !flags.contains(Flags::WITH_XATTRS) {
-        return Ok(());
+        return Ok(false);
     }
 
     let xattrs = match xattr::flistxattr(fd) {
         Ok(names) => names,
         Err(Errno::EOPNOTSUPP) => {
             fs_feature_flags.remove(Flags::WITH_XATTRS);
-            return Ok(());
+            return Ok(false);
         }
         Err(Errno::E2BIG) => {
             match skip_e2big_xattr {
-                true => return Ok(()),
+                true => return Ok(false),
                 false => {
                     bail!("{} (try --skip-e2big-xattr)", Errno::E2BIG.to_string());
                 }
             };
         }
-        Err(Errno::EBADF) => return Ok(()), // symlinks
+        Err(Errno::EBADF) => return Ok(false), // symlinks
+        Err(Errno::ESTALE) => return Ok(true),
         Err(err) => return Err(err).context("failed to read xattrs"),
     };
 
     for attr in &xattrs {
         if xattr::is_security_capability(attr) {
-            get_fcaps(meta, fd, flags, fs_feature_flags)?;
+            if get_fcaps(meta, fd, flags, fs_feature_flags)? {
+                return Ok(true);
+            }
             continue;
         }
 
@@ -1505,35 +1531,37 @@ fn get_xattr_fcaps_acl(
             Err(Errno::EBADF) => (),   // symlinks, shouldn't be able to reach this either
             Err(Errno::E2BIG) => {
                 match skip_e2big_xattr {
-                    true => return Ok(()),
+                    true => return Ok(false),
                     false => {
                         bail!("{} (try --skip-e2big-xattr)", Errno::E2BIG.to_string());
                     }
                 };
             }
+            Err(Errno::ESTALE) => return Ok(true), // symlinks
             Err(err) => {
                 return Err(err).context(format!("error reading extended attribute {attr:?}"))
             }
         }
     }
 
-    Ok(())
+    Ok(false)
 }
 
-fn get_chattr(metadata: &mut Metadata, fd: RawFd) -> Result<(), Error> {
+fn get_chattr(metadata: &mut Metadata, fd: RawFd) -> Result<bool, Error> {
     let mut attr: libc::c_long = 0;
 
     match unsafe { fs::read_attr_fd(fd, &mut attr) } {
         Ok(_) => (),
+        Err(Errno::ESTALE) => return Ok(true),
         Err(errno) if errno_is_unsupported(errno) => {
-            return Ok(());
+            return Ok(false);
         }
         Err(err) => return Err(err).context("failed to read file attributes"),
     }
 
     metadata.stat.flags |= Flags::from_chattr(attr).bits();
 
-    Ok(())
+    Ok(false)
 }
 
 fn get_fat_attr(metadata: &mut Metadata, fd: RawFd, fs_magic: i64) -> Result<(), Error> {
@@ -1564,30 +1592,34 @@ fn get_quota_project_id(
     fd: RawFd,
     flags: Flags,
     magic: i64,
-) -> Result<(), Error> {
+) -> Result<bool, Error> {
     if !(metadata.is_dir() || metadata.is_regular_file()) {
-        return Ok(());
+        return Ok(false);
     }
 
     if !flags.contains(Flags::WITH_QUOTA_PROJID) {
-        return Ok(());
+        return Ok(false);
     }
 
     use proxmox_sys::linux::magic::*;
 
     match magic {
         EXT4_SUPER_MAGIC | XFS_SUPER_MAGIC | FUSE_SUPER_MAGIC | ZFS_SUPER_MAGIC => (),
-        _ => return Ok(()),
+        _ => return Ok(false),
     }
 
     let mut fsxattr = fs::FSXAttr::default();
     let res = unsafe { fs::fs_ioc_fsgetxattr(fd, &mut fsxattr) };
 
+    if let Err(Errno::ESTALE) = res {
+        return Ok(true);
+    }
+
     // On some FUSE filesystems it can happen that ioctl is not supported.
     // For these cases projid is set to 0 while the error is ignored.
     if let Err(errno) = res {
         if errno_is_unsupported(errno) {
-            return Ok(());
+            return Ok(false);
         } else {
             return Err(errno).context("error while reading quota project id");
         }
@@ -1597,7 +1629,7 @@ fn get_quota_project_id(
     if projid != 0 {
         metadata.quota_project_id = Some(pxar::format::QuotaProjectId { projid });
     }
-    Ok(())
+    Ok(false)
 }
 
 fn get_acl(
@@ -1840,7 +1872,7 @@ mod tests {
         let fs_magic = detect_fs_type(dir.as_raw_fd()).unwrap();
         let stat = nix::sys::stat::fstat(dir.as_raw_fd()).unwrap();
         let mut fs_feature_flags = Flags::from_magic(fs_magic);
-        let metadata = get_metadata(
+        let (metadata, _) = get_metadata(
             dir.as_raw_fd(),
             &stat,
             fs_feature_flags,
@@ -1937,7 +1969,7 @@ mod tests {
             let stat = nix::sys::stat::fstat(source_dir.as_raw_fd()).unwrap();
             let mut fs_feature_flags = Flags::from_magic(fs_magic);
 
-            let metadata = get_metadata(
+            let (metadata, _) = get_metadata(
                 source_dir.as_raw_fd(),
                 &stat,
                 fs_feature_flags,
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel