ovl: implement volatile-specific fsync error behaviour
authorSargun Dhillon <sargun@sargun.me>
Fri, 8 Jan 2021 00:10:43 +0000 (16:10 -0800)
committerMiklos Szeredi <mszeredi@redhat.com>
Thu, 28 Jan 2021 09:22:48 +0000 (10:22 +0100)
Overlayfs's volatile option allows the user to bypass all forced sync calls
to the upperdir filesystem. This comes at the cost of safety. We can never
ensure that the user's data is intact, but we can make a best effort to
expose whether or not the data is likely to be in a bad state.

The best way to handle this in the time being is that if an overlayfs's
upperdir experiences an error after a volatile mount occurs, that error
will be returned on fsync, fdatasync, sync, and syncfs. This is
contradictory to the traditional behaviour of VFS which fails the call
once, and only raises an error if a subsequent fsync error has occurred,
and been raised by the filesystem.

One awkward aspect of the patch is that we have to manually set the
superblock's errseq_t after the sync_fs callback as opposed to just
returning an error from syncfs. This is because the call chain looks
something like this:

sys_syncfs ->
sync_filesystem ->
__sync_filesystem ->
/* The return value is ignored here
sb->s_op->sync_fs(sb)
_sync_blockdev
/* Where the VFS fetches the error to raise to userspace */
errseq_check_and_advance

Because of this we call errseq_set every time the sync_fs callback occurs.
Due to the nature of this seen / unseen dichotomy, if the upperdir is an
inconsistent state at the initial mount time, overlayfs will refuse to
mount, as overlayfs cannot get a snapshot of the upperdir's errseq that
will increment on error until the user calls syncfs.

Signed-off-by: Sargun Dhillon <sargun@sargun.me>
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Fixes: c86243b090bc ("ovl: provide a mount option "volatile"")
Cc: stable@vger.kernel.org
Reviewed-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Documentation/filesystems/overlayfs.rst
fs/overlayfs/file.c
fs/overlayfs/overlayfs.h
fs/overlayfs/ovl_entry.h
fs/overlayfs/readdir.c
fs/overlayfs/super.c
fs/overlayfs/util.c

index 587a939739290628f7c236443493f570c6df7040..78240e29b0bb676963ffe1dfb023d5e501fd1e51 100644 (file)
@@ -586,6 +586,14 @@ without significant effort.
 The advantage of mounting with the "volatile" option is that all forms of
 sync calls to the upper filesystem are omitted.
 
+In order to avoid a giving a false sense of safety, the syncfs (and fsync)
+semantics of volatile mounts are slightly different than that of the rest of
+VFS.  If any writeback error occurs on the upperdir's filesystem after a
+volatile mount takes place, all sync functions will return an error.  Once this
+condition is reached, the filesystem will not recover, and every subsequent sync
+call will return an error, even if the upperdir has not experience a new error
+since the last sync call.
+
 When overlay is mounted with "volatile" option, the directory
 "$workdir/work/incompat/volatile" is created.  During next mount, overlay
 checks for this directory and refuses to mount if present. This is a strong
index bd9dd38347aed87ed263d5df56ffcd3e658ea562..077d3ad343f689f4f517b804e2bfc848082a4d2b 100644 (file)
@@ -398,8 +398,9 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
        const struct cred *old_cred;
        int ret;
 
-       if (!ovl_should_sync(OVL_FS(file_inode(file)->i_sb)))
-               return 0;
+       ret = ovl_sync_status(OVL_FS(file_inode(file)->i_sb));
+       if (ret <= 0)
+               return ret;
 
        ret = ovl_real_fdget_meta(file, &real, !datasync);
        if (ret)
index b487e48c7fd4276d22caf17a56e75f5f298a8cdf..cb4e2d60ecf9ceb2a4aeceecca1748d3002008eb 100644 (file)
@@ -324,6 +324,7 @@ int ovl_check_metacopy_xattr(struct ovl_fs *ofs, struct dentry *dentry);
 bool ovl_is_metacopy_dentry(struct dentry *dentry);
 char *ovl_get_redirect_xattr(struct ovl_fs *ofs, struct dentry *dentry,
                             int padding);
+int ovl_sync_status(struct ovl_fs *ofs);
 
 static inline bool ovl_is_impuredir(struct super_block *sb,
                                    struct dentry *dentry)
index fbd5e27ce66bd3aad0c44e406ed943c016c1ea32..63efee554f69a7c1e862650d8f7b388576fda2cd 100644 (file)
@@ -81,6 +81,8 @@ struct ovl_fs {
        atomic_long_t last_ino;
        /* Whiteout dentry cache */
        struct dentry *whiteout;
+       /* r/o snapshot of upperdir sb's only taken on volatile mounts */
+       errseq_t errseq;
 };
 
 static inline struct vfsmount *ovl_upper_mnt(struct ovl_fs *ofs)
