ovl: fix POSIX ACL setting

Setting POSIX ACL needs special handling:

1) Some permission checks are done by ->setxattr() which now uses mounter's
creds ("ovl: do operations on underlying file system in mounter's
context").  These permission checks need to be done with current cred as
well.

2) Setting ACL can fail for various reasons.  We do not need to copy up in
these cases.

In the mean time switch to using generic_setxattr.

[Arnd Bergmann] Fix link error without POSIX ACL. posix_acl_from_xattr()
doesn't have a 'static inline' implementation when CONFIG_FS_POSIX_ACL is
disabled, and I could not come up with an obvious way to do it.

This instead avoids the link error by defining two sets of ACL operations
and letting the compiler drop one of the two at compile time depending
on CONFIG_FS_POSIX_ACL. This avoids all references to the ACL code,
also leading to smaller code.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Orabug: 26401569

(backport upstream commit d837a49bd57f1ec2f6411efa829fecc34002b110)

Signed-off-by: Shan Hai <shan.hai@oracle.com>
Reviewed-by: Somasundaram Krishnasamy <somasundaram.krishnasamy@oracle.com>

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index bb9c2293517a3d00da88c1360388b241d114f964..da2c44908f07ac8323a67aebcc8b303e0f75064b 100644 (file)
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -961,7 +961,7 @@ const struct inode_operations ovl_dir_inode_operations = {
        .mknod          = ovl_mknod,
        .permission     = ovl_permission,
        .getattr        = ovl_dir_getattr,
-       .setxattr       = ovl_setxattr,
+       .setxattr       = generic_setxattr,
        .getxattr       = ovl_getxattr,
        .listxattr      = ovl_listxattr,
        .removexattr    = ovl_removexattr,

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 97a9f1ad4e8bbeb4c3f3724e7d0fb686d3340e91..8fe071b9ee7e4bfe3e391fb8e06e587b4b1398e7 100644 (file)
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -210,7 +210,9 @@ static int ovl_readlink(struct dentry *dentry, char __user *buf, int bufsiz)
 
 static bool ovl_is_private_xattr(const char *name)
 {
-       return strncmp(name, OVL_XATTR_PRE_NAME, OVL_XATTR_PRE_LEN) == 0;
+#define OVL_XATTR_PRE_NAME OVL_XATTR_PREFIX "."
+       return strncmp(name, OVL_XATTR_PRE_NAME,
+                      sizeof(OVL_XATTR_PRE_NAME) - 1) == 0;
 }
 
 int ovl_setxattr(struct dentry *dentry, const char *name,
@@ -224,10 +226,6 @@ int ovl_setxattr(struct dentry *dentry, const char *name,
        if (err)
                goto out;
 
-       err = -EPERM;
-       if (ovl_is_private_xattr(name))
-               goto out_drop_write;
-
        err = ovl_copy_up(dentry);
        if (err)
                goto out_drop_write;
@@ -435,7 +433,7 @@ static const struct inode_operations ovl_file_inode_operations = {
        .setattr        = ovl_setattr,
        .permission     = ovl_permission,
        .getattr        = ovl_getattr,
-       .setxattr       = ovl_setxattr,
+       .setxattr       = generic_setxattr,
        .getxattr       = ovl_getxattr,
        .listxattr      = ovl_listxattr,
        .removexattr    = ovl_removexattr,
@@ -449,7 +447,7 @@ static const struct inode_operations ovl_symlink_inode_operations = {
        .put_link       = ovl_put_link,
        .readlink       = ovl_readlink,
        .getattr        = ovl_getattr,
-       .setxattr       = ovl_setxattr,
+       .setxattr       = generic_setxattr,
        .getxattr       = ovl_getxattr,
        .listxattr      = ovl_listxattr,
        .removexattr    = ovl_removexattr,

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index b5b1009ed1158204f6bfbeec27cf4e6c2f2fa480..d30ca4cf2192e40f20fa1406af478e6f85b17fce 100644 (file)
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -23,9 +23,9 @@ enum ovl_path_type {
 #define OVL_TYPE_MERGE_OR_LOWER(type) \
        (OVL_TYPE_MERGE(type) || !OVL_TYPE_UPPER(type))
 
-#define OVL_XATTR_PRE_NAME "trusted.overlay."
-#define OVL_XATTR_PRE_LEN  16
-#define OVL_XATTR_OPAQUE   OVL_XATTR_PRE_NAME"opaque"
+
+#define OVL_XATTR_PREFIX XATTR_TRUSTED_PREFIX "overlay"
+#define OVL_XATTR_OPAQUE OVL_XATTR_PREFIX ".opaque"
 
 #define OVL_ISUPPER_MASK 1UL
 

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index c591c6599637aaef6a1e124918bafe51bcfc5937..8ec4feb17f137f2ef70363b676bb9a48f7405a32 100644 (file)
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -19,6 +19,7 @@
 #include <linux/sched.h>
 #include <linux/statfs.h>
 #include <linux/seq_file.h>
+#include <linux/posix_acl_xattr.h>
 #include "overlayfs.h"
 
 MODULE_AUTHOR("Miklos Szeredi <miklos@szeredi.hu>");
@@ -861,6 +862,94 @@ static unsigned int ovl_split_lowerdirs(char *str)
        return ctr;
 }
 
+static int ovl_posix_acl_xattr_set(struct dentry *dentry, const char *name,
+                                  const void *value, size_t size, int flags,
+                                  int handler_flags)
+{
+       struct dentry *workdir = ovl_workdir(dentry);
+       struct inode *realinode = ovl_inode_real(d_inode(dentry), NULL);
+       struct posix_acl *acl = NULL;
+       int err;
+
+       /* Check that everything is OK before copy-up */
+       if (value) {
+               acl = posix_acl_from_xattr(&init_user_ns, value, size);
+               if (IS_ERR(acl))
+                       return PTR_ERR(acl);
+       }
+       err = -EOPNOTSUPP;
+       if (!IS_POSIXACL(d_inode(workdir)))
+               goto out_acl_release;
+       if (!realinode->i_op->set_acl)
+               goto out_acl_release;
+       if (handler_flags == ACL_TYPE_DEFAULT &&
+                               !S_ISDIR(d_inode(dentry)->i_mode)) {
+               err = acl ? -EACCES : 0;
+               goto out_acl_release;
+       }
+       err = -EPERM;
+       if (!inode_owner_or_capable(d_inode(dentry)))
+               goto out_acl_release;
+
+       posix_acl_release(acl);
+
+       return ovl_setxattr(dentry, name, value, size, flags);
+
+out_acl_release:
+       posix_acl_release(acl);
+       return err;
+}
+
+static int ovl_other_xattr_set(struct dentry *dentry, const char *name,
+                              const void *value, size_t size, int flags,
+                              int handler_flags)
+{
+       return ovl_setxattr(dentry, name, value, size, flags);
+}
+
+static int ovl_own_xattr_set(struct dentry *dentry, const char *name,
+                            const void *value, size_t size, int flags,
+                            int handler_flags)
+{
+       return -EPERM;
+}
+
+static const struct xattr_handler ovl_posix_acl_access_xattr_handler = {
+       .prefix = XATTR_NAME_POSIX_ACL_ACCESS,
+       .flags = ACL_TYPE_ACCESS,
+       .set = ovl_posix_acl_xattr_set,
+};
+
+static const struct xattr_handler ovl_posix_acl_default_xattr_handler = {
+       .prefix = XATTR_NAME_POSIX_ACL_DEFAULT,
+       .flags = ACL_TYPE_DEFAULT,
+       .set = ovl_posix_acl_xattr_set,
+};
+
+static const struct xattr_handler ovl_own_xattr_handler = {
+       .prefix = OVL_XATTR_PREFIX,
+       .set = ovl_own_xattr_set,
+};
+
+static const struct xattr_handler ovl_other_xattr_handler = {
+       .prefix = "", /* catch all */
+       .set = ovl_other_xattr_set,
+};
+
+static const struct xattr_handler *ovl_xattr_handlers[] = {
+       &ovl_posix_acl_access_xattr_handler,
+       &ovl_posix_acl_default_xattr_handler,
+       &ovl_own_xattr_handler,
+       &ovl_other_xattr_handler,
+       NULL
+};
+
+static const struct xattr_handler *ovl_xattr_noacl_handlers[] = {
+       &ovl_own_xattr_handler,
+       &ovl_other_xattr_handler,
+       NULL,
+};
+
 static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 {
        struct path upperpath = { NULL, NULL };
@@ -1065,6 +1154,10 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 
        sb->s_magic = OVERLAYFS_SUPER_MAGIC;
        sb->s_op = &ovl_super_operations;
+       if (IS_ENABLED(CONFIG_FS_POSIX_ACL))
+               sb->s_xattr = ovl_xattr_handlers;
+       else
+               sb->s_xattr = ovl_xattr_noacl_handlers;
        sb->s_root = root_dentry;
        sb->s_fs_info = ufs;
        sb->s_flags |= MS_POSIXACL;