lkml.org 
[lkml]   [2011]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] tmpfs: implement generic xattr support
Quoting Miklos Szeredi (miklos@szeredi.hu):
> Michal Suchanek <hramrach@centrum.cz> writes:
> > Applying this patch is not sufficient. Apparently more xattrs are
> > needed but adding them on top of this patch should be easy.
> >
> > The ones mentioned in the overlayfs doc are
> >
> > trusted.overlay.whiteout
> > trusted.overlay.opaque
> >
> > The patch implements security.*
>
> Here's an updated patch. It changes a number of things:
>
> - it implements shmem specific xattr ops instead of using the generic_*
> functions. Which means that there's no back and forth between the
> VFS and the filesystem. I basically copied the btrfs way of doing
> things.
>
> - adds a new config option: CONFIG_TMPFS_XATTR and makes
> CONFIG_TMPFS_POSIX_ACL depend on this. This way xattr support can be
> turned on without also adding ACL support.
>
> - now supports trusted.* namespace needed by overlayfs in addition to
> security.*. Doesn't yet support user.* since that needs more thought
> wrt. kernel memory use.
>
> - supports xattrs on symlinks, again needed by overlayfs
>
> Hugh, Eric, does this look acceptable?
>
> Thanks,
> Miklos
>
> ---
> From: Eric Paris <eparis@redhat.com>
> Subject: tmpfs: implement generic xattr support
>
> This patch implements generic xattrs for tmpfs filesystems. The feodra
> project, while trying to replace suid apps with file capabilities,
> realized that tmpfs, which is used on the build systems, does not
> support file capabilities and thus cannot be used to build packages
> which use file capabilities. Xattrs are also needed for overlayfs.
>
> The xattr interface is a bit, odd. If a filesystem does not implement any
> {get,set,list}xattr functions the VFS will call into some random LSM hooks and
> the running LSM can then implement some method for handling xattrs. SELinux
> for example provides a method to support security.selinux but no other
> security.* xattrs.
>
> As it stands today when one enables CONFIG_TMPFS_POSIX_ACL tmpfs will have
> xattr handler routines specifically to handle acls. Because of this tmpfs
> would loose the VFS/LSM helpers to support the running LSM. To make up for
> that tmpfs had stub functions that did nothing but call into the LSM hooks
> which implement the helpers.
>
> This new patch does not use the LSM fallback functions and instead
> just implements a native get/set/list xattr feature for the full
> security.* and trusted.* namespace like a normal filesystem. This
> means that tmpfs can now support both security.selinux and
> security.capability, which was not previously possible.
>
> The basic implementation is that I attach a:
>
> struct shmem_xattr {
> struct list_head list; /* anchored by shmem_inode_info->xattr_list */
> char *name;
> size_t size;
> char value[0];
> };
>
> Into the struct shmem_inode_info for each xattr that is set. This
> implementation could easily support the user.* namespace as well,
> except some care needs to be taken to prevent large amounts of
> unswappable memory being allocated for unprivileged users.
>
> Signed-off-by: Eric Paris <eparis@redhat.com>
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>

Looks good to me.

Acked-by: Serge Hallyn <serge.hallyn@ubuntu.com>

thanks,
-serge

> ---
> fs/Kconfig | 18 ++
> include/linux/shmem_fs.h | 1
> mm/shmem.c | 302 +++++++++++++++++++++++++++++++++++++++--------
> 3 files changed, 273 insertions(+), 48 deletions(-)
>
> Index: linux-2.6/fs/Kconfig
> ===================================================================
> --- linux-2.6.orig/fs/Kconfig 2011-04-19 21:09:33.000000000 +0200
> +++ linux-2.6/fs/Kconfig 2011-04-19 21:09:35.000000000 +0200
> @@ -121,9 +121,25 @@ config TMPFS
>
> See <file:Documentation/filesystems/tmpfs.txt> for details.
>
> +config TMPFS_XATTR
> + bool "Tmpfs extended attributes"
> + depends on TMPFS
> + default y
> + help
> + Extended attributes are name:value pairs associated with inodes by
> + the kernel or by users (see the attr(5) manual page, or visit
> + <http://acl.bestbits.at/> for details).
> +
> + Currently this enables support for the trusted.* and
> + security.* namespaces.
> +
> + If unsure, say N.
> +
> + You need this for POSIX ACL support on tmpfs.
> +
> config TMPFS_POSIX_ACL
> bool "Tmpfs POSIX Access Control Lists"
> - depends on TMPFS
> + depends on TMPFS_XATTR
> select GENERIC_ACL
> help
> POSIX Access Control Lists (ACLs) support permissions for users and
> Index: linux-2.6/include/linux/shmem_fs.h
> ===================================================================
> --- linux-2.6.orig/include/linux/shmem_fs.h 2011-04-19 21:09:25.000000000 +0200
> +++ linux-2.6/include/linux/shmem_fs.h 2011-04-19 21:09:35.000000000 +0200
> @@ -19,6 +19,7 @@ struct shmem_inode_info {
> struct page *i_indirect; /* top indirect blocks page */
> swp_entry_t i_direct[SHMEM_NR_DIRECT]; /* first blocks */
> struct list_head swaplist; /* chain of maybes on swap */
> + struct list_head xattr_list; /* list of shmem_xattr */
> struct inode vfs_inode;
> };
>
> Index: linux-2.6/mm/shmem.c
> ===================================================================
> --- linux-2.6.orig/mm/shmem.c 2011-04-19 21:09:25.000000000 +0200
> +++ linux-2.6/mm/shmem.c 2011-04-19 21:09:35.000000000 +0200
> @@ -99,6 +99,13 @@ static struct vfsmount *shm_mnt;
> /* Pretend that each entry is of this size in directory's i_size */
> #define BOGO_DIRENT_SIZE 20
>
> +struct shmem_xattr {
> + struct list_head list; /* anchored by shmem_inode_info->xattr_list */
> + char *name; /* xattr name */
> + size_t size;
> + char value[0];
> +};
> +
> /* Flag allocation requirements to shmem_getpage and shmem_swp_alloc */
> enum sgp_type {
> SGP_READ, /* don't exceed i_size, don't allocate page */
> @@ -822,6 +829,7 @@ static int shmem_notify_change(struct de
> static void shmem_evict_inode(struct inode *inode)
> {
> struct shmem_inode_info *info = SHMEM_I(inode);
> + struct shmem_xattr *xattr, *nxattr;
>
> if (inode->i_mapping->a_ops == &shmem_aops) {
> truncate_inode_pages(inode->i_mapping, 0);
> @@ -834,6 +842,11 @@ static void shmem_evict_inode(struct ino
> mutex_unlock(&shmem_swaplist_mutex);
> }
> }
> +
> + list_for_each_entry_safe(xattr, nxattr, &info->xattr_list, list) {
> + kfree(xattr->name);
> + kfree(xattr);
> + }
> BUG_ON(inode->i_blocks);
> shmem_free_inode(inode->i_sb);
> end_writeback(inode);
> @@ -1597,6 +1610,7 @@ static struct inode *shmem_get_inode(str
> spin_lock_init(&info->lock);
> info->flags = flags & VM_NORESERVE;
> INIT_LIST_HEAD(&info->swaplist);
> + INIT_LIST_HEAD(&info->xattr_list);
> cache_no_acl(inode);
>
> switch (mode & S_IFMT) {
> @@ -2048,62 +2062,225 @@ static void shmem_put_link(struct dentry
> }
> }
>
> -static const struct inode_operations shmem_symlink_inline_operations = {
> - .readlink = generic_readlink,
> - .follow_link = shmem_follow_link_inline,
> -};
> -
> -static const struct inode_operations shmem_symlink_inode_operations = {
> - .readlink = generic_readlink,
> - .follow_link = shmem_follow_link,
> - .put_link = shmem_put_link,
> -};
> -
> -#ifdef CONFIG_TMPFS_POSIX_ACL
> +#ifdef CONFIG_TMPFS_XATTR
> /*
> - * Superblocks without xattr inode operations will get security.* xattr
> - * support from the VFS "for free". As soon as we have any other xattrs
> + * Superblocks without xattr inode operations may get some security.* xattr
> + * support from the LSM "for free". As soon as we have any other xattrs
> * like ACLs, we also need to implement the security.* handlers at
> * filesystem level, though.
> */
>
> -static size_t shmem_xattr_security_list(struct dentry *dentry, char *list,
> - size_t list_len, const char *name,
> - size_t name_len, int handler_flags)
> +static int shmem_xattr_get(struct dentry *dentry, const char *name,
> + void *buffer, size_t size)
> {
> - return security_inode_listsecurity(dentry->d_inode, list, list_len);
> -}
> + struct shmem_inode_info *info;
> + struct shmem_xattr *xattr;
> + int ret = -ENODATA;
>
> -static int shmem_xattr_security_get(struct dentry *dentry, const char *name,
> - void *buffer, size_t size, int handler_flags)
> -{
> - if (strcmp(name, "") == 0)
> - return -EINVAL;
> - return xattr_getsecurity(dentry->d_inode, name, buffer, size);
> + info = SHMEM_I(dentry->d_inode);
> +
> + spin_lock(&dentry->d_inode->i_lock);
> + list_for_each_entry(xattr, &info->xattr_list, list) {
> + if (strcmp(name, xattr->name))
> + continue;
> +
> + ret = xattr->size;
> + if (buffer) {
> + if (size < xattr->size)
> + ret = -ERANGE;
> + else
> + memcpy(buffer, xattr->value, xattr->size);
> + }
> + break;
> + }
> + spin_unlock(&dentry->d_inode->i_lock);
> + return ret;
> }
>
> -static int shmem_xattr_security_set(struct dentry *dentry, const char *name,
> - const void *value, size_t size, int flags, int handler_flags)
> +static int shmem_xattr_set(struct dentry *dentry, const char *name,
> + const void *value, size_t size, int flags)
> {
> - if (strcmp(name, "") == 0)
> - return -EINVAL;
> - return security_inode_setsecurity(dentry->d_inode, name, value,
> - size, flags);
> + struct inode *inode = dentry->d_inode;
> + struct shmem_inode_info *info = SHMEM_I(inode);
> + struct shmem_xattr *xattr;
> + struct shmem_xattr *new_xattr = NULL;
> + size_t len;
> + int err = 0;
> +
> + /* value == NULL means remove */
> + if (value) {
> + /* wrap around? */
> + len = sizeof(*new_xattr) + size;
> + if (len <= sizeof(*new_xattr))
> + return -ENOMEM;
> +
> + new_xattr = kmalloc(len, GFP_NOFS);
> + if (!new_xattr)
> + return -ENOMEM;
> +
> + new_xattr->name = kstrdup(name, GFP_NOFS);
> + if (!new_xattr->name) {
> + kfree(new_xattr);
> + return -ENOMEM;
> + }
> +
> + new_xattr->size = size;
> + memcpy(new_xattr->value, value, size);
> + }
> +
> + spin_lock(&inode->i_lock);
> + list_for_each_entry(xattr, &info->xattr_list, list) {
> + if (!strcmp(name, xattr->name)) {
> + if (flags & XATTR_CREATE) {
> + xattr = new_xattr;
> + err = -EEXIST;
> + } else if (new_xattr) {
> + list_replace(&xattr->list, &new_xattr->list);
> + } else {
> + list_del(&xattr->list);
> + }
> + goto out;
> + }
> + }
> + if (flags & XATTR_REPLACE) {
> + xattr = new_xattr;
> + err = -ENODATA;
> + } else {
> + list_add(&new_xattr->list, &info->xattr_list);
> + xattr = NULL;
> + }
> +out:
> + spin_unlock(&inode->i_lock);
> + if (xattr)
> + kfree(xattr->name);
> + kfree(xattr);
> + return 0;
> }
>
> -static const struct xattr_handler shmem_xattr_security_handler = {
> - .prefix = XATTR_SECURITY_PREFIX,
> - .list = shmem_xattr_security_list,
> - .get = shmem_xattr_security_get,
> - .set = shmem_xattr_security_set,
> -};
>
> static const struct xattr_handler *shmem_xattr_handlers[] = {
> +#ifdef CONFIG_TMPFS_POSIX_ACL
> &generic_acl_access_handler,
> &generic_acl_default_handler,
> - &shmem_xattr_security_handler,
> +#endif
> NULL
> };
> +
> +static int shmem_xattr_validate(const char *name)
> +{
> + struct { const char *prefix; size_t len; } arr[] = {
> + { XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN },
> + { XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN }};
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(arr); i++) {
> + size_t preflen = arr[i].len;
> + if (strncmp(name, arr[i].prefix, preflen) == 0) {
> + if (!name[preflen])
> + return -EINVAL;
> + return 0;
> + }
> + }
> + return -EOPNOTSUPP;
> +}
> +
> +static ssize_t shmem_getxattr(struct dentry *dentry, const char *name,
> + void *buffer, size_t size)
> +{
> + int err;
> +
> + /*
> + * If this is a request for a synthetic attribute in the system.*
> + * namespace use the generic infrastructure to resolve a handler
> + * for it via sb->s_xattr.
> + */
> + if (!strncmp(name, XATTR_SYSTEM_PREFIX, XATTR_SYSTEM_PREFIX_LEN))
> + return generic_getxattr(dentry, name, buffer, size);
> +
> + err = shmem_xattr_validate(name);
> + if (err)
> + return err;
> +
> + return shmem_xattr_get(dentry, name, buffer, size);
> +}
> +
> +static int shmem_setxattr(struct dentry *dentry, const char *name,
> + const void *value, size_t size, int flags)
> +{
> + int err;
> +
> + /*
> + * If this is a request for a synthetic attribute in the system.*
> + * namespace use the generic infrastructure to resolve a handler
> + * for it via sb->s_xattr.
> + */
> + if (!strncmp(name, XATTR_SYSTEM_PREFIX, XATTR_SYSTEM_PREFIX_LEN))
> + return generic_setxattr(dentry, name, value, size, flags);
> +
> + err = shmem_xattr_validate(name);
> + if (err)
> + return err;
> +
> + if (size == 0)
> + value = ""; /* empty EA, do not remove */
> +
> + return shmem_xattr_set(dentry, name, value, size, flags);
> +
> +}
> +
> +static int shmem_removexattr(struct dentry *dentry, const char *name)
> +{
> + int err;
> +
> + /*
> + * If this is a request for a synthetic attribute in the system.*
> + * namespace use the generic infrastructure to resolve a handler
> + * for it via sb->s_xattr.
> + */
> + if (!strncmp(name, XATTR_SYSTEM_PREFIX, XATTR_SYSTEM_PREFIX_LEN))
> + return generic_removexattr(dentry, name);
> +
> + err = shmem_xattr_validate(name);
> + if (err)
> + return err;
> +
> + return shmem_xattr_set(dentry, name, NULL, 0, XATTR_REPLACE);
> +}
> +
> +static bool xattr_is_trusted(const char *name)
> +{
> + return !strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN);
> +}
> +
> +static ssize_t shmem_listxattr(struct dentry *dentry, char *buffer, size_t size)
> +{
> + bool trusted = capable(CAP_SYS_ADMIN);
> + struct shmem_xattr *xattr;
> + struct shmem_inode_info *info;
> + size_t used = 0;
> +
> + info = SHMEM_I(dentry->d_inode);
> +
> + spin_lock(&dentry->d_inode->i_lock);
> + list_for_each_entry(xattr, &info->xattr_list, list) {
> + /* skip "trusted." attributes for unprivileged callers */
> + if (!trusted && xattr_is_trusted(xattr->name))
> + continue;
> +
> + used += strlen(xattr->name) + 1;
> + if (buffer) {
> + if (size < used) {
> + used = -ERANGE;
> + break;
> + }
> + strncpy(buffer, xattr->name, strlen(xattr->name) + 1);
> + buffer += strlen(xattr->name) + 1;
> + }
> + }
> + spin_unlock(&dentry->d_inode->i_lock);
> +
> + return used;
> +}
> #endif
>
> static struct dentry *shmem_get_parent(struct dentry *child)
> @@ -2384,8 +2561,10 @@ int shmem_fill_super(struct super_block
> sb->s_magic = TMPFS_MAGIC;
> sb->s_op = &shmem_ops;
> sb->s_time_gran = 1;
> -#ifdef CONFIG_TMPFS_POSIX_ACL
> +#ifdef CONFIG_TMPFS_XATTR
> sb->s_xattr = shmem_xattr_handlers;
> +#endif
> +#ifdef CONFIG_TMPFS_POSIX_ACL
> sb->s_flags |= MS_POSIXACL;
> #endif
>
> @@ -2483,16 +2662,41 @@ static const struct file_operations shme
> static const struct inode_operations shmem_inode_operations = {
> .setattr = shmem_notify_change,
> .truncate_range = shmem_truncate_range,
> +#ifdef CONFIG_TMPFS_XATTR
> + .setxattr = shmem_setxattr,
> + .getxattr = shmem_getxattr,
> + .listxattr = shmem_listxattr,
> + .removexattr = shmem_removexattr,
> +#endif
> #ifdef CONFIG_TMPFS_POSIX_ACL
> - .setxattr = generic_setxattr,
> - .getxattr = generic_getxattr,
> - .listxattr = generic_listxattr,
> - .removexattr = generic_removexattr,
> .check_acl = generic_check_acl,
> #endif
>
> };
>
> +static const struct inode_operations shmem_symlink_inline_operations = {
> + .readlink = generic_readlink,
> + .follow_link = shmem_follow_link_inline,
> +#ifdef CONFIG_TMPFS_XATTR
> + .setxattr = shmem_setxattr,
> + .getxattr = shmem_getxattr,
> + .listxattr = shmem_listxattr,
> + .removexattr = shmem_removexattr,
> +#endif
> +};
> +
> +static const struct inode_operations shmem_symlink_inode_operations = {
> + .readlink = generic_readlink,
> + .follow_link = shmem_follow_link,
> + .put_link = shmem_put_link,
> +#ifdef CONFIG_TMPFS_XATTR
> + .setxattr = shmem_setxattr,
> + .getxattr = shmem_getxattr,
> + .listxattr = shmem_listxattr,
> + .removexattr = shmem_removexattr,
> +#endif
> +};
> +
> static const struct inode_operations shmem_dir_inode_operations = {
> #ifdef CONFIG_TMPFS
> .create = shmem_create,
> @@ -2505,23 +2709,27 @@ static const struct inode_operations shm
> .mknod = shmem_mknod,
> .rename = shmem_rename,
> #endif
> -#ifdef CONFIG_TMPFS_POSIX_ACL
> - .setattr = shmem_notify_change,
> +#ifdef CONFIG_TMPFS_XATTR
> .setxattr = generic_setxattr,
> .getxattr = generic_getxattr,
> .listxattr = generic_listxattr,
> .removexattr = generic_removexattr,
> +#endif
> +#ifdef CONFIG_TMPFS_POSIX_ACL
> + .setattr = shmem_notify_change,
> .check_acl = generic_check_acl,
> #endif
> };
>
> static const struct inode_operations shmem_special_inode_operations = {
> -#ifdef CONFIG_TMPFS_POSIX_ACL
> - .setattr = shmem_notify_change,
> +#ifdef CONFIG_TMPFS_XATTR
> .setxattr = generic_setxattr,
> .getxattr = generic_getxattr,
> .listxattr = generic_listxattr,
> .removexattr = generic_removexattr,
> +#endif
> +#ifdef CONFIG_TMPFS_POSIX_ACL
> + .setattr = shmem_notify_change,
> .check_acl = generic_check_acl,
> #endif
> };
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/


\
 
 \ /
  Last update: 2011-04-20 18:05    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site