Merge branch 'overlayfs-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs

Pull overlayfs fixes from Miklos Szeredi:

 - fix incomplete syncing of filesystem

 - fix regression in readdir on ovl over 9p

 - only follow redirects when needed

 - misc fixes and cleanups

* 'overlayfs-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs:
  ovl: fix overlay: warning prefix
  ovl: Use PTR_ERR_OR_ZERO()
  ovl: Sync upper dirty data when syncing overlayfs
  ovl: update ctx->pos on impure dir iteration
  ovl: Pass ovl_get_nlink() parameters in right order
  ovl: don't follow redirects if redirect_dir=off

8 files changed, 137 insertions, 26 deletions

diff --git a/Documentation/filesystems/overlayfs.txt b/Documentation/filesystems/overlayfs.txt
index 8caa60734647..e6a5f4912b6d 100644
--- a/Documentation/filesystems/overlayfs.txt
+++ b/Documentation/filesystems/overlayfs.txt
@@ -156,6 +156,40 @@ handle it in two different ways:
    root of the overlay.  Finally the directory is moved to the new
    location.
 
+There are several ways to tune the "redirect_dir" feature.
+
+Kernel config options:
+
+- OVERLAY_FS_REDIRECT_DIR:
+    If this is enabled, then redirect_dir is turned on by  default.
+- OVERLAY_FS_REDIRECT_ALWAYS_FOLLOW:
+    If this is enabled, then redirects are always followed by default. Enabling
+    this results in a less secure configuration.  Enable this option only when
+    worried about backward compatibility with kernels that have the redirect_dir
+    feature and follow redirects even if turned off.
+
+Module options (can also be changed through /sys/module/overlay/parameters/*):
+
+- "redirect_dir=BOOL":
+    See OVERLAY_FS_REDIRECT_DIR kernel config option above.
+- "redirect_always_follow=BOOL":
+    See OVERLAY_FS_REDIRECT_ALWAYS_FOLLOW kernel config option above.
+- "redirect_max=NUM":
+    The maximum number of bytes in an absolute redirect (default is 256).
+
+Mount options:
+
+- "redirect_dir=on":
+    Redirects are enabled.
+- "redirect_dir=follow":
+    Redirects are not created, but followed.
+- "redirect_dir=off":
+    Redirects are not created and only followed if "redirect_always_follow"
+    feature is enabled in the kernel/module config.
+- "redirect_dir=nofollow":
+    Redirects are not created and not followed (equivalent to "redirect_dir=off"
+    if "redirect_always_follow" feature is not enabled).
+
 Non-directories
 ---------------
 
diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
index cbfc196e5dc5..5ac415466861 100644
--- a/fs/overlayfs/Kconfig
+++ b/fs/overlayfs/Kconfig
@@ -24,6 +24,16 @@ config OVERLAY_FS_REDIRECT_DIR
           an overlay which has redirects on a kernel that doesn't support this
           feature will have unexpected results.
 
+config OVERLAY_FS_REDIRECT_ALWAYS_FOLLOW
+        bool "Overlayfs: follow redirects even if redirects are turned off"
+        default y
+        depends on OVERLAY_FS
+        help
+          Disable this to get a possibly more secure configuration, but that
+          might not be backward compatible with previous kernels.
+
+          For more information, see Documentation/filesystems/overlayfs.txt
+
 config OVERLAY_FS_INDEX
         bool "Overlayfs: turn on inodes index feature by default"
         depends on OVERLAY_FS
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index e13921824c70..f9788bc116a8 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -887,7 +887,8 @@ static int ovl_set_redirect(struct dentry *dentry, bool samedir)
                 spin_unlock(&dentry->d_lock);
         } else {
                 kfree(redirect);
-                pr_warn_ratelimited("overlay: failed to set redirect (%i)\n", err);
+                pr_warn_ratelimited("overlayfs: failed to set redirect (%i)\n",
+                                    err);
                 /* Fall back to userspace copy-up */
                 err = -EXDEV;
         }
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 625ed8066570..beb945e1963c 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -435,7 +435,7 @@ int ovl_verify_index(struct dentry *index, struct ovl_path *lower,
 
         /* Check if index is orphan and don't warn before cleaning it */
         if (d_inode(index)->i_nlink == 1 &&
-            ovl_get_nlink(index, origin.dentry, 0) == 0)
+            ovl_get_nlink(origin.dentry, index, 0) == 0)
                 err = -ENOENT;
 
         dput(origin.dentry);
@@ -681,6 +681,22 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
                 if (d.stop)
                         break;
 