index 60d751f28fea5cacefecbb5d93575db2f003a735..f404a78e6b607bf1d2b854506ebb3f72cd8af8d2 100644 (file)
@@ -900,8 +900,9 @@ static int ovl_dir_fsync(struct file *file, loff_t start, loff_t end,
        struct file *realfile;
        int err;
 
-       if (!ovl_should_sync(OVL_FS(file->f_path.dentry->d_sb)))
-               return 0;
+       err = ovl_sync_status(OVL_FS(file->f_path.dentry->d_sb));
+       if (err <= 0)
+               return err;
 
        realfile = ovl_dir_real_file(file, true);
        err = PTR_ERR_OR_ZERO(realfile);
index 82cd6d55a5a1300941ee231a752b59bf4c37df18..d58b8f2bf9d0a45b4745e6feb776f7651c6362be 100644 (file)
@@ -264,11 +264,20 @@ static int ovl_sync_fs(struct super_block *sb, int wait)
        struct super_block *upper_sb;
        int ret;
 
-       if (!ovl_upper_mnt(ofs))
-               return 0;
+       ret = ovl_sync_status(ofs);
+       /*
+        * We have to always set the err, because the return value isn't
+        * checked in syncfs, and instead indirectly return an error via
+        * the sb's writeback errseq, which VFS inspects after this call.
+        */
+       if (ret < 0) {
+               errseq_set(&sb->s_wb_err, -EIO);
+               return -EIO;
+       }
+
+       if (!ret)
+               return ret;
 
-       if (!ovl_should_sync(ofs))
-               return 0;
        /*
         * Not called for sync(2) call or an emergency sync (SB_I_SKIP_SYNC).
         * All the super blocks will be iterated, including upper_sb.
@@ -1993,6 +2002,8 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
        sb->s_op = &ovl_super_operations;
 
        if (ofs->config.upperdir) {
+               struct super_block *upper_sb;
+
                if (!ofs->config.workdir) {
                        pr_err("missing 'workdir'\n");
                        goto out_err;
@@ -2002,6 +2013,16 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
                if (err)
                        goto out_err;
 
+               upper_sb = ovl_upper_mnt(ofs)->mnt_sb;
+               if (!ovl_should_sync(ofs)) {
+                       ofs->errseq = errseq_sample(&upper_sb->s_wb_err);
+                       if (errseq_check(&upper_sb->s_wb_err, ofs->errseq)) {
+                               err = -EIO;
+                               pr_err("Cannot mount volatile when upperdir has an unseen error. Sync upperdir fs to clear state.\n");
+                               goto out_err;
+                       }
+               }
+
                err = ovl_get_workdir(sb, ofs, &upperpath);
                if (err)
                        goto out_err;
@@ -2009,9 +2030,8 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
                if (!ofs->workdir)
                        sb->s_flags |= SB_RDONLY;
 
-               sb->s_stack_depth = ovl_upper_mnt(ofs)->mnt_sb->s_stack_depth;
-               sb->s_time_gran = ovl_upper_mnt(ofs)->mnt_sb->s_time_gran;
-
+               sb->s_stack_depth = upper_sb->s_stack_depth;
+               sb->s_time_gran = upper_sb->s_time_gran;
        }
        oe = ovl_get_lowerstack(sb, splitlower, numlower, ofs, layers);
        err = PTR_ERR(oe);
index 6569031af3cdd4ca0fed5cf035161a7f1a33248c..9826b003f1d2702132c4cd12920230723b02416d 100644 (file)
@@ -962,3 +962,30 @@ err_free:
        kfree(buf);
        return ERR_PTR(res);
 }
+
+/*
+ * ovl_sync_status() - Check fs sync status for volatile mounts
+ *
+ * Returns 1 if this is not a volatile mount and a real sync is required.
+ *
+ * Returns 0 if syncing can be skipped because mount is volatile, and no errors
+ * have occurred on the upperdir since the mount.
+ *
+ * Returns -errno if it is a volatile mount, and the error that occurred since
+ * the last mount. If the error code changes, it'll return the latest error
+ * code.
+ */
+
+int ovl_sync_status(struct ovl_fs *ofs)
+{
+       struct vfsmount *mnt;
+
+       if (ovl_should_sync(ofs))
+               return 1;
+
+       mnt = ovl_upper_mnt(ofs);
+       if (!mnt)
+               return 0;
+
+       return errseq_check(&mnt->mnt_sb->s_wb_err, ofs->errseq);
+}