Repository: andrewchambers/bupstash (language: rust)

Issue:
bupstash put: Operation not permitted when backed-up file is owned by other user but world-readable
My first backup over ssh to a NAS next to me ran for a bit and then failed with `bupstash put: Operation not permitted (os error 1)`.

I can reproduce it reliably, the annoying part was that it happened in a directory of large VM images, and for some reason I don't understand yet, the next run seems to start again reading a 20 GB file where there's _no_ error, instead of starting directly with the failing file.

But I let `strace` run for a while now, and can see which syscall it is:

```c
openat(AT_FDCWD, "/home/niklas/vms/MSEdge - Win10.qcow2", O_RDONLY|O_NOATIME|O_CLOEXEC) = -1 EPERM (Operation not permitted)
```

Here it's the `O_NOATIME` that give the permission error, without that it works (I checked it with Python's `os.open("/path/...",os.O_RDONLY|os.O_NOATIME)`).

The file is owned by another user (qemu) and permissions are `-rw-r--r--`.

I believe it fails because of the different permissions: `man 2 open` says about `O_NOATIME` that

```
              This flag can be employed only if one of the following conditions is true:

              *  The effective UID of the process matches the owner UID of the file.

              *  The  calling  process  has the CAP_FOWNER capability in its user namespace
                 and the owner UID of the file has a mapping in the namespace.
```

so the first point explains the `EPERM`.

Glib retries for such cases, see https://gitlab.gnome.org/GNOME/glib/-/issues/874.

Given that bupstash does backups and `NOATIME` is good for performance, it might be best to implement such fallback, and to write into the docs that doing backups as root is recommended if you're backing up many files made by other users so that `NOATIME` can be used.

If you already necessarily `stat()` the files before `openat()` (I haven't checked yet if bupstash does that), then retrying `openat()` would not be necessary, it could directly be invoked without `O_NOATIME`, going straight into the fallback.


Discussion / hints:
I've filed #107 for better error messages in such cases.

Fix patch:
diff --git a/src/client.rs b/src/client.rs
index 0a2f146b..af84f736 100644
--- a/src/client.rs
+++ b/src/client.rs
@@ -16,6 +16,7 @@ use super::xid::*;
 use super::xtar;
 use std::collections::BTreeMap;
 use std::convert::TryInto;
+use std::io::ErrorKind;
 use std::os::unix::ffi::OsStrExt;
 use std::os::unix::fs::FileTypeExt;
 use std::os::unix::fs::MetadataExt;
@@ -541,10 +542,22 @@ cfg_if::cfg_if! {
     if #[cfg(target_os = "linux")] {
 
         fn open_file_for_sending(fpath: &std::path::Path) -> Result<std::fs::File, std::io::Error> {
+            // Try with O_NOATIME first; if it fails, e.g. because the user we
+            // run as is not the file owner, retry without. See #106.
             let f = std::fs::OpenOptions::new()
                 .read(true)
                 .custom_flags(libc::O_NOATIME)
-                .open(fpath)?;
+                .open(fpath)
+                .or_else(|error| {
+                    match error.kind() {
+                        ErrorKind::PermissionDenied => {
+                            std::fs::OpenOptions::new()
+                                .read(true)
+                                .open(fpath)
+                        }
+                        _ => Err(error)
+                    }
+                })?;
 
             // For linux at least, shift file pages to the tail of the page cache, allowing
             // the kernel to quickly evict these pages. This works well for the case of system


Test patch: