Being able to read a file despite being the owner but having read
permissions is pefectly fine, however due to the fact that O_NOATIME is
passed in ovl_open_realfile (which also affects lowerdir), the open()
fails.
Now in normal situations where the overlayfs is mounted as root, this
shouldn't be a problem, but as soon as you have a networked file system,
things go bad.
That's what happens in NixOS VM tests, where there is a 9p file system
mounted in a guest VM and a lowerdir of overlayfs on top of that. If the
file owner on the host is the same as the current uid of qemu process,
the open() works correctly. However if it's not the case, it will fail
with EPERM on the host side (even though you have read access).
I originally thought about adding a condition on whether to add the
flag, but I only see two options here, which IMHO are bad in their own
rights:
* Using inode_owner_or_capable() to check whether to add O_NOATIME,
which has the downside that it will not work with networked file
systems where you map different users (I've tested this already with
a different patch[1]).
* Check for failure of open_with_fake_path() and retry without
O_NOATIME, which *could* be an option, but I think that might come
with a performance penalty.
Actually, a third option would be to just ignore O_NOATIME in fs/namei.c
instead of returning -EPERM, but I think that could open up a whole
range of other bugs.
So I'm simply removing the O_NOATIME flag when it comes to lowerdir,
because for now it is IMHO the most sensible option, as it doesn't cause
problems with network file systems and also leaves the atime/noatime
decision to the administrator of the corresponding system. This also
reverts the behaviour to what we had prior to Linux 4.19.
Originally, I had only removed the flag from ovl_open_realfile, but as
Amir Goldstein pointed out, this is also the case when copying files to
upperdir. On NixOS I didn't notice this, because Nix store paths are
never changed and only new ones are added if the build recipe differs.
Signed-off-by: aszlig <aszlig@nix.build>
---
fs/overlayfs/copy_up.c | 7 ++++++-
fs/overlayfs/file.c | 2 +-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 9e62dcf06fc4..e43d2a2e9d49 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -131,7 +131,12 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len)
if (len == 0)
return 0;
- old_file = ovl_path_open(old, O_LARGEFILE | O_RDONLY);
+ /*
+ * Note that this is not using ovl_path_open() because it will use
+ * O_NOATIME, which in turn could cause issues for networked file
+ * systems.
+ */
+ old_file = dentry_open(old, O_LARGEFILE | O_RDONLY, current_cred());
if (IS_ERR(old_file))
return PTR_ERR(old_file);
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 84dd957efa24..3f9b9275267b 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -31,7 +31,7 @@ static struct file *ovl_open_realfile(const struct file *file,
const struct cred *old_cred;
old_cred = ovl_override_creds(inode->i_sb);
- realfile = open_with_fake_path(&file->f_path, file->f_flags | O_NOATIME,
+ realfile = open_with_fake_path(&file->f_path, file->f_flags,
realinode, current_cred());
revert_creds(old_cred);
--
2.19.2
Attachment:
signature.asc
Description: PGP signature
