linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] ovl: honor flag MS_SILENT at mount
@ 2015-06-29 17:18 Konstantin Khlebnikov
  2015-06-29 17:18 ` [PATCH 2/4] ovl: honor MS_NOEXEC flag at overlayfs mount Konstantin Khlebnikov
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Konstantin Khlebnikov @ 2015-06-29 17:18 UTC (permalink / raw)
  To: linux-fsdevel, Miklos Szeredi, linux-kernel, Alexander Viro,
	linux-unionfs
  Cc: linux-security-module

This patch hides error about missing lowerdir if MS_SILENT is set.

We use mount(NULL, "/", "overlay", MS_SILENT, NULL) for testing support of
overlayfs: syscall returns -ENODEV if it's not supported. Otherwise kernel
automatically loads module and returns -EINVAL because lowerdir is missing.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 fs/overlayfs/super.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index bf8537c7f455..00e1d0b3abdd 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -840,7 +840,8 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 
 	err = -EINVAL;
 	if (!ufs->config.lowerdir) {
-		pr_err("overlayfs: missing 'lowerdir'\n");
+		if (!silent)
+			pr_err("overlayfs: missing 'lowerdir'\n");
 		goto out_free_config;
 	}
 


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/4] ovl: honor MS_NOEXEC flag at overlayfs mount
  2015-06-29 17:18 [PATCH 1/4] ovl: honor flag MS_SILENT at mount Konstantin Khlebnikov
@ 2015-06-29 17:18 ` Konstantin Khlebnikov
  2015-06-29 17:18 ` [PATCH 3/4] ovl: honor MS_NOSUID " Konstantin Khlebnikov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Konstantin Khlebnikov @ 2015-06-29 17:18 UTC (permalink / raw)
  To: linux-fsdevel, Miklos Szeredi, linux-kernel, Alexander Viro,
	linux-unionfs
  Cc: linux-security-module

Overlayfs opens files directly at underlying filesystem. File->f_path.mnt
points to private clone of lower/upper mount. Thus MS_NOEXEC at overlayfs
mount has no effect because all checks are done against underlying mount.

This patch clears "noexec" at private lower/upper mounts (if noexec isn't
locked) and checks MNT_NOEXEC in overlayfs mount flags in ->dentry_open().

MS_NODEV already works correctly because kernel checks it before opening.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 Documentation/filesystems/Locking |    2 +-
 Documentation/filesystems/vfs.txt |    2 +-
 fs/open.c                         |    2 +-
 fs/overlayfs/inode.c              |   14 ++++++++++++--
 fs/overlayfs/super.c              |   14 ++++++++++++++
 include/linux/fs.h                |    3 ++-
 6 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 6a34a0f4d37c..f949910a9ff8 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -67,7 +67,7 @@ prototypes:
 				struct file *, unsigned open_flag,
 				umode_t create_mode, int *opened);
 	int (*tmpfile) (struct inode *, struct dentry *, umode_t);
-	int (*dentry_open)(struct dentry *, struct file *, const struct cred *);
+	int (*dentry_open)(struct path *, struct file *, const struct cred *);
 
 locking rules:
 	all may block
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 5eb8456fc41e..f5557f06ce12 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -364,7 +364,7 @@ struct inode_operations {
 	int (*atomic_open)(struct inode *, struct dentry *, struct file *,
 			unsigned open_flag, umode_t create_mode, int *opened);
 	int (*tmpfile) (struct inode *, struct dentry *, umode_t);
-	int (*dentry_open)(struct dentry *, struct file *, const struct cred *);
+	int (*dentry_open)(struct path *, struct file *, const struct cred *);
 };
 
 Again, all methods are called without any locks being held, unless
diff --git a/fs/open.c b/fs/open.c
index e0250bdcc440..b619e28b2740 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -865,7 +865,7 @@ int vfs_open(const struct path *path, struct file *filp,
 	struct inode *inode = path->dentry->d_inode;
 
 	if (inode->i_op->dentry_open)
-		return inode->i_op->dentry_open(path->dentry, filp, cred);
+		return inode->i_op->dentry_open(path, filp, cred);
 	else {
 		filp->f_path = *path;
 		return do_dentry_open(filp, NULL, cred);
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 308379b2d0b2..b9a40c8dbd1e 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -10,6 +10,7 @@
 #include <linux/fs.h>
 #include <linux/slab.h>
 #include <linux/xattr.h>
+#include <linux/mount.h>
 #include "overlayfs.h"
 
 static int ovl_copy_up_last(struct dentry *dentry, struct iattr *attr,
@@ -337,14 +338,23 @@ static bool ovl_open_need_copy_up(int flags, enum ovl_path_type type,
 	return true;
 }
 
-static int ovl_dentry_open(struct dentry *dentry, struct file *file,
-		    const struct cred *cred)
+static int ovl_dentry_open(const struct path *path, struct file *file,
+			   const struct cred *cred)
 {
+	struct dentry *dentry = path->dentry;
 	int err;
 	struct path realpath;
 	enum ovl_path_type type;
 	bool want_write = false;
 
+	/*
+	 * After opening file->f_path.mnt points to private lower/upper mount.
+	 * Here is the last chance to check flags at overlayfs mount.
+	 */
+	if ((file->f_flags & __FMODE_EXEC) &&
+	    (path->mnt->mnt_flags & MNT_NOEXEC))
+		return -EACCES;
+
 	type = ovl_path_real(dentry, &realpath);
 	if (ovl_open_need_copy_up(file->f_flags, type, realpath.dentry)) {
 		want_write = true;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 00e1d0b3abdd..64c3714acf38 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -923,6 +923,13 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 			goto out_put_lowerpath;
 		}
 
+		/*
+		 * Drop noexec at upper mount if allowed. We'll check
+		 * MNT_NOEXEC at overlayfs mount in ovl_dentry_open().
+		 */
+		if (!(ufs->upper_mnt->mnt_flags & MNT_LOCK_NOEXEC))
+			ufs->upper_mnt->mnt_flags &= ~MNT_NOEXEC;
+
 		ufs->workdir = ovl_workdir_create(ufs->upper_mnt, workpath.dentry);
 		err = PTR_ERR(ufs->workdir);
 		if (IS_ERR(ufs->workdir)) {
@@ -951,6 +958,13 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		 */
 		mnt->mnt_flags |= MNT_READONLY;
 
+		/*
+		 * Drop noexec at lower mount if allowed. We'll check
+		 * MNT_NOEXEC at overlayfs mount in ovl_dentry_open().
+		 */
+		if (!(mnt->mnt_flags & MNT_LOCK_NOEXEC))
+			mnt->mnt_flags &= ~MNT_NOEXEC;
+
 		ufs->lower_mnt[ufs->numlower] = mnt;
 		ufs->numlower++;
 	}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e351da4a934f..55c3edd9519e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1653,7 +1653,8 @@ struct inode_operations {
 	int (*set_acl)(struct inode *, struct posix_acl *, int);
 
 	/* WARNING: probably going away soon, do not use! */
-	int (*dentry_open)(struct dentry *, struct file *, const struct cred *);
+	int (*dentry_open)(const struct path *, struct file *,
+			   const struct cred *);
 } ____cacheline_aligned;
 
 ssize_t rw_copy_check_uvector(int type, const struct iovec __user * uvector,

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/4] ovl: honor MS_NOSUID flag at overlayfs mount
  2015-06-29 17:18 [PATCH 1/4] ovl: honor flag MS_SILENT at mount Konstantin Khlebnikov
  2015-06-29 17:18 ` [PATCH 2/4] ovl: honor MS_NOEXEC flag at overlayfs mount Konstantin Khlebnikov
@ 2015-06-29 17:18 ` Konstantin Khlebnikov
  2015-06-29 17:19 ` [PATCH 4/4] ovl: forbid overlayfs on top of overlayfs Konstantin Khlebnikov
  2016-03-07  9:50 ` [PATCH 1/4] ovl: honor flag MS_SILENT at mount Miklos Szeredi
  3 siblings, 0 replies; 5+ messages in thread
From: Konstantin Khlebnikov @ 2015-06-29 17:18 UTC (permalink / raw)
  To: linux-fsdevel, Miklos Szeredi, linux-kernel, Alexander Viro,
	linux-unionfs
  Cc: linux-security-module

Mount flag MS_NOSUID in overlayfs has the same problem as MS_NOEXEC:
overlayfs opens files at underlying filesystem thus kernel check flags
at private clone of lower/upper mountpoint instead of overlayfs itself.

This patch saves MNT_NOSUID from overlayfs mount in file->f_mode when
opens file and checks it together with MNT_NOSUID. This behavior
stricter than usual: opened file stays nosuid even after remounting
overlayfs without MS_NOSUID but that should be fine.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 fs/exec.c                |    3 ++-
 fs/overlayfs/inode.c     |    4 ++++
 fs/overlayfs/super.c     |    6 ++++++
 include/linux/fs.h       |    3 +++
 security/commoncap.c     |    3 ++-
 security/selinux/hooks.c |    3 ++-
 6 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 1977c2a553ac..0d81c6a43ef4 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1289,7 +1289,8 @@ static void bprm_fill_uid(struct linux_binprm *bprm)
 	bprm->cred->euid = current_euid();
 	bprm->cred->egid = current_egid();
 
-	if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
+	if ((bprm->file->f_mode & FMODE_NOSUID) ||
+	    (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID))
 		return;
 
 	if (task_no_new_privs(current))
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index b9a40c8dbd1e..13be68335b1d 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -373,6 +373,10 @@ static int ovl_dentry_open(const struct path *path, struct file *file,
 	}
 
 	err = vfs_open(&realpath, file, cred);
+
+	if (path->mnt->mnt_flags & MNT_NOSUID)
+		file->f_mode |= FMODE_NOSUID;
+
 out_drop_write:
 	if (want_write)
 		ovl_drop_write(dentry);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 64c3714acf38..e5db31f78c7d 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -930,6 +930,9 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		if (!(ufs->upper_mnt->mnt_flags & MNT_LOCK_NOEXEC))
 			ufs->upper_mnt->mnt_flags &= ~MNT_NOEXEC;
 
+		if (!(ufs->upper_mnt->mnt_flags & MNT_LOCK_NOSUID))
+			ufs->upper_mnt->mnt_flags &= ~MNT_NOSUID;
+
 		ufs->workdir = ovl_workdir_create(ufs->upper_mnt, workpath.dentry);
 		err = PTR_ERR(ufs->workdir);
 		if (IS_ERR(ufs->workdir)) {
@@ -965,6 +968,9 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 		if (!(mnt->mnt_flags & MNT_LOCK_NOEXEC))
 			mnt->mnt_flags &= ~MNT_NOEXEC;
 
+		if (!(mnt->mnt_flags & MNT_LOCK_NOSUID))
+			mnt->mnt_flags &= ~MNT_NOSUID;
+
 		ufs->lower_mnt[ufs->numlower] = mnt;
 		ufs->numlower++;
 	}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 55c3edd9519e..abc5cd658ad0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -136,6 +136,9 @@ typedef void (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 /* Has write method(s) */
 #define FMODE_CAN_WRITE         ((__force fmode_t)0x40000)
 
+/* Ignode S_ISUID and S_ISGID bits when executing this file */
+#define FMODE_NOSUID		((__force fmode_t)0x80000)
+
 /* File was opened by fanotify and shouldn't generate fanotify events */
 #define FMODE_NONOTIFY		((__force fmode_t)0x4000000)
 
diff --git a/security/commoncap.c b/security/commoncap.c
index d103f5a4043d..15deb600503b 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -437,7 +437,8 @@ static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_c
 	if (!file_caps_enabled)
 		return 0;
 
-	if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
+	if ((bprm->file->f_mode & FMODE_NOSUID) ||
+	    (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID))
 		return 0;
 
 	rc = get_vfs_caps_from_disk(bprm->file->f_path.dentry, &vcaps);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 623108199641..437e0e131fbd 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2137,7 +2137,8 @@ static int check_nnp_nosuid(const struct linux_binprm *bprm,
 			    const struct task_security_struct *new_tsec)
 {
 	int nnp = (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS);
-	int nosuid = (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID);
+	int nosuid = (bprm->file->f_mode & FMODE_NOSUID) ||
+		     (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID);
 	int rc;
 
 	if (!nnp && !nosuid)

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 4/4] ovl: forbid overlayfs on top of overlayfs
  2015-06-29 17:18 [PATCH 1/4] ovl: honor flag MS_SILENT at mount Konstantin Khlebnikov
  2015-06-29 17:18 ` [PATCH 2/4] ovl: honor MS_NOEXEC flag at overlayfs mount Konstantin Khlebnikov
  2015-06-29 17:18 ` [PATCH 3/4] ovl: honor MS_NOSUID " Konstantin Khlebnikov
@ 2015-06-29 17:19 ` Konstantin Khlebnikov
  2016-03-07  9:50 ` [PATCH 1/4] ovl: honor flag MS_SILENT at mount Miklos Szeredi
  3 siblings, 0 replies; 5+ messages in thread
From: Konstantin Khlebnikov @ 2015-06-29 17:19 UTC (permalink / raw)
  To: linux-fsdevel, Miklos Szeredi, linux-kernel, Alexander Viro,
	linux-unionfs
  Cc: linux-security-module

Mounting overlayfs using other overlayfs instance as lower or upper
layer triggers false-positive lockdep warning about possible locking
recursion of ->i_mutex in iterate_dir().

Nesting already limited with FILESYSTEM_MAX_STACK_DEPTH = 2 to prevent
kernel stack overflow. Overlayfs supports multiple lower layers thus
the same configuration could be constructed without nesting.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 fs/overlayfs/super.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index e5db31f78c7d..381af9a5ece9 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -700,10 +700,13 @@ static bool ovl_is_allowed_fs_type(struct dentry *root)
 
 	/*
 	 * We don't support:
+	 *  - overlayfs
 	 *  - automount filesystems
 	 *  - filesystems with revalidate (FIXME for lower layer)
 	 *  - filesystems with case insensitive names
 	 */
+	if (root->d_sb->s_op == &ovl_super_operations)
+		return false;
 	if (dop &&
 	    (dop->d_manage || dop->d_automount ||
 	     dop->d_revalidate || dop->d_weak_revalidate ||


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/4] ovl: honor flag MS_SILENT at mount
  2015-06-29 17:18 [PATCH 1/4] ovl: honor flag MS_SILENT at mount Konstantin Khlebnikov
                   ` (2 preceding siblings ...)
  2015-06-29 17:19 ` [PATCH 4/4] ovl: forbid overlayfs on top of overlayfs Konstantin Khlebnikov
@ 2016-03-07  9:50 ` Miklos Szeredi
  3 siblings, 0 replies; 5+ messages in thread
From: Miklos Szeredi @ 2016-03-07  9:50 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: Linux-Fsdevel, Kernel Mailing List, Alexander Viro, linux-unionfs, LSM

On Mon, Jun 29, 2015 at 7:18 PM, Konstantin Khlebnikov
<khlebnikov@yandex-team.ru> wrote:
> This patch hides error about missing lowerdir if MS_SILENT is set.
>
> We use mount(NULL, "/", "overlay", MS_SILENT, NULL) for testing support of
> overlayfs: syscall returns -ENODEV if it's not supported. Otherwise kernel
> automatically loads module and returns -EINVAL because lowerdir is missing.

Queued.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-03-07  9:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-29 17:18 [PATCH 1/4] ovl: honor flag MS_SILENT at mount Konstantin Khlebnikov
2015-06-29 17:18 ` [PATCH 2/4] ovl: honor MS_NOEXEC flag at overlayfs mount Konstantin Khlebnikov
2015-06-29 17:18 ` [PATCH 3/4] ovl: honor MS_NOSUID " Konstantin Khlebnikov
2015-06-29 17:19 ` [PATCH 4/4] ovl: forbid overlayfs on top of overlayfs Konstantin Khlebnikov
2016-03-07  9:50 ` [PATCH 1/4] ovl: honor flag MS_SILENT at mount Miklos Szeredi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).