| Message ID | CALr9Q3b=U8jRZv_rEXX0VmeC1f_wgjXOEtZA=bkxfmba+a1YnA@mail.gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
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); > } >
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); }