Skip to content

Commit

Permalink
ovl: fix POSIX ACL setting
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Miklos Szeredi committed Jul 29, 2016
1 parent 51f7e52 commit d837a49
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 11 deletions.
2 changes: 1 addition & 1 deletion fs/overlayfs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,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,
Expand Down
12 changes: 5 additions & 7 deletions fs/overlayfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,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, struct inode *inode,
Expand All @@ -205,10 +207,6 @@ int ovl_setxattr(struct dentry *dentry, struct inode *inode,
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;
Expand Down Expand Up @@ -389,7 +387,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,
Expand All @@ -402,7 +400,7 @@ static const struct inode_operations ovl_symlink_inode_operations = {
.get_link = ovl_get_link,
.readlink = ovl_readlink,
.getattr = ovl_getattr,
.setxattr = ovl_setxattr,
.setxattr = generic_setxattr,
.getxattr = ovl_getxattr,
.listxattr = ovl_listxattr,
.removexattr = ovl_removexattr,
Expand Down
6 changes: 3 additions & 3 deletions fs/overlayfs/overlayfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
95 changes: 95 additions & 0 deletions fs/overlayfs/super.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,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>");
Expand Down Expand Up @@ -966,6 +967,96 @@ static unsigned int ovl_split_lowerdirs(char *str)
return ctr;
}

static int ovl_posix_acl_xattr_set(const struct xattr_handler *handler,
struct dentry *dentry, struct inode *inode,
const char *name, const void *value,
size_t size, int flags)
{
struct dentry *workdir = ovl_workdir(dentry);
struct inode *realinode = ovl_inode_real(inode, 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(inode->i_mode)) {
err = acl ? -EACCES : 0;
goto out_acl_release;
}
err = -EPERM;
if (!inode_owner_or_capable(inode))
goto out_acl_release;

posix_acl_release(acl);

return ovl_setxattr(dentry, inode, handler->name, value, size, flags);

out_acl_release:
posix_acl_release(acl);
return err;
}

static int ovl_other_xattr_set(const struct xattr_handler *handler,
struct dentry *dentry, struct inode *inode,
const char *name, const void *value,
size_t size, int flags)
{
return ovl_setxattr(dentry, inode, name, value, size, flags);
}

static int ovl_own_xattr_set(const struct xattr_handler *handler,
struct dentry *dentry, struct inode *inode,
const char *name, const void *value,
size_t size, int flags)
{
return -EPERM;
}

static const struct xattr_handler ovl_posix_acl_access_xattr_handler = {
.name = 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 = {
.name = 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 };
Expand Down Expand Up @@ -1178,6 +1269,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;
Expand Down

0 comments on commit d837a49

Please sign in to comment.