ovl: don't follow redirects if redirect_dir=off

Overlayfs is following redirects even when redirects are disabled. If this
is unintentional (probably the majority of cases) then this can be a
problem.  E.g. upper layer comes from untrusted USB drive, and attacker
crafts a redirect to enable read access to otherwise unreadable
directories.

If "redirect_dir=off", then turn off following as well as creation of
redirects.  If "redirect_dir=follow", then turn on following, but turn off
creation of redirects (which is what "redirect_dir=off" does now).

This is a backward incompatible change, so make it dependent on a config
option.

Reported-by: David Howells <dhowells@redhat.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>

diff --git a/Documentation/filesystems/overlayfs.txt b/Documentation/filesystems/overlayfs.txt
index 8caa60734647f70a777b8a568ba9f2dd99182fb8..e6a5f4912b6d4a4910ed1d2fc494fff304ab47ff 100644 (file)
--- 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 cbfc196e5dc53b2cb2376524c5955af20bd07ea6..5ac4154668613d158d63301272039ee5f5b814e4 100644 (file)
--- 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/namei.c b/fs/overlayfs/namei.c
index 625ed8066570607b6140a0c22c39f152135a5c81..2a12dc2e9840f803214732226cfebeb8e1ac01c3 100644 (file)
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -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/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 752bab645879e5fce43e86d45e835d94d3c44b22..9d0bc03bf6e4563ef280fe5e8f695cc9f1e8abd5 100644 (file)
--- 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/super.c b/fs/overlayfs/super.c
index 288d20f9a55a220d3782f4eaf10e62c491c3fc5c..13a8a8617e44bc92f66a6a2e78248afc068cf6bc 100644 (file)
--- 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);
@@ -295,6 +303,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 +326,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 +359,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 +370,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 +399,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 +464,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 +493,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 +1195,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)