From c3b090ac8ac0cc71d9e7e80b0c8ae947936ceb0b Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Wed, 22 Jul 2020 16:01:50 +0200 Subject: [PATCH] backup: list images: handle walkdir error, catch "lost+found" We support using an ext4 mountpoint directly as datastore and even do so ourself when creating one through the disk manage code. Such ext4 ountpoints have a lost+found directory which only root can traverse into. As the GC list images is done as backup:backup user walkdir gets an error. We cannot ignore just all permission errors, as they could lead to missing some backup indexes and thus possibly sweeping more chunks than desired. While *normally* that should not happen through our stack, we had already user report that they do rsyncs to move a datastore from old to new server and got the permission wrong. So for now be still very strict, only allow a "lost+found" directory as immediate child of the datastore base directory, nothing else. If deemed safe, this can always be made less strict. Possibly by filtering the known backup-types on the highest level first. Signed-off-by: Thomas Lamprecht --- src/backup/datastore.rs | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs index abeb101c..e66ae843 100644 --- a/src/backup/datastore.rs +++ b/src/backup/datastore.rs @@ -340,9 +340,30 @@ impl DataStore { .map(|s| s.starts_with(".")) .unwrap_or(false) } - + let handle_entry_err = |err: walkdir::Error| { + if let Some(inner) = err.io_error() { + let path = err.path().unwrap_or(Path::new("")); + match inner.kind() { + io::ErrorKind::PermissionDenied => { + // only allow to skip ext4 fsck directory, avoid GC if, for example, + // a user got file permissions wrong on datastore rsync to new server + if err.depth() > 1 || !path.ends_with("lost+found") { + bail!("cannot continue garbage-collection safely, permission denied on: {}", path.display()) + } + }, + _ => bail!("unexpected error on datastore traversal: {} - {}", inner, path.display()), + } + } + Ok(()) + }; for entry in walker.filter_entry(|e| !is_hidden(e)) { - let path = entry?.into_path(); + let path = match entry { + Ok(entry) => entry.into_path(), + Err(err) => { + handle_entry_err(err)?; + continue + }, + }; if let Ok(archive_type) = archive_type(&path) { if archive_type == ArchiveType::FixedIndex || archive_type == ArchiveType::DynamicIndex { list.push(path);