lkml.org 
[lkml]   [2012]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] Add security.selinux XATTR support for the UBIFS. Also fix couple of bugs in UBIFS extended attribute length calculation.
    On 4/8/2012 11:40 PM, Subodh Nijsure wrote:
    > On Sun, Apr 8, 2012 at 7:37 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
    >> On 4/8/2012 6:47 AM, Subodh Nijsure wrote:
    >>> TESTING: Tested on MX28 based platforms using Micron MT29F2G08ABAEAH4 NAND
    >>> With these change we are able to label UBIFS filesystem with security.selinux
    >>> and run system with selinux enabled.
    >>> Also ran integck test on UBI filesystem.
    >>>
    >>> Signed-off-by: Subodh Nijsure <snijsure@grid-net.com>
    >> There is no reason to restrict xattr support to SELinux.
    >> SELinux specific behavior belongs within the SELinux LSM.
    >> This code needs to support the full xattr semantics.
    >>
    >> Nacked-by: Casey Schaufler <casey@scahufler-ca.com>
    >>
    >>> ---
    >>> fs/ubifs/dir.c | 4 ++
    >>> fs/ubifs/file.c | 6 ++
    >>> fs/ubifs/journal.c | 12 +++-
    >>> fs/ubifs/super.c | 3 +
    >>> fs/ubifs/ubifs.h | 9 +++
    >>> fs/ubifs/xattr.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++++----
    >>> 6 files changed, 167 insertions(+), 14 deletions(-)
    >>>
    >>> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
    >>> index ec9f187..f4e06c4 100644
    >>> --- a/fs/ubifs/dir.c
    >>> +++ b/fs/ubifs/dir.c
    >>> @@ -293,6 +293,7 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
    >>> ubifs_release_budget(c, &req);
    >>> insert_inode_hash(inode);
    >>> d_instantiate(dentry, inode);
    >>> + ubifs_init_security(dir, inode, &dentry->d_name);
    >>> return 0;
    >>>
    >>> out_cancel:
    >>> @@ -754,6 +755,7 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
    >>>
    >>> ubifs_release_budget(c, &req);
    >>> d_instantiate(dentry, inode);
    >>> + ubifs_init_security(dir, inode, &dentry->d_name);
    >>> return 0;
    >>>
    >>> out_cancel:
    >>> @@ -831,6 +833,7 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry,
    >>> ubifs_release_budget(c, &req);
    >>> insert_inode_hash(inode);
    >>> d_instantiate(dentry, inode);
    >>> + ubifs_init_security(dir, inode, &dentry->d_name);
    >>> return 0;
    >>>
    >>> out_cancel:
    >>> @@ -907,6 +910,7 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry,
    >>> ubifs_release_budget(c, &req);
    >>> insert_inode_hash(inode);
    >>> d_instantiate(dentry, inode);
    >>> + ubifs_init_security(dir, inode, &dentry->d_name);
    >>> return 0;
    >>>
    >>> out_cancel:
    >>> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
    >>> index 5c8f6dc..b8e9d66 100644
    >>> --- a/fs/ubifs/file.c
    >>> +++ b/fs/ubifs/file.c
    >>> @@ -1575,6 +1575,12 @@ const struct inode_operations ubifs_symlink_inode_operations = {
    >>> .follow_link = ubifs_follow_link,
    >>> .setattr = ubifs_setattr,
    >>> .getattr = ubifs_getattr,
    >>> +#ifdef CONFIG_UBIFS_FS_XATTR
    >>> + .setxattr = ubifs_symlink_setxattr,
    >>> + .getxattr = ubifs_symlink_getxattr,
    >>> + .listxattr = ubifs_listxattr,
    >>> + .removexattr = ubifs_removexattr,
    >>> +#endif
    >>> };
    >>>
    >>> const struct file_operations ubifs_file_operations = {
    >>> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
    >>> index 2f438ab..725dc17 100644
    >>> --- a/fs/ubifs/journal.c
    >>> +++ b/fs/ubifs/journal.c
    >>> @@ -553,7 +553,8 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
    >>>
    >>> dbg_jnl("ino %lu, dent '%.*s', data len %d in dir ino %lu",
    >>> inode->i_ino, nm->len, nm->name, ui->data_len, dir->i_ino);
    >>> - ubifs_assert(dir_ui->data_len == 0);
    >>> + if (!xent)
    >>> + ubifs_assert(dir_ui->data_len == 0);
    >>> ubifs_assert(mutex_is_locked(&dir_ui->ui_mutex));
    >>>
    >>> dlen = UBIFS_DENT_NODE_SZ + nm->len + 1;
    >>> @@ -572,7 +573,11 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
    >>>
    >>> aligned_dlen = ALIGN(dlen, 8);
    >>> aligned_ilen = ALIGN(ilen, 8);
    >>> - len = aligned_dlen + aligned_ilen + UBIFS_INO_NODE_SZ;
    >>> + /* Make sure to account for dir_ui+data_len in length calculation
    >>> + * in case there is extended attribute.
    >>> + */
    >>> + len = aligned_dlen + aligned_ilen +
    >>> + UBIFS_INO_NODE_SZ + dir_ui->data_len;
    >>> dent = kmalloc(len, GFP_NOFS);
    >>> if (!dent)
    >>> return -ENOMEM;
    >>> @@ -649,7 +654,8 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
    >>>
    >>> ino_key_init(c, &ino_key, dir->i_ino);
    >>> ino_offs += aligned_ilen;
    >>> - err = ubifs_tnc_add(c, &ino_key, lnum, ino_offs, UBIFS_INO_NODE_SZ);
    >>> + err = ubifs_tnc_add(c, &ino_key, lnum, ino_offs,
    >>> + UBIFS_INO_NODE_SZ + dir_ui->data_len);
    >>> if (err)
    >>> goto out_ro;
    >>>
    >>> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
    >>> index 76e4e05..c28ac04 100644
    >>> --- a/fs/ubifs/super.c
    >>> +++ b/fs/ubifs/super.c
    >>> @@ -2061,6 +2061,9 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
    >>> if (c->max_inode_sz > MAX_LFS_FILESIZE)
    >>> sb->s_maxbytes = c->max_inode_sz = MAX_LFS_FILESIZE;
    >>> sb->s_op = &ubifs_super_operations;
    >>> +#ifdef CONFIG_UBIFS_FS_XATTR
    >>> + sb->s_xattr = ubifs_xattr_handlers;
    >>> +#endif
    >>>
    >>> mutex_lock(&c->umount_mutex);
    >>> err = mount_ubifs(c);
    >>> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
    >>> index 93d59ac..60b57f7 100644
    >>> --- a/fs/ubifs/ubifs.h
    >>> +++ b/fs/ubifs/ubifs.h
    >>> @@ -36,6 +36,7 @@
    >>> #include <linux/mtd/ubi.h>
    >>> #include <linux/pagemap.h>
    >>> #include <linux/backing-dev.h>
    >>> +#include <linux/security.h>
    >>> #include "ubifs-media.h"
    >>>
    >>> /* Version of this UBIFS implementation */
    >>> @@ -1454,6 +1455,7 @@ extern spinlock_t ubifs_infos_lock;
    >>> extern atomic_long_t ubifs_clean_zn_cnt;
    >>> extern struct kmem_cache *ubifs_inode_slab;
    >>> extern const struct super_operations ubifs_super_operations;
    >>> +extern const struct xattr_handler *ubifs_xattr_handlers[];
    >>> extern const struct address_space_operations ubifs_file_address_operations;
    >>> extern const struct file_operations ubifs_file_operations;
    >>> extern const struct inode_operations ubifs_file_inode_operations;
    >>> @@ -1736,10 +1738,17 @@ int ubifs_getattr(struct vfsmount *mnt, struct dentry *dentry,
    >>> struct kstat *stat);
    >>>
    >>> /* xattr.c */
    >>> +
    >>> int ubifs_setxattr(struct dentry *dentry, const char *name,
    >>> const void *value, size_t size, int flags);
    >>> +int ubifs_init_security(struct inode *dentry, struct inode *inode,
    >>> + const struct qstr *qstr);
    >>> +int ubifs_symlink_setxattr(struct dentry *dentry, const char *name,
    >>> + const void *value, size_t size, int flags);
    >>> ssize_t ubifs_getxattr(struct dentry *dentry, const char *name, void *buf,
    >>> size_t size);
    >>> +ssize_t ubifs_symlink_getxattr(struct dentry *dentry, const char *name,
    >>> + void *buf, size_t size);
    >>> ssize_t ubifs_listxattr(struct dentry *dentry, char *buffer, size_t size);
    >>> int ubifs_removexattr(struct dentry *dentry, const char *name);
    >>>
    >>> diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
    >>> index 85b2722..a402c7d 100644
    >>> --- a/fs/ubifs/xattr.c
    >>> +++ b/fs/ubifs/xattr.c
    >>> @@ -209,11 +209,11 @@ static int change_xattr(struct ubifs_info *c, struct inode *host,
    >>> goto out_free;
    >>> }
    >>> inode->i_size = ui->ui_size = size;
    >>> - ui->data_len = size;
    >>>
    >>> mutex_lock(&host_ui->ui_mutex);
    >>> host->i_ctime = ubifs_current_time(host);
    >>> host_ui->xattr_size -= CALC_XATTR_BYTES(ui->data_len);
    >>> + ui->data_len = size;
    >>> host_ui->xattr_size += CALC_XATTR_BYTES(size);
    >>>
    >>> /*
    >>> @@ -293,18 +293,16 @@ static struct inode *iget_xattr(struct ubifs_info *c, ino_t inum)
    >>> return ERR_PTR(-EINVAL);
    >>> }
    >>>
    >>> -int ubifs_setxattr(struct dentry *dentry, const char *name,
    >>> - const void *value, size_t size, int flags)
    >>> +static int __ubifs_setxattr(struct inode *host, const char *name,
    >>> + const void *value, size_t size, int flags)
    >>> {
    >>> - struct inode *inode, *host = dentry->d_inode;
    >>> + struct inode *inode;
    >>> struct ubifs_info *c = host->i_sb->s_fs_info;
    >>> struct qstr nm = { .name = name, .len = strlen(name) };
    >>> struct ubifs_dent_node *xent;
    >>> union ubifs_key key;
    >>> int err, type;
    >>>
    >>> - dbg_gen("xattr '%s', host ino %lu ('%.*s'), size %zd", name,
    >>> - host->i_ino, dentry->d_name.len, dentry->d_name.name, size);
    >>> ubifs_assert(mutex_is_locked(&host->i_mutex));
    >>>
    >>> if (size > UBIFS_MAX_INO_DATA)
    >>> @@ -356,10 +354,29 @@ out_free:
    >>> return err;
    >>> }
    >>>
    >>> -ssize_t ubifs_getxattr(struct dentry *dentry, const char *name, void *buf,
    >>> - size_t size)
    >>> +int ubifs_symlink_setxattr(struct dentry *dentry, const char *name,
    >>> + const void *value, size_t size, int flags)
    >>> {
    >>> - struct inode *inode, *host = dentry->d_inode;
    >>> + struct inode *host = dentry->d_inode;
    >>> + dbg_gen("xattr '%s', host ino %lu ('%.*s'), size %zd", name,
    >>> + host->i_ino, dentry->d_name.len, dentry->d_name.name, size);
    >>> + return __ubifs_setxattr(host, name, value, size, flags);
    >>> +}
    >>> +
    >>> +int ubifs_setxattr(struct dentry *dentry, const char *name,
    >>> + const void *value, size_t size, int flags)
    >>> +{
    >>> + struct inode *host = dentry->d_inode;
    >>> + dbg_gen("xattr '%s', host ino %lu ('%.*s'), size %zd", name,
    >>> + host->i_ino, dentry->d_name.len, dentry->d_name.name, size);
    >>> + return __ubifs_setxattr(host, name, value, size, flags);
    >>> +}
    >>> +
    >>> +static
    >>> +ssize_t __ubifs_getxattr(struct inode *host, const char *name, void *buf,
    >>> + size_t size)
    >>> +{
    >>> + struct inode *inode;
    >>> struct ubifs_info *c = host->i_sb->s_fs_info;
    >>> struct qstr nm = { .name = name, .len = strlen(name) };
    >>> struct ubifs_inode *ui;
    >>> @@ -367,8 +384,8 @@ ssize_t ubifs_getxattr(struct dentry *dentry, const char *name, void *buf,
    >>> union ubifs_key key;
    >>> int err;
    >>>
    >>> - dbg_gen("xattr '%s', ino %lu ('%.*s'), buf size %zd", name,
    >>> - host->i_ino, dentry->d_name.len, dentry->d_name.name, size);
    >>> + dbg_gen("xattr '%s', ino %lu buf size %zd", name,
    >>> + host->i_ino, size);
    >>>
    >>> err = check_namespace(&nm);
    >>> if (err < 0)
    >>> @@ -416,6 +433,25 @@ out_unlock:
    >>> return err;
    >>> }
    >>>
    >>> +ssize_t ubifs_symlink_getxattr(struct dentry *dentry, const char *name,
    >>> + void *buf, size_t size)
    >>> +{
    >>> + struct inode *host = dentry->d_inode;
    >>> + dbg_gen("xattr '%s', ino %lu ('%.*s'), buf size %zd", name,
    >>> + host->i_ino, dentry->d_name.len, dentry->d_name.name, size);
    >>> + return __ubifs_getxattr(host, name, buf, size);
    >>> +}
    >>> +
    >>> +ssize_t ubifs_getxattr(struct dentry *dentry, const char *name, void *buf,
    >>> + size_t size)
    >>> +{
    >>> + struct inode *host = dentry->d_inode;
    >>> + dbg_gen("xattr '%s', ino %lu ('%.*s'), buf size %zd", name,
    >>> + host->i_ino, dentry->d_name.len, dentry->d_name.name, size);
    >>> + return __ubifs_getxattr(host, name, buf, size);
    >>> +}
    >>> +
    >>> +
    >>> ssize_t ubifs_listxattr(struct dentry *dentry, char *buffer, size_t size)
    >>> {
    >>> union ubifs_key key;
    >>> @@ -568,3 +604,92 @@ out_free:
    >>> kfree(xent);
    >>> return err;
    >>> }
    >>> +
    >>> +size_t
    >>> +ubifs_security_listxattr(struct dentry *d, char *list, size_t list_size,
    >>> + const char *name, size_t name_len, int flags)
    >>> +{
    >>> + const int prefix_len = XATTR_SECURITY_PREFIX_LEN;
    >>> + const size_t total_len = prefix_len + name_len + 1;
    >>> + if (list && total_len <= list_size) {
    >>> + memcpy(list, XATTR_SECURITY_PREFIX, prefix_len);
    >>> + memcpy(list+prefix_len, name, name_len);
    >>> + list[prefix_len + name_len] = '\0';
    >>> + }
    >>> + return total_len;
    >>> +}
    >>> +
    >>> +
    >>> +int ubifs_security_getxattr(struct dentry *d, const char *name,
    >>> + void *buffer, size_t size, int flags)
    >>> +{
    >>> + if (strcmp(name, "") == 0)
    >>> + return -EINVAL;
    >>> + return __ubifs_getxattr(d->d_inode, XATTR_NAME_SELINUX, buffer, size);
    >>> +}
    >>> +
    >>> +
    >>> +int ubifs_security_setxattr(struct dentry *d, const char *name,
    >>> + const void *value, size_t size,
    >>> + int flags, int handler_flags)
    >>> +{
    >>> + if (strcmp(name, "") == 0)
    >>> + return -EINVAL;
    >>> + return __ubifs_setxattr(d->d_inode, XATTR_NAME_SELINUX, value,
    >> This is silly. If you're supporting xattrs there is no reason
    >> single out a particular LSM. Make this general so that Smack
    >> and any other LSM that wants to use xattrs will work.
    >>
    >> Move the changes into the SELinux LSM if you are going to
    >> hard code it.
    >>
    > Yup, my bad, following change fixes that specific issue and one can
    > set xattr for all security.* namespace. Will submit patch v2
    > shortly...

    Thank you. The update looks fine, although I haven't tried it.

    >
    > diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
    > index a402c7d..c8be7cd 100644
    > --- a/fs/ubifs/xattr.c
    > +++ b/fs/ubifs/xattr.c
    > @@ -625,7 +625,7 @@ int ubifs_security_getxattr(struct dentry *d,
    > const char *name,
    > {
    > if (strcmp(name, "") == 0)
    > return -EINVAL;
    > - return __ubifs_getxattr(d->d_inode, XATTR_NAME_SELINUX, buffer, size);
    > + return __ubifs_getxattr(d->d_inode, name, buffer, size);
    > }
    >
    >
    > @@ -635,7 +635,7 @@ int ubifs_security_setxattr(struct dentry *d,
    > const char *name,
    > {
    > if (strcmp(name, "") == 0)
    > return -EINVAL;
    > - return __ubifs_setxattr(d->d_inode, XATTR_NAME_SELINUX, value,
    > + return __ubifs_setxattr(d->d_inode, name, value,
    > size, flags);
    > }
    >



    \
     
     \ /
      Last update: 2012-04-09 17:17    [W:0.075 / U:1.004 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site