+                /*
+                 * Following redirects can have security consequences: it's like
+                 * a symlink into the lower layer without the permission checks.
+                 * This is only a problem if the upper layer is untrusted (e.g
+                 * comes from an USB drive).  This can allow a non-readable file
+                 * or directory to become readable.
+                 *
+                 * Only following redirects when redirects are enabled disables
+                 * this attack vector when not necessary.
+                 */
+                err = -EPERM;
+                if (d.redirect && !ofs->config.redirect_follow) {
+                        pr_warn_ratelimited("overlay: refusing to follow redirect for (%pd2)\n", dentry);
+                        goto out_put;
+                }
+
                 if (d.redirect && d.redirect[0] == '/' && poe != roe) {
                         poe = roe;
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 13eab09a6b6f..b489099ccd49 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -180,7 +180,7 @@ static inline int ovl_do_whiteout(struct inode *dir, struct dentry *dentry)
 static inline struct dentry *ovl_do_tmpfile(struct dentry *dentry, umode_t mode)
 {
         struct dentry *ret = vfs_tmpfile(dentry, mode, 0);
-        int err = IS_ERR(ret) ? PTR_ERR(ret) : 0;
+        int err = PTR_ERR_OR_ZERO(ret);
 
         pr_debug("tmpfile(%pd2, 0%o) = %i\n", dentry, mode, err);
         return ret;
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 752bab645879..9d0bc03bf6e4 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -14,6 +14,8 @@ struct ovl_config {
         char *workdir;
         bool default_permissions;
         bool redirect_dir;
+        bool redirect_follow;
+        const char *redirect_mode;
         bool index;
 };
 
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 0daa4354fec4..8c98578d27a1 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -499,7 +499,7 @@ out:
         return err;
 
 fail:
-        pr_warn_ratelimited("overlay: failed to look up (%s) for ino (%i)\n",
+        pr_warn_ratelimited("overlayfs: failed to look up (%s) for ino (%i)\n",
                             p->name, err);
         goto out;
 }
@@ -663,7 +663,10 @@ static int ovl_iterate_real(struct file *file, struct dir_context *ctx)
                         return PTR_ERR(rdt.cache);
         }
 
-        return iterate_dir(od->realfile, &rdt.ctx);
+        err = iterate_dir(od->realfile, &rdt.ctx);
+        ctx->pos = rdt.ctx.pos;
+
+        return err;
 }
 
 
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 288d20f9a55a..76440feb79f6 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -33,6 +33,13 @@ module_param_named(redirect_dir, ovl_redirect_dir_def, bool, 0644);
 MODULE_PARM_DESC(ovl_redirect_dir_def,
                  "Default to on or off for the redirect_dir feature");
 
+static bool ovl_redirect_always_follow =
+        IS_ENABLED(CONFIG_OVERLAY_FS_REDIRECT_ALWAYS_FOLLOW);
+module_param_named(redirect_always_follow, ovl_redirect_always_follow,
+                   bool, 0644);
+MODULE_PARM_DESC(ovl_redirect_always_follow,
+                 "Follow redirects even if redirect_dir feature is turned off");
+
 static bool ovl_index_def = IS_ENABLED(CONFIG_OVERLAY_FS_INDEX);
 module_param_named(index, ovl_index_def, bool, 0644);
 MODULE_PARM_DESC(ovl_index_def,
@@ -232,6 +239,7 @@ static void ovl_free_fs(struct ovl_fs *ofs)
         kfree(ofs->config.lowerdir);
         kfree(ofs->config.upperdir);
         kfree(ofs->config.workdir);
+        kfree(ofs->config.redirect_mode);
         if (ofs->creator_cred)
                 put_cred(ofs->creator_cred);
         kfree(ofs);
@@ -244,6 +252,7 @@ static void ovl_put_super(struct super_block *sb)
         ovl_free_fs(ofs);
 }
 
+/* Sync real dirty inodes in upper filesystem (if it exists) */
 static int ovl_sync_fs(struct super_block *sb, int wait)
 {
         struct ovl_fs *ofs = sb->s_fs_info;
@@ -252,14 +261,24 @@ static int ovl_sync_fs(struct super_block *sb, int wait)
 
         if (!ofs->upper_mnt)
                 return 0;
-        upper_sb = ofs->upper_mnt->mnt_sb;
-        if (!upper_sb->s_op->sync_fs)
+
+        /*
+         * If this is a sync(2) call or an emergency sync, all the super blocks
+         * will be iterated, including upper_sb, so no need to do anything.
+         *
+         * If this is a syncfs(2) call, then we do need to call
+         * sync_filesystem() on upper_sb, but enough if we do it when being
+         * called with wait == 1.
+         */
+        if (!wait)
                 return 0;
 
-        /* real inodes have already been synced by sync_filesystem(ovl_sb) */
+        upper_sb = ofs->upper_mnt->mnt_sb;
+
         down_read(&upper_sb->s_umount);
-        ret = upper_sb->s_op->sync_fs(upper_sb, wait);
+        ret = sync_filesystem(upper_sb);
         up_read(&upper_sb->s_umount);
+
         return ret;
 }
 
@@ -295,6 +314,11 @@ static bool ovl_force_readonly(struct ovl_fs *ofs)
         return (!ofs->upper_mnt || !ofs->workdir);
 }
 
+static const char *ovl_redirect_mode_def(void)
+{
+        return ovl_redirect_dir_def ? "on" : "off";
+}
+
 /**
  * ovl_show_options
  *
@@ -313,12 +337,10 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
         }
         if (ofs->config.default_permissions)
                 seq_puts(m, ",default_permissions");
-        if (ofs->config.redirect_dir != ovl_redirect_dir_def)
-                seq_printf(m, ",redirect_dir=%s",
-                           ofs->config.redirect_dir ? "on" : "off");
+        if (strcmp(ofs->config.redirect_mode, ovl_redirect_mode_def()) != 0)
+                seq_printf(m, ",redirect_dir=%s", ofs->config.redirect_mode);
         if (ofs->config.index != ovl_index_def)
-                seq_printf(m, ",index=%s",
-                           ofs->config.index ? "on" : "off");
+                seq_printf(m, ",index=%s", ofs->config.index ? "on" : "off");
         return 0;
 }
 
@@ -348,8 +370,7 @@ enum {
         OPT_UPPERDIR,
         OPT_WORKDIR,
         OPT_DEFAULT_PERMISSIONS,
-        OPT_REDIRECT_DIR_ON,
-        OPT_REDIRECT_DIR_OFF,
+        OPT_REDIRECT_DIR,
         OPT_INDEX_ON,
         OPT_INDEX_OFF,
         OPT_ERR,
@@ -360,8 +381,7 @@ static const match_table_t ovl_tokens = {
         {OPT_UPPERDIR,                  "upperdir=%s"},
         {OPT_WORKDIR,                   "workdir=%s"},
         {OPT_DEFAULT_PERMISSIONS,       "default_permissions"},
-        {OPT_REDIRECT_DIR_ON,           "redirect_dir=on"},
-        {OPT_REDIRECT_DIR_OFF,          "redirect_dir=off"},
+        {OPT_REDIRECT_DIR,              "redirect_dir=%s"},
         {OPT_INDEX_ON,                  "index=on"},
         {OPT_INDEX_OFF,                 "index=off"},
         {OPT_ERR,                       NULL}
@@ -390,10 +410,37 @@ static char *ovl_next_opt(char **s)
         return sbegin;
 }
 
+static int ovl_parse_redirect_mode(struct ovl_config *config, const char *mode)
+{
+        if (strcmp(mode, "on") == 0) {
+                config->redirect_dir = true;
+                /*
+                 * Does not make sense to have redirect creation without
+                 * redirect following.
+                 */
+                config->redirect_follow = true;
+        } else if (strcmp(mode, "follow") == 0) {
+                config->redirect_follow = true;
+        } else if (strcmp(mode, "off") == 0) {
+                if (ovl_redirect_always_follow)
+                        config->redirect_follow = true;
+        } else if (strcmp(mode, "nofollow") != 0) {
+                pr_err("overlayfs: bad mount option \"redirect_dir=%s\"\n",
+                       mode);
+                return -EINVAL;
+        }
+
+        return 0;
+}
+
 static int ovl_parse_opt(char *opt, struct ovl_config *config)
 {
         char *p;
 
+        config->redirect_mode = kstrdup(ovl_redirect_mode_def(), GFP_KERNEL);
+        if (!config->redirect_mode)
+                return -ENOMEM;
+
         while ((p = ovl_next_opt(&opt)) != NULL) {
                 int token;
                 substring_t args[MAX_OPT_ARGS];
@@ -428,12 +475,11 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
                         config->default_permissions = true;
                         break;
 
-                case OPT_REDIRECT_DIR_ON:
-                        config->redirect_dir = true;
-                        break;
-
-                case OPT_REDIRECT_DIR_OFF:
-                        config->redirect_dir = false;
+                case OPT_REDIRECT_DIR:
+                        kfree(config->redirect_mode);
+                        config->redirect_mode = match_strdup(&args[0]);
+                        if (!config->redirect_mode)
+                                return -ENOMEM;
                         break;
 
                 case OPT_INDEX_ON:
@@ -458,7 +504,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
                 config->workdir = NULL;
         }
 
-        return 0;
+        return ovl_parse_redirect_mode(config, config->redirect_mode);
 }
 
 #define OVL_WORKDIR_NAME "work"
@@ -1160,7 +1206,6 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
         if (!cred)
                 goto out_err;
 
-        ofs->config.redirect_dir = ovl_redirect_dir_def;
         ofs->config.index = ovl_index_def;
         err = ovl_parse_opt((char *) data, &ofs->config);
         if (err)