Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 438c84c2 authored by Miklos Szeredi's avatar Miklos Szeredi
Browse files

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: default avatarDavid Howells <dhowells@redhat.com>
Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
parent ae64f9bd
Loading
Loading
Loading
Loading
+34 −0
Original line number Diff line number Diff line
@@ -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
---------------

+10 −0
Original line number Diff line number Diff line
@@ -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
+16 −0
Original line number Diff line number Diff line
@@ -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;

+2 −0
Original line number Diff line number Diff line
@@ -14,6 +14,8 @@ struct ovl_config {
	char *workdir;
	bool default_permissions;
	bool redirect_dir;
	bool redirect_follow;
	const char *redirect_mode;
	bool index;
};

+51 −17
Original line number Diff line number Diff line
@@ -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)