[v2,03/18] ovl: stack miscattr ops

Commit Message

Miklos Szeredi March 22, 2021, 2:49 p.m. UTC

Add stacking for the miscattr operations.

Add hack for calling security_file_ioctl() for now.  Probably better to
have a pair of specific hooks for these operations.

Signed-off-by: Miklos Szeredi <[e-mail]>
---
 fs/overlayfs/dir.c       |  2 ++
 fs/overlayfs/inode.c     | 77 ++++++++++++++++++++++++++++++++++++++++
 fs/overlayfs/overlayfs.h |  3 ++
 3 files changed, 82 insertions(+)

Comments

Al Viro March 24, 2021, 5:09 a.m. UTC | #1

On Mon, Mar 22, 2021 at 03:49:01PM +0100, Miklos Szeredi wrote:

> +int ovl_miscattr_set(struct user_namespace *mnt_userns,
> +		     struct dentry *dentry, struct miscattr *ma)
> +{
> +	struct inode *inode = d_inode(dentry);
> +	struct dentry *upperdentry;
> +	const struct cred *old_cred;
> +	int err;
> +
> +	err = ovl_want_write(dentry);
> +	if (err)
> +		goto out;
> +
> +	err = ovl_copy_up(dentry);
> +	if (!err) {
> +		upperdentry = ovl_dentry_upper(dentry);
> +
> +		old_cred = ovl_override_creds(inode->i_sb);
> +		err = ovl_security_miscattr(dentry, ma, true);
> +		if (!err)
> +			err = vfs_miscattr_set(&init_user_ns, upperdentry, ma);
> +		revert_creds(old_cred);
> +		ovl_copyflags(ovl_inode_real(inode), inode);
> +	}
> +	ovl_drop_write(dentry);
> +out:
> +	return err;
> +}

Umm...  No equivalents of
        /*  
         * Prevent copy up if immutable and has no CAP_LINUX_IMMUTABLE
         * capability.
         */ 
        ret = -EPERM;
        if (!ovl_has_upperdata(inode) && IS_IMMUTABLE(inode) &&
            !capable(CAP_LINUX_IMMUTABLE))
                goto unlock;

        ret = ovl_maybe_copy_up(file_dentry(file), O_WRONLY);
        if (ret)
                goto unlock;
in the current tree?

Al Viro March 24, 2021, 5:18 a.m. UTC | #2

On Wed, Mar 24, 2021 at 05:09:59AM +0000, Al Viro wrote:
> On Mon, Mar 22, 2021 at 03:49:01PM +0100, Miklos Szeredi wrote:

> Umm...  No equivalents of
>         /*  
>          * Prevent copy up if immutable and has no CAP_LINUX_IMMUTABLE
>          * capability.
>          */ 
>         ret = -EPERM;
>         if (!ovl_has_upperdata(inode) && IS_IMMUTABLE(inode) &&
>             !capable(CAP_LINUX_IMMUTABLE))
>                 goto unlock;
> 

Nevermind, you take care of that in the caller...

Patch

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 836f14b9d3a6..3cf091d0d0ff 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1301,4 +1301,6 @@  const struct inode_operations ovl_dir_inode_operations = {
 	.listxattr	= ovl_listxattr,
 	.get_acl	= ovl_get_acl,
 	.update_time	= ovl_update_time,
+	.miscattr_get	= ovl_miscattr_get,
+	.miscattr_set	= ovl_miscattr_set,
 };
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 003cf83bf78a..4915eea6d115 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -11,6 +11,8 @@ 
 #include <linux/posix_acl.h>
 #include <linux/ratelimit.h>
 #include <linux/fiemap.h>
+#include <linux/miscattr.h>
+#include <linux/security.h>
 #include "overlayfs.h"
 
 
@@ -500,6 +502,79 @@  static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	return err;
 }
 
+/*
+ * Work around the fact that security_file_ioctl() takes a file argument.
+ * Introducing security_inode_miscattr_get/set() hooks would solve this issue
+ * properly.
+ */
+static int ovl_security_miscattr(struct dentry *dentry, struct miscattr *ma,
+				 bool set)
+{
+	struct path realpath;
+	struct file *file;
+	unsigned int cmd;
+	int err;
+
+	ovl_path_real(dentry, &realpath);
+	file = dentry_open(&realpath, O_RDONLY, current_cred());
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	if (set)
+		cmd = ma->xattr_valid ? FS_IOC_FSSETXATTR : FS_IOC_SETFLAGS;
+	else
+		cmd = ma->xattr_valid ? FS_IOC_FSGETXATTR : FS_IOC_GETFLAGS;
+
+	err = security_file_ioctl(file, cmd, 0);
+	fput(file);
+
+	return err;
+}
+
+int ovl_miscattr_set(struct user_namespace *mnt_userns,
+		     struct dentry *dentry, struct miscattr *ma)
+{
+	struct inode *inode = d_inode(dentry);
+	struct dentry *upperdentry;
+	const struct cred *old_cred;
+	int err;
+
+	err = ovl_want_write(dentry);
+	if (err)
+		goto out;
+
+	err = ovl_copy_up(dentry);
+	if (!err) {
+		upperdentry = ovl_dentry_upper(dentry);
+
+		old_cred = ovl_override_creds(inode->i_sb);
+		err = ovl_security_miscattr(dentry, ma, true);
+		if (!err)
+			err = vfs_miscattr_set(&init_user_ns, upperdentry, ma);
+		revert_creds(old_cred);
+		ovl_copyflags(ovl_inode_real(inode), inode);
+	}
+	ovl_drop_write(dentry);
+out:
+	return err;
+}
+
+int ovl_miscattr_get(struct dentry *dentry, struct miscattr *ma)
+{
+	struct inode *inode = d_inode(dentry);
+	struct dentry *realdentry = ovl_dentry_real(dentry);
+	const struct cred *old_cred;
+	int err;
+
+	old_cred = ovl_override_creds(inode->i_sb);
+	err = ovl_security_miscattr(dentry, ma, false);
+	if (!err)
+		err = vfs_miscattr_get(realdentry, ma);
+	revert_creds(old_cred);
+
+	return err;
+}
+
 static const struct inode_operations ovl_file_inode_operations = {
 	.setattr	= ovl_setattr,
 	.permission	= ovl_permission,
@@ -508,6 +583,8 @@  static const struct inode_operations ovl_file_inode_operations = {
 	.get_acl	= ovl_get_acl,
 	.update_time	= ovl_update_time,
 	.fiemap		= ovl_fiemap,
+	.miscattr_get	= ovl_miscattr_get,
+	.miscattr_set	= ovl_miscattr_set,
 };
 
 static const struct inode_operations ovl_symlink_inode_operations = {
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 95cff83786a5..abd4d1316897 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -521,6 +521,9 @@  int __init ovl_aio_request_cache_init(void);
 void ovl_aio_request_cache_destroy(void);
 long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
 long ovl_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
+int ovl_miscattr_get(struct dentry *dentry, struct miscattr *ma);
+int ovl_miscattr_set(struct user_namespace *mnt_userns,
+		     struct dentry *dentry, struct miscattr *ma);
 
 /* copy_up.c */
 int ovl_copy_up(struct dentry *dentry);