* [PATCH v3] Add security.* XATTR support for the UBIFS
@ 2012-04-11 21:28 ` subodh.nijsure
0 siblings, 0 replies; 19+ messages in thread
From: subodh.nijsure @ 2012-04-11 21:28 UTC (permalink / raw)
To: linux-mtd, Artem Bityutskiy
Cc: Adrian Hunter, linux-kernel, linux-security-module, Subodh Nijsure
From: Subodh Nijsure <snijsure@grid-net.com>
Also fix couple of bugs in UBIFS extended attribute length calculation.
Changes in v3:
Invoke ubifs_init_security only if CONFIG_UBIFS_FS_XATTR is defined.
Invoke it before d_instantiate and check ubifs_init_security return
code.
Changes in v2:
Instead of just handling security.selinux extended attribute handle
all security.* attributes.
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.
This change also allows one to set other security.* extended
attributes, such as security.smack security.evm, security.ima
Ran integck test on UBI filesystem.
Signed-off-by: Subodh Nijsure <snijsure@grid-net.com>
---
fs/ubifs/dir.c | 47 +++++++++++++++++
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, 210 insertions(+), 14 deletions(-)
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index ec9f187..422bcec 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -292,6 +292,18 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
+
+#ifdef CONFIG_UBIFS_FS_XATTR
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
+#endif
+
d_instantiate(dentry, inode);
return 0;
@@ -753,6 +765,17 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
mutex_unlock(&dir_ui->ui_mutex);
ubifs_release_budget(c, &req);
+
+#ifdef CONFIG_UBIFS_FS_XATTR
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
+#endif
d_instantiate(dentry, inode);
return 0;
@@ -830,6 +853,18 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry,
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
+
+#ifdef CONFIG_UBIFS_FS_XATTR
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
+#endif
+
d_instantiate(dentry, inode);
return 0;
@@ -906,6 +941,18 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry,
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
+
+#ifdef CONFIG_UBIFS_FS_XATTR
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
+#endif
+
d_instantiate(dentry, inode);
return 0;
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..c8be7cd 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, name, 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, name, value,
+ size, flags);
+}
+
+struct xattr_handler ubifs_xattr_security_handler = {
+ .prefix = XATTR_SECURITY_PREFIX,
+ .list = ubifs_security_listxattr,
+ .get = ubifs_security_getxattr,
+ .set = ubifs_security_setxattr,
+};
+
+const struct xattr_handler *ubifs_xattr_handlers[] = {
+ &ubifs_xattr_security_handler,
+ NULL
+};
+
+static int ubifs_initxattrs(struct inode *inode,
+ const struct xattr *xattr_array, void *fs_info)
+{
+ const struct xattr *xattr;
+ char *name;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
+ strlen(xattr->name) + 1, GFP_NOFS);
+ if (!name) {
+ err = -ENOMEM;
+ break;
+ }
+ strcpy(name, XATTR_SECURITY_PREFIX);
+ strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
+
+ err = __ubifs_setxattr(inode, xattr->name, xattr->value,
+ xattr->value_len, 0);
+ kfree(name);
+ if (err < 0)
+ break;
+ }
+ return err;
+}
+
+int
+ubifs_init_security(struct inode *dentry, struct inode *inode,
+ const struct qstr *qstr)
+{
+ struct ubifs_inode *dir_ui = ubifs_inode(inode);
+ int err = 0;
+
+ mutex_lock(&dir_ui->ui_mutex);
+ mutex_lock(&inode->i_mutex);
+
+ err = security_inode_init_security(inode, dentry, qstr,
+ &ubifs_initxattrs, 0);
+ mutex_unlock(&inode->i_mutex);
+ mutex_unlock(&dir_ui->ui_mutex);
+ return err;
+}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3] Add security.* XATTR support for the UBIFS
@ 2012-04-11 21:28 ` subodh.nijsure
0 siblings, 0 replies; 19+ messages in thread
From: subodh.nijsure @ 2012-04-11 21:28 UTC (permalink / raw)
To: linux-mtd, Artem Bityutskiy
Cc: linux-security-module, Subodh Nijsure, Adrian Hunter, linux-kernel
From: Subodh Nijsure <snijsure@grid-net.com>
Also fix couple of bugs in UBIFS extended attribute length calculation.
Changes in v3:
Invoke ubifs_init_security only if CONFIG_UBIFS_FS_XATTR is defined.
Invoke it before d_instantiate and check ubifs_init_security return
code.
Changes in v2:
Instead of just handling security.selinux extended attribute handle
all security.* attributes.
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.
This change also allows one to set other security.* extended
attributes, such as security.smack security.evm, security.ima
Ran integck test on UBI filesystem.
Signed-off-by: Subodh Nijsure <snijsure@grid-net.com>
---
fs/ubifs/dir.c | 47 +++++++++++++++++
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, 210 insertions(+), 14 deletions(-)
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index ec9f187..422bcec 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -292,6 +292,18 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
+
+#ifdef CONFIG_UBIFS_FS_XATTR
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
+#endif
+
d_instantiate(dentry, inode);
return 0;
@@ -753,6 +765,17 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
mutex_unlock(&dir_ui->ui_mutex);
ubifs_release_budget(c, &req);
+
+#ifdef CONFIG_UBIFS_FS_XATTR
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
+#endif
d_instantiate(dentry, inode);
return 0;
@@ -830,6 +853,18 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry,
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
+
+#ifdef CONFIG_UBIFS_FS_XATTR
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
+#endif
+
d_instantiate(dentry, inode);
return 0;
@@ -906,6 +941,18 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry,
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
+
+#ifdef CONFIG_UBIFS_FS_XATTR
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
+#endif
+
d_instantiate(dentry, inode);
return 0;
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..c8be7cd 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, name, 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, name, value,
+ size, flags);
+}
+
+struct xattr_handler ubifs_xattr_security_handler = {
+ .prefix = XATTR_SECURITY_PREFIX,
+ .list = ubifs_security_listxattr,
+ .get = ubifs_security_getxattr,
+ .set = ubifs_security_setxattr,
+};
+
+const struct xattr_handler *ubifs_xattr_handlers[] = {
+ &ubifs_xattr_security_handler,
+ NULL
+};
+
+static int ubifs_initxattrs(struct inode *inode,
+ const struct xattr *xattr_array, void *fs_info)
+{
+ const struct xattr *xattr;
+ char *name;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
+ strlen(xattr->name) + 1, GFP_NOFS);
+ if (!name) {
+ err = -ENOMEM;
+ break;
+ }
+ strcpy(name, XATTR_SECURITY_PREFIX);
+ strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
+
+ err = __ubifs_setxattr(inode, xattr->name, xattr->value,
+ xattr->value_len, 0);
+ kfree(name);
+ if (err < 0)
+ break;
+ }
+ return err;
+}
+
+int
+ubifs_init_security(struct inode *dentry, struct inode *inode,
+ const struct qstr *qstr)
+{
+ struct ubifs_inode *dir_ui = ubifs_inode(inode);
+ int err = 0;
+
+ mutex_lock(&dir_ui->ui_mutex);
+ mutex_lock(&inode->i_mutex);
+
+ err = security_inode_init_security(inode, dentry, qstr,
+ &ubifs_initxattrs, 0);
+ mutex_unlock(&inode->i_mutex);
+ mutex_unlock(&dir_ui->ui_mutex);
+ return err;
+}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
2012-04-11 21:28 ` subodh.nijsure
@ 2012-04-11 23:27 ` Mimi Zohar
-1 siblings, 0 replies; 19+ messages in thread
From: Mimi Zohar @ 2012-04-11 23:27 UTC (permalink / raw)
To: subodh.nijsure
Cc: linux-mtd, Artem Bityutskiy, Adrian Hunter, linux-kernel,
linux-security-module, Subodh Nijsure
On Wed, 2012-04-11 at 14:28 -0700, subodh.nijsure@gmail.com wrote:
> From: Subodh Nijsure <snijsure@grid-net.com>
>
> Also fix couple of bugs in UBIFS extended attribute length calculation.
>
> Changes in v3:
> Invoke ubifs_init_security only if CONFIG_UBIFS_FS_XATTR is defined.
> Invoke it before d_instantiate and check ubifs_init_security return
> code.
> Changes in v2:
> Instead of just handling security.selinux extended attribute handle
> all security.* attributes.
>
> 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.
> This change also allows one to set other security.* extended
> attributes, such as security.smack security.evm, security.ima
> Ran integck test on UBI filesystem.
>
> Signed-off-by: Subodh Nijsure <snijsure@grid-net.com>
> ---
> fs/ubifs/dir.c | 47 +++++++++++++++++
> 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, 210 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index ec9f187..422bcec 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -292,6 +292,18 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
>
> ubifs_release_budget(c, &req);
> insert_inode_hash(inode);
> +
> +#ifdef CONFIG_UBIFS_FS_XATTR
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> +#endif
> +
The preferred method for including code based on CONFIG options is to
define a stub function in an include file. #ifdef's in C code is
frowned upon (refer to section 2.2 of Documentation/SubmittingPatches).
thanks,
Mimi
> d_instantiate(dentry, inode);
> return 0;
>
> @@ -753,6 +765,17 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
> mutex_unlock(&dir_ui->ui_mutex);
>
> ubifs_release_budget(c, &req);
> +
> +#ifdef CONFIG_UBIFS_FS_XATTR
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> +#endif
> d_instantiate(dentry, inode);
> return 0;
>
> @@ -830,6 +853,18 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry,
>
> ubifs_release_budget(c, &req);
> insert_inode_hash(inode);
> +
> +#ifdef CONFIG_UBIFS_FS_XATTR
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> +#endif
> +
> d_instantiate(dentry, inode);
> return 0;
>
> @@ -906,6 +941,18 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry,
>
> ubifs_release_budget(c, &req);
> insert_inode_hash(inode);
> +
> +#ifdef CONFIG_UBIFS_FS_XATTR
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> +#endif
> +
> d_instantiate(dentry, inode);
> return 0;
>
> 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..c8be7cd 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, name, 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, name, value,
> + size, flags);
> +}
> +
> +struct xattr_handler ubifs_xattr_security_handler = {
> + .prefix = XATTR_SECURITY_PREFIX,
> + .list = ubifs_security_listxattr,
> + .get = ubifs_security_getxattr,
> + .set = ubifs_security_setxattr,
> +};
> +
> +const struct xattr_handler *ubifs_xattr_handlers[] = {
> + &ubifs_xattr_security_handler,
> + NULL
> +};
> +
> +static int ubifs_initxattrs(struct inode *inode,
> + const struct xattr *xattr_array, void *fs_info)
> +{
> + const struct xattr *xattr;
> + char *name;
> + int err = 0;
> +
> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
> + name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
> + strlen(xattr->name) + 1, GFP_NOFS);
> + if (!name) {
> + err = -ENOMEM;
> + break;
> + }
> + strcpy(name, XATTR_SECURITY_PREFIX);
> + strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
> +
> + err = __ubifs_setxattr(inode, xattr->name, xattr->value,
> + xattr->value_len, 0);
> + kfree(name);
> + if (err < 0)
> + break;
> + }
> + return err;
> +}
> +
> +int
> +ubifs_init_security(struct inode *dentry, struct inode *inode,
> + const struct qstr *qstr)
> +{
> + struct ubifs_inode *dir_ui = ubifs_inode(inode);
> + int err = 0;
> +
> + mutex_lock(&dir_ui->ui_mutex);
> + mutex_lock(&inode->i_mutex);
> +
> + err = security_inode_init_security(inode, dentry, qstr,
> + &ubifs_initxattrs, 0);
> + mutex_unlock(&inode->i_mutex);
> + mutex_unlock(&dir_ui->ui_mutex);
> + return err;
> +}
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
@ 2012-04-11 23:27 ` Mimi Zohar
0 siblings, 0 replies; 19+ messages in thread
From: Mimi Zohar @ 2012-04-11 23:27 UTC (permalink / raw)
To: subodh.nijsure
Cc: Subodh Nijsure, Artem Bityutskiy, linux-kernel, Adrian Hunter,
linux-security-module, linux-mtd
On Wed, 2012-04-11 at 14:28 -0700, subodh.nijsure@gmail.com wrote:
> From: Subodh Nijsure <snijsure@grid-net.com>
>
> Also fix couple of bugs in UBIFS extended attribute length calculation.
>
> Changes in v3:
> Invoke ubifs_init_security only if CONFIG_UBIFS_FS_XATTR is defined.
> Invoke it before d_instantiate and check ubifs_init_security return
> code.
> Changes in v2:
> Instead of just handling security.selinux extended attribute handle
> all security.* attributes.
>
> 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.
> This change also allows one to set other security.* extended
> attributes, such as security.smack security.evm, security.ima
> Ran integck test on UBI filesystem.
>
> Signed-off-by: Subodh Nijsure <snijsure@grid-net.com>
> ---
> fs/ubifs/dir.c | 47 +++++++++++++++++
> 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, 210 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index ec9f187..422bcec 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -292,6 +292,18 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
>
> ubifs_release_budget(c, &req);
> insert_inode_hash(inode);
> +
> +#ifdef CONFIG_UBIFS_FS_XATTR
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> +#endif
> +
The preferred method for including code based on CONFIG options is to
define a stub function in an include file. #ifdef's in C code is
frowned upon (refer to section 2.2 of Documentation/SubmittingPatches).
thanks,
Mimi
> d_instantiate(dentry, inode);
> return 0;
>
> @@ -753,6 +765,17 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
> mutex_unlock(&dir_ui->ui_mutex);
>
> ubifs_release_budget(c, &req);
> +
> +#ifdef CONFIG_UBIFS_FS_XATTR
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> +#endif
> d_instantiate(dentry, inode);
> return 0;
>
> @@ -830,6 +853,18 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry,
>
> ubifs_release_budget(c, &req);
> insert_inode_hash(inode);
> +
> +#ifdef CONFIG_UBIFS_FS_XATTR
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> +#endif
> +
> d_instantiate(dentry, inode);
> return 0;
>
> @@ -906,6 +941,18 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry,
>
> ubifs_release_budget(c, &req);
> insert_inode_hash(inode);
> +
> +#ifdef CONFIG_UBIFS_FS_XATTR
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> +#endif
> +
> d_instantiate(dentry, inode);
> return 0;
>
> 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..c8be7cd 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, name, 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, name, value,
> + size, flags);
> +}
> +
> +struct xattr_handler ubifs_xattr_security_handler = {
> + .prefix = XATTR_SECURITY_PREFIX,
> + .list = ubifs_security_listxattr,
> + .get = ubifs_security_getxattr,
> + .set = ubifs_security_setxattr,
> +};
> +
> +const struct xattr_handler *ubifs_xattr_handlers[] = {
> + &ubifs_xattr_security_handler,
> + NULL
> +};
> +
> +static int ubifs_initxattrs(struct inode *inode,
> + const struct xattr *xattr_array, void *fs_info)
> +{
> + const struct xattr *xattr;
> + char *name;
> + int err = 0;
> +
> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
> + name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
> + strlen(xattr->name) + 1, GFP_NOFS);
> + if (!name) {
> + err = -ENOMEM;
> + break;
> + }
> + strcpy(name, XATTR_SECURITY_PREFIX);
> + strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
> +
> + err = __ubifs_setxattr(inode, xattr->name, xattr->value,
> + xattr->value_len, 0);
> + kfree(name);
> + if (err < 0)
> + break;
> + }
> + return err;
> +}
> +
> +int
> +ubifs_init_security(struct inode *dentry, struct inode *inode,
> + const struct qstr *qstr)
> +{
> + struct ubifs_inode *dir_ui = ubifs_inode(inode);
> + int err = 0;
> +
> + mutex_lock(&dir_ui->ui_mutex);
> + mutex_lock(&inode->i_mutex);
> +
> + err = security_inode_init_security(inode, dentry, qstr,
> + &ubifs_initxattrs, 0);
> + mutex_unlock(&inode->i_mutex);
> + mutex_unlock(&dir_ui->ui_mutex);
> + return err;
> +}
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
2012-04-11 23:27 ` Mimi Zohar
@ 2012-04-11 23:45 ` Subodh Nijsure
-1 siblings, 0 replies; 19+ messages in thread
From: Subodh Nijsure @ 2012-04-11 23:45 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-mtd, Artem Bityutskiy, Adrian Hunter, linux-kernel,
linux-security-module, Subodh Nijsure
On Wed, Apr 11, 2012 at 4:27 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2012-04-11 at 14:28 -0700, subodh.nijsure@gmail.com wrote:
>> From: Subodh Nijsure <snijsure@grid-net.com>
>>
>> Also fix couple of bugs in UBIFS extended attribute length calculation.
>>
>> Changes in v3:
>> Invoke ubifs_init_security only if CONFIG_UBIFS_FS_XATTR is defined.
>> Invoke it before d_instantiate and check ubifs_init_security return
>> code.
>
>> Changes in v2:
>> Instead of just handling security.selinux extended attribute handle
>> all security.* attributes.
>>
>> 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.
>> This change also allows one to set other security.* extended
>> attributes, such as security.smack security.evm, security.ima
>> Ran integck test on UBI filesystem.
>>
>> Signed-off-by: Subodh Nijsure <snijsure@grid-net.com>
>> ---
>> fs/ubifs/dir.c | 47 +++++++++++++++++
>> 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, 210 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
>> index ec9f187..422bcec 100644
>> --- a/fs/ubifs/dir.c
>> +++ b/fs/ubifs/dir.c
>> @@ -292,6 +292,18 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
>>
>> ubifs_release_budget(c, &req);
>> insert_inode_hash(inode);
>> +
>> +#ifdef CONFIG_UBIFS_FS_XATTR
>> +
>> + err = ubifs_init_security(dir, inode, &dentry->d_name);
>> + if (err) {
>> + ubifs_err("cannot initialize extended attribute, error %d",
>> + err);
>> + goto out_cancel;
>> + }
>> +
>> +#endif
>> +
>
> The preferred method for including code based on CONFIG options is to
> define a stub function in an include file. #ifdef's in C code is
> frowned upon (refer to section 2.2 of Documentation/SubmittingPatches).
>
> thanks,
>
> Mimi
I can certainly change the code to follow that guideline, sorry I missed it...
There is other code in ubifs that uses #ifdef
CONFIG_UBIFS_FS_XATTR/#endif should I be changing that or expectation
is not to introduce new #ifdef/#endif? And if its the prior should
that change come in as separate patch set?
BTW, would very much appreciate if someone can actually test this
patch with UBIFS on their hardware. I have posted a patch to
mtd-utils/integck test program on linux-mtd mailing list.
Regards,
-Subodh
>
>> d_instantiate(dentry, inode);
>> return 0;
>>
>> @@ -753,6 +765,17 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
>> mutex_unlock(&dir_ui->ui_mutex);
>>
>> ubifs_release_budget(c, &req);
>> +
>> +#ifdef CONFIG_UBIFS_FS_XATTR
>> +
>> + err = ubifs_init_security(dir, inode, &dentry->d_name);
>> + if (err) {
>> + ubifs_err("cannot initialize extended attribute, error %d",
>> + err);
>> + goto out_cancel;
>> + }
>> +
>> +#endif
>> d_instantiate(dentry, inode);
>> return 0;
>>
>> @@ -830,6 +853,18 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry,
>>
>> ubifs_release_budget(c, &req);
>> insert_inode_hash(inode);
>> +
>> +#ifdef CONFIG_UBIFS_FS_XATTR
>> +
>> + err = ubifs_init_security(dir, inode, &dentry->d_name);
>> + if (err) {
>> + ubifs_err("cannot initialize extended attribute, error %d",
>> + err);
>> + goto out_cancel;
>> + }
>> +
>> +#endif
>> +
>> d_instantiate(dentry, inode);
>> return 0;
>>
>> @@ -906,6 +941,18 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry,
>>
>> ubifs_release_budget(c, &req);
>> insert_inode_hash(inode);
>> +
>> +#ifdef CONFIG_UBIFS_FS_XATTR
>> +
>> + err = ubifs_init_security(dir, inode, &dentry->d_name);
>> + if (err) {
>> + ubifs_err("cannot initialize extended attribute, error %d",
>> + err);
>> + goto out_cancel;
>> + }
>> +
>> +#endif
>> +
>> d_instantiate(dentry, inode);
>> return 0;
>>
>> 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..c8be7cd 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, name, 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, name, value,
>> + size, flags);
>> +}
>> +
>> +struct xattr_handler ubifs_xattr_security_handler = {
>> + .prefix = XATTR_SECURITY_PREFIX,
>> + .list = ubifs_security_listxattr,
>> + .get = ubifs_security_getxattr,
>> + .set = ubifs_security_setxattr,
>> +};
>> +
>> +const struct xattr_handler *ubifs_xattr_handlers[] = {
>> + &ubifs_xattr_security_handler,
>> + NULL
>> +};
>> +
>> +static int ubifs_initxattrs(struct inode *inode,
>> + const struct xattr *xattr_array, void *fs_info)
>> +{
>> + const struct xattr *xattr;
>> + char *name;
>> + int err = 0;
>> +
>> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
>> + name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
>> + strlen(xattr->name) + 1, GFP_NOFS);
>> + if (!name) {
>> + err = -ENOMEM;
>> + break;
>> + }
>> + strcpy(name, XATTR_SECURITY_PREFIX);
>> + strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
>> +
>> + err = __ubifs_setxattr(inode, xattr->name, xattr->value,
>> + xattr->value_len, 0);
>> + kfree(name);
>> + if (err < 0)
>> + break;
>> + }
>> + return err;
>> +}
>> +
>> +int
>> +ubifs_init_security(struct inode *dentry, struct inode *inode,
>> + const struct qstr *qstr)
>> +{
>> + struct ubifs_inode *dir_ui = ubifs_inode(inode);
>> + int err = 0;
>> +
>> + mutex_lock(&dir_ui->ui_mutex);
>> + mutex_lock(&inode->i_mutex);
>> +
>> + err = security_inode_init_security(inode, dentry, qstr,
>> + &ubifs_initxattrs, 0);
>> + mutex_unlock(&inode->i_mutex);
>> + mutex_unlock(&dir_ui->ui_mutex);
>> + return err;
>> +}
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
@ 2012-04-11 23:45 ` Subodh Nijsure
0 siblings, 0 replies; 19+ messages in thread
From: Subodh Nijsure @ 2012-04-11 23:45 UTC (permalink / raw)
To: Mimi Zohar
Cc: Subodh Nijsure, Artem Bityutskiy, linux-kernel, Adrian Hunter,
linux-security-module, linux-mtd
On Wed, Apr 11, 2012 at 4:27 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2012-04-11 at 14:28 -0700, subodh.nijsure@gmail.com wrote:
>> From: Subodh Nijsure <snijsure@grid-net.com>
>>
>> Also fix couple of bugs in UBIFS extended attribute length calculation.
>>
>> Changes in v3:
>> Invoke ubifs_init_security only if CONFIG_UBIFS_FS_XATTR is defined.
>> Invoke it before d_instantiate and check ubifs_init_security return
>> code.
>
>> Changes in v2:
>> Instead of just handling security.selinux extended attribute handle
>> all security.* attributes.
>>
>> 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.
>> This change also allows one to set other security.* extended
>> attributes, such as security.smack security.evm, security.ima
>> Ran integck test on UBI filesystem.
>>
>> Signed-off-by: Subodh Nijsure <snijsure@grid-net.com>
>> ---
>> fs/ubifs/dir.c | 47 +++++++++++++++++
>> 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, 210 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
>> index ec9f187..422bcec 100644
>> --- a/fs/ubifs/dir.c
>> +++ b/fs/ubifs/dir.c
>> @@ -292,6 +292,18 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
>>
>> ubifs_release_budget(c, &req);
>> insert_inode_hash(inode);
>> +
>> +#ifdef CONFIG_UBIFS_FS_XATTR
>> +
>> + err = ubifs_init_security(dir, inode, &dentry->d_name);
>> + if (err) {
>> + ubifs_err("cannot initialize extended attribute, error %d",
>> + err);
>> + goto out_cancel;
>> + }
>> +
>> +#endif
>> +
>
> The preferred method for including code based on CONFIG options is to
> define a stub function in an include file. #ifdef's in C code is
> frowned upon (refer to section 2.2 of Documentation/SubmittingPatches).
>
> thanks,
>
> Mimi
I can certainly change the code to follow that guideline, sorry I missed it...
There is other code in ubifs that uses #ifdef
CONFIG_UBIFS_FS_XATTR/#endif should I be changing that or expectation
is not to introduce new #ifdef/#endif? And if its the prior should
that change come in as separate patch set?
BTW, would very much appreciate if someone can actually test this
patch with UBIFS on their hardware. I have posted a patch to
mtd-utils/integck test program on linux-mtd mailing list.
Regards,
-Subodh
>
>> d_instantiate(dentry, inode);
>> return 0;
>>
>> @@ -753,6 +765,17 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
>> mutex_unlock(&dir_ui->ui_mutex);
>>
>> ubifs_release_budget(c, &req);
>> +
>> +#ifdef CONFIG_UBIFS_FS_XATTR
>> +
>> + err = ubifs_init_security(dir, inode, &dentry->d_name);
>> + if (err) {
>> + ubifs_err("cannot initialize extended attribute, error %d",
>> + err);
>> + goto out_cancel;
>> + }
>> +
>> +#endif
>> d_instantiate(dentry, inode);
>> return 0;
>>
>> @@ -830,6 +853,18 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry,
>>
>> ubifs_release_budget(c, &req);
>> insert_inode_hash(inode);
>> +
>> +#ifdef CONFIG_UBIFS_FS_XATTR
>> +
>> + err = ubifs_init_security(dir, inode, &dentry->d_name);
>> + if (err) {
>> + ubifs_err("cannot initialize extended attribute, error %d",
>> + err);
>> + goto out_cancel;
>> + }
>> +
>> +#endif
>> +
>> d_instantiate(dentry, inode);
>> return 0;
>>
>> @@ -906,6 +941,18 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry,
>>
>> ubifs_release_budget(c, &req);
>> insert_inode_hash(inode);
>> +
>> +#ifdef CONFIG_UBIFS_FS_XATTR
>> +
>> + err = ubifs_init_security(dir, inode, &dentry->d_name);
>> + if (err) {
>> + ubifs_err("cannot initialize extended attribute, error %d",
>> + err);
>> + goto out_cancel;
>> + }
>> +
>> +#endif
>> +
>> d_instantiate(dentry, inode);
>> return 0;
>>
>> 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..c8be7cd 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, name, 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, name, value,
>> + size, flags);
>> +}
>> +
>> +struct xattr_handler ubifs_xattr_security_handler = {
>> + .prefix = XATTR_SECURITY_PREFIX,
>> + .list = ubifs_security_listxattr,
>> + .get = ubifs_security_getxattr,
>> + .set = ubifs_security_setxattr,
>> +};
>> +
>> +const struct xattr_handler *ubifs_xattr_handlers[] = {
>> + &ubifs_xattr_security_handler,
>> + NULL
>> +};
>> +
>> +static int ubifs_initxattrs(struct inode *inode,
>> + const struct xattr *xattr_array, void *fs_info)
>> +{
>> + const struct xattr *xattr;
>> + char *name;
>> + int err = 0;
>> +
>> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
>> + name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
>> + strlen(xattr->name) + 1, GFP_NOFS);
>> + if (!name) {
>> + err = -ENOMEM;
>> + break;
>> + }
>> + strcpy(name, XATTR_SECURITY_PREFIX);
>> + strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
>> +
>> + err = __ubifs_setxattr(inode, xattr->name, xattr->value,
>> + xattr->value_len, 0);
>> + kfree(name);
>> + if (err < 0)
>> + break;
>> + }
>> + return err;
>> +}
>> +
>> +int
>> +ubifs_init_security(struct inode *dentry, struct inode *inode,
>> + const struct qstr *qstr)
>> +{
>> + struct ubifs_inode *dir_ui = ubifs_inode(inode);
>> + int err = 0;
>> +
>> + mutex_lock(&dir_ui->ui_mutex);
>> + mutex_lock(&inode->i_mutex);
>> +
>> + err = security_inode_init_security(inode, dentry, qstr,
>> + &ubifs_initxattrs, 0);
>> + mutex_unlock(&inode->i_mutex);
>> + mutex_unlock(&dir_ui->ui_mutex);
>> + return err;
>> +}
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
2012-04-11 23:45 ` Subodh Nijsure
@ 2012-04-12 0:08 ` Mimi Zohar
-1 siblings, 0 replies; 19+ messages in thread
From: Mimi Zohar @ 2012-04-12 0:08 UTC (permalink / raw)
To: Subodh Nijsure
Cc: linux-mtd, Artem Bityutskiy, Adrian Hunter, linux-kernel,
linux-security-module, Subodh Nijsure
On Wed, 2012-04-11 at 16:45 -0700, Subodh Nijsure wrote:
> On Wed, Apr 11, 2012 at 4:27 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Wed, 2012-04-11 at 14:28 -0700, subodh.nijsure@gmail.com wrote:
> >> From: Subodh Nijsure <snijsure@grid-net.com>
> >>
> >> Also fix couple of bugs in UBIFS extended attribute length calculation.
> >>
> >> Changes in v3:
> >> Invoke ubifs_init_security only if CONFIG_UBIFS_FS_XATTR is defined.
> >> Invoke it before d_instantiate and check ubifs_init_security return
> >> code.
> >
> >> Changes in v2:
> >> Instead of just handling security.selinux extended attribute handle
> >> all security.* attributes.
> >>
> >> 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.
> >> This change also allows one to set other security.* extended
> >> attributes, such as security.smack security.evm, security.ima
> >> Ran integck test on UBI filesystem.
> >>
> >> Signed-off-by: Subodh Nijsure <snijsure@grid-net.com>
> >> ---
> >> fs/ubifs/dir.c | 47 +++++++++++++++++
> >> 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, 210 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> >> index ec9f187..422bcec 100644
> >> --- a/fs/ubifs/dir.c
> >> +++ b/fs/ubifs/dir.c
> >> @@ -292,6 +292,18 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
> >>
> >> ubifs_release_budget(c, &req);
> >> insert_inode_hash(inode);
> >> +
> >> +#ifdef CONFIG_UBIFS_FS_XATTR
> >> +
> >> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> >> + if (err) {
> >> + ubifs_err("cannot initialize extended attribute, error %d",
> >> + err);
> >> + goto out_cancel;
> >> + }
> >> +
> >> +#endif
> >> +
> >
> > The preferred method for including code based on CONFIG options is to
> > define a stub function in an include file. #ifdef's in C code is
> > frowned upon (refer to section 2.2 of Documentation/SubmittingPatches).
> >
> > thanks,
> >
> > Mimi
>
> I can certainly change the code to follow that guideline, sorry I missed it...
>
> There is other code in ubifs that uses #ifdef
> CONFIG_UBIFS_FS_XATTR/#endif should I be changing that or expectation
> is not to introduce new #ifdef/#endif? And if its the prior should
> that change come in as separate patch set?
>
> BTW, would very much appreciate if someone can actually test this
> patch with UBIFS on their hardware. I have posted a patch to
> mtd-utils/integck test program on linux-mtd mailing list.
>
> Regards,
> -Subodh
Definitely keep your patch separate from any other cleanup. Most, if
not all other filesystems, put the xattr code in a separate file. The
Makefile includes the file based on the CONFIG option. Refer to
fs/ext3|ext4/Makefile for an example.
thanks,
Mimi
> >> d_instantiate(dentry, inode);
> >> return 0;
> >>
> >> @@ -753,6 +765,17 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
> >> mutex_unlock(&dir_ui->ui_mutex);
> >>
> >> ubifs_release_budget(c, &req);
> >> +
> >> +#ifdef CONFIG_UBIFS_FS_XATTR
> >> +
> >> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> >> + if (err) {
> >> + ubifs_err("cannot initialize extended attribute, error %d",
> >> + err);
> >> + goto out_cancel;
> >> + }
> >> +
> >> +#endif
> >> d_instantiate(dentry, inode);
> >> return 0;
> >>
> >> @@ -830,6 +853,18 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry,
> >>
> >> ubifs_release_budget(c, &req);
> >> insert_inode_hash(inode);
> >> +
> >> +#ifdef CONFIG_UBIFS_FS_XATTR
> >> +
> >> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> >> + if (err) {
> >> + ubifs_err("cannot initialize extended attribute, error %d",
> >> + err);
> >> + goto out_cancel;
> >> + }
> >> +
> >> +#endif
> >> +
> >> d_instantiate(dentry, inode);
> >> return 0;
> >>
> >> @@ -906,6 +941,18 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry,
> >>
> >> ubifs_release_budget(c, &req);
> >> insert_inode_hash(inode);
> >> +
> >> +#ifdef CONFIG_UBIFS_FS_XATTR
> >> +
> >> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> >> + if (err) {
> >> + ubifs_err("cannot initialize extended attribute, error %d",
> >> + err);
> >> + goto out_cancel;
> >> + }
> >> +
> >> +#endif
> >> +
> >> d_instantiate(dentry, inode);
> >> return 0;
> >>
> >> 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..c8be7cd 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, name, 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, name, value,
> >> + size, flags);
> >> +}
> >> +
> >> +struct xattr_handler ubifs_xattr_security_handler = {
> >> + .prefix = XATTR_SECURITY_PREFIX,
> >> + .list = ubifs_security_listxattr,
> >> + .get = ubifs_security_getxattr,
> >> + .set = ubifs_security_setxattr,
> >> +};
> >> +
> >> +const struct xattr_handler *ubifs_xattr_handlers[] = {
> >> + &ubifs_xattr_security_handler,
> >> + NULL
> >> +};
> >> +
> >> +static int ubifs_initxattrs(struct inode *inode,
> >> + const struct xattr *xattr_array, void *fs_info)
> >> +{
> >> + const struct xattr *xattr;
> >> + char *name;
> >> + int err = 0;
> >> +
> >> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
> >> + name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
> >> + strlen(xattr->name) + 1, GFP_NOFS);
> >> + if (!name) {
> >> + err = -ENOMEM;
> >> + break;
> >> + }
> >> + strcpy(name, XATTR_SECURITY_PREFIX);
> >> + strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
> >> +
> >> + err = __ubifs_setxattr(inode, xattr->name, xattr->value,
> >> + xattr->value_len, 0);
> >> + kfree(name);
> >> + if (err < 0)
> >> + break;
> >> + }
> >> + return err;
> >> +}
> >> +
> >> +int
> >> +ubifs_init_security(struct inode *dentry, struct inode *inode,
> >> + const struct qstr *qstr)
> >> +{
> >> + struct ubifs_inode *dir_ui = ubifs_inode(inode);
> >> + int err = 0;
> >> +
> >> + mutex_lock(&dir_ui->ui_mutex);
> >> + mutex_lock(&inode->i_mutex);
> >> +
> >> + err = security_inode_init_security(inode, dentry, qstr,
> >> + &ubifs_initxattrs, 0);
> >> + mutex_unlock(&inode->i_mutex);
> >> + mutex_unlock(&dir_ui->ui_mutex);
> >> + return err;
> >> +}
> >
> >
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
@ 2012-04-12 0:08 ` Mimi Zohar
0 siblings, 0 replies; 19+ messages in thread
From: Mimi Zohar @ 2012-04-12 0:08 UTC (permalink / raw)
To: Subodh Nijsure
Cc: Subodh Nijsure, Artem Bityutskiy, linux-kernel, Adrian Hunter,
linux-security-module, linux-mtd
On Wed, 2012-04-11 at 16:45 -0700, Subodh Nijsure wrote:
> On Wed, Apr 11, 2012 at 4:27 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Wed, 2012-04-11 at 14:28 -0700, subodh.nijsure@gmail.com wrote:
> >> From: Subodh Nijsure <snijsure@grid-net.com>
> >>
> >> Also fix couple of bugs in UBIFS extended attribute length calculation.
> >>
> >> Changes in v3:
> >> Invoke ubifs_init_security only if CONFIG_UBIFS_FS_XATTR is defined.
> >> Invoke it before d_instantiate and check ubifs_init_security return
> >> code.
> >
> >> Changes in v2:
> >> Instead of just handling security.selinux extended attribute handle
> >> all security.* attributes.
> >>
> >> 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.
> >> This change also allows one to set other security.* extended
> >> attributes, such as security.smack security.evm, security.ima
> >> Ran integck test on UBI filesystem.
> >>
> >> Signed-off-by: Subodh Nijsure <snijsure@grid-net.com>
> >> ---
> >> fs/ubifs/dir.c | 47 +++++++++++++++++
> >> 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, 210 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> >> index ec9f187..422bcec 100644
> >> --- a/fs/ubifs/dir.c
> >> +++ b/fs/ubifs/dir.c
> >> @@ -292,6 +292,18 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
> >>
> >> ubifs_release_budget(c, &req);
> >> insert_inode_hash(inode);
> >> +
> >> +#ifdef CONFIG_UBIFS_FS_XATTR
> >> +
> >> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> >> + if (err) {
> >> + ubifs_err("cannot initialize extended attribute, error %d",
> >> + err);
> >> + goto out_cancel;
> >> + }
> >> +
> >> +#endif
> >> +
> >
> > The preferred method for including code based on CONFIG options is to
> > define a stub function in an include file. #ifdef's in C code is
> > frowned upon (refer to section 2.2 of Documentation/SubmittingPatches).
> >
> > thanks,
> >
> > Mimi
>
> I can certainly change the code to follow that guideline, sorry I missed it...
>
> There is other code in ubifs that uses #ifdef
> CONFIG_UBIFS_FS_XATTR/#endif should I be changing that or expectation
> is not to introduce new #ifdef/#endif? And if its the prior should
> that change come in as separate patch set?
>
> BTW, would very much appreciate if someone can actually test this
> patch with UBIFS on their hardware. I have posted a patch to
> mtd-utils/integck test program on linux-mtd mailing list.
>
> Regards,
> -Subodh
Definitely keep your patch separate from any other cleanup. Most, if
not all other filesystems, put the xattr code in a separate file. The
Makefile includes the file based on the CONFIG option. Refer to
fs/ext3|ext4/Makefile for an example.
thanks,
Mimi
> >> d_instantiate(dentry, inode);
> >> return 0;
> >>
> >> @@ -753,6 +765,17 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
> >> mutex_unlock(&dir_ui->ui_mutex);
> >>
> >> ubifs_release_budget(c, &req);
> >> +
> >> +#ifdef CONFIG_UBIFS_FS_XATTR
> >> +
> >> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> >> + if (err) {
> >> + ubifs_err("cannot initialize extended attribute, error %d",
> >> + err);
> >> + goto out_cancel;
> >> + }
> >> +
> >> +#endif
> >> d_instantiate(dentry, inode);
> >> return 0;
> >>
> >> @@ -830,6 +853,18 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry,
> >>
> >> ubifs_release_budget(c, &req);
> >> insert_inode_hash(inode);
> >> +
> >> +#ifdef CONFIG_UBIFS_FS_XATTR
> >> +
> >> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> >> + if (err) {
> >> + ubifs_err("cannot initialize extended attribute, error %d",
> >> + err);
> >> + goto out_cancel;
> >> + }
> >> +
> >> +#endif
> >> +
> >> d_instantiate(dentry, inode);
> >> return 0;
> >>
> >> @@ -906,6 +941,18 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry,
> >>
> >> ubifs_release_budget(c, &req);
> >> insert_inode_hash(inode);
> >> +
> >> +#ifdef CONFIG_UBIFS_FS_XATTR
> >> +
> >> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> >> + if (err) {
> >> + ubifs_err("cannot initialize extended attribute, error %d",
> >> + err);
> >> + goto out_cancel;
> >> + }
> >> +
> >> +#endif
> >> +
> >> d_instantiate(dentry, inode);
> >> return 0;
> >>
> >> 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..c8be7cd 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, name, 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, name, value,
> >> + size, flags);
> >> +}
> >> +
> >> +struct xattr_handler ubifs_xattr_security_handler = {
> >> + .prefix = XATTR_SECURITY_PREFIX,
> >> + .list = ubifs_security_listxattr,
> >> + .get = ubifs_security_getxattr,
> >> + .set = ubifs_security_setxattr,
> >> +};
> >> +
> >> +const struct xattr_handler *ubifs_xattr_handlers[] = {
> >> + &ubifs_xattr_security_handler,
> >> + NULL
> >> +};
> >> +
> >> +static int ubifs_initxattrs(struct inode *inode,
> >> + const struct xattr *xattr_array, void *fs_info)
> >> +{
> >> + const struct xattr *xattr;
> >> + char *name;
> >> + int err = 0;
> >> +
> >> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
> >> + name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
> >> + strlen(xattr->name) + 1, GFP_NOFS);
> >> + if (!name) {
> >> + err = -ENOMEM;
> >> + break;
> >> + }
> >> + strcpy(name, XATTR_SECURITY_PREFIX);
> >> + strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
> >> +
> >> + err = __ubifs_setxattr(inode, xattr->name, xattr->value,
> >> + xattr->value_len, 0);
> >> + kfree(name);
> >> + if (err < 0)
> >> + break;
> >> + }
> >> + return err;
> >> +}
> >> +
> >> +int
> >> +ubifs_init_security(struct inode *dentry, struct inode *inode,
> >> + const struct qstr *qstr)
> >> +{
> >> + struct ubifs_inode *dir_ui = ubifs_inode(inode);
> >> + int err = 0;
> >> +
> >> + mutex_lock(&dir_ui->ui_mutex);
> >> + mutex_lock(&inode->i_mutex);
> >> +
> >> + err = security_inode_init_security(inode, dentry, qstr,
> >> + &ubifs_initxattrs, 0);
> >> + mutex_unlock(&inode->i_mutex);
> >> + mutex_unlock(&dir_ui->ui_mutex);
> >> + return err;
> >> +}
> >
> >
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
2012-04-11 21:28 ` subodh.nijsure
@ 2012-04-12 7:11 ` Artem Bityutskiy
-1 siblings, 0 replies; 19+ messages in thread
From: Artem Bityutskiy @ 2012-04-12 7:11 UTC (permalink / raw)
To: subodh.nijsure
Cc: linux-mtd, Adrian Hunter, linux-kernel, linux-security-module,
Subodh Nijsure
[-- Attachment #1: Type: text/plain, Size: 614 bytes --]
On Wed, 2012-04-11 at 14:28 -0700, subodh.nijsure@gmail.com wrote:
> +#ifdef CONFIG_UBIFS_FS_XATTR
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> +#endif
Suboodh,
would you please first create a preparational patch which removes
the CONFIG_UBIFS_FS_XATTR completely and makes xattr support to be
always present.
And then you could add the security stuff on top of this without the
#ifdef churn.
Thanks!
--
Best Regards,
Artem Bityutskiy
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
@ 2012-04-12 7:11 ` Artem Bityutskiy
0 siblings, 0 replies; 19+ messages in thread
From: Artem Bityutskiy @ 2012-04-12 7:11 UTC (permalink / raw)
To: subodh.nijsure
Cc: linux-security-module, Subodh Nijsure, linux-mtd, Adrian Hunter,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 614 bytes --]
On Wed, 2012-04-11 at 14:28 -0700, subodh.nijsure@gmail.com wrote:
> +#ifdef CONFIG_UBIFS_FS_XATTR
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> +#endif
Suboodh,
would you please first create a preparational patch which removes
the CONFIG_UBIFS_FS_XATTR completely and makes xattr support to be
always present.
And then you could add the security stuff on top of this without the
#ifdef churn.
Thanks!
--
Best Regards,
Artem Bityutskiy
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
2012-04-12 7:11 ` Artem Bityutskiy
(?)
@ 2012-04-12 13:10 ` Subodh Nijsure
-1 siblings, 0 replies; 19+ messages in thread
From: Subodh Nijsure @ 2012-04-12 13:10 UTC (permalink / raw)
To: dedekind1; +Cc: linux-mtd
On Thu, Apr 12, 2012 at 12:11 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Wed, 2012-04-11 at 14:28 -0700, subodh.nijsure@gmail.com wrote:
>> +#ifdef CONFIG_UBIFS_FS_XATTR
>> +
>> + err = ubifs_init_security(dir, inode, &dentry->d_name);
>> + if (err) {
>> + ubifs_err("cannot initialize extended attribute, error %d",
>> + err);
>> + goto out_cancel;
>> + }
>> +
>> +#endif
>
> Suboodh,
>
> would you please first create a preparational patch which removes
> the CONFIG_UBIFS_FS_XATTR completely and makes xattr support to be
> always present.
>
> And then you could add the security stuff on top of this without the
> #ifdef churn.
>
> Thanks!
I will do that.
In general modification will be to move all the real code within
#ifdef CONFIG_UBIFS_FS_XATTR/#endif to fs/ubifs/xattr.c, without ifdef
of course.
And for cases where its not defined within #ifndef
CONFIG_UBIFS_FS_XATTR/#endif, provide inline static implementation of
functions that always return "success", where required, in ubifs.h
-Subodh
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
2012-04-11 21:28 ` subodh.nijsure
@ 2012-04-13 13:41 ` Stephen Smalley
-1 siblings, 0 replies; 19+ messages in thread
From: Stephen Smalley @ 2012-04-13 13:41 UTC (permalink / raw)
To: subodh.nijsure
Cc: linux-mtd, Artem Bityutskiy, Adrian Hunter, linux-kernel,
linux-security-module, Subodh Nijsure
On Wed, 2012-04-11 at 14:28 -0700, subodh.nijsure@gmail.com wrote:
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index ec9f187..422bcec 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -292,6 +292,18 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
>
> ubifs_release_budget(c, &req);
> insert_inode_hash(inode);
> +
> +#ifdef CONFIG_UBIFS_FS_XATTR
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> +#endif
> +
> d_instantiate(dentry, inode);
> return 0;
I don't know this code very well, but simply reusing the out_cancel
error path when ubifs_init_security() fails looks wrong to me. You'll
end up calling ubifs_release_budget() a second time. You could move the
ubifs_init_security() call a bit earlier, prior to the
ubifs_release_budget() call, so that it only happens once on the error
path. I also am unclear on what if anything you need to do about the
prior ubifs_jnl_update() if ubifs_init_security() fails; it seems like
you either need to update the journal again to reflect the inode
deletion or you need to move ubifs_init_security() prior to
ubifs_jnl_update() as well.
I don't know if this would work, but I would be inclined to move the
call to ubifs_init_security() immediately after the ubifs_new_inode()
call, and introduce a new out_security: label just before the
make_bad_inode() call to which you can jump on error. However, that
might produce a strange state in the journal, with the setxattr recorded
before the inode creation. Optimally they would both be part of the
same journal transaction, as in ext4, and thus applied atomically or not
at all (i.e. a file is either created with a security xattr or not at
all).
--
Stephen Smalley
National Security Agency
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
@ 2012-04-13 13:41 ` Stephen Smalley
0 siblings, 0 replies; 19+ messages in thread
From: Stephen Smalley @ 2012-04-13 13:41 UTC (permalink / raw)
To: subodh.nijsure
Cc: Subodh Nijsure, Artem Bityutskiy, linux-kernel, Adrian Hunter,
linux-security-module, linux-mtd
On Wed, 2012-04-11 at 14:28 -0700, subodh.nijsure@gmail.com wrote:
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index ec9f187..422bcec 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -292,6 +292,18 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
>
> ubifs_release_budget(c, &req);
> insert_inode_hash(inode);
> +
> +#ifdef CONFIG_UBIFS_FS_XATTR
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> +#endif
> +
> d_instantiate(dentry, inode);
> return 0;
I don't know this code very well, but simply reusing the out_cancel
error path when ubifs_init_security() fails looks wrong to me. You'll
end up calling ubifs_release_budget() a second time. You could move the
ubifs_init_security() call a bit earlier, prior to the
ubifs_release_budget() call, so that it only happens once on the error
path. I also am unclear on what if anything you need to do about the
prior ubifs_jnl_update() if ubifs_init_security() fails; it seems like
you either need to update the journal again to reflect the inode
deletion or you need to move ubifs_init_security() prior to
ubifs_jnl_update() as well.
I don't know if this would work, but I would be inclined to move the
call to ubifs_init_security() immediately after the ubifs_new_inode()
call, and introduce a new out_security: label just before the
make_bad_inode() call to which you can jump on error. However, that
might produce a strange state in the journal, with the setxattr recorded
before the inode creation. Optimally they would both be part of the
same journal transaction, as in ext4, and thus applied atomically or not
at all (i.e. a file is either created with a security xattr or not at
all).
--
Stephen Smalley
National Security Agency
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
2012-05-11 12:01 ` Tetsuo Handa
@ 2012-05-11 14:25 ` Subodh Nijsure
-1 siblings, 0 replies; 19+ messages in thread
From: Subodh Nijsure @ 2012-05-11 14:25 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: linux-mtd, linux-security-module, linux-kernel
On 05/11/2012 05:01 AM, Tetsuo Handa wrote:
> Subodh Nijsure wrote:
>> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
>> index ad6e550..cec3ffab 100644
>> --- a/fs/ubifs/dir.c
>> +++ b/fs/ubifs/dir.c
>> @@ -292,6 +292,14 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
>>
> According to 3.4-rc6,
>
> mutex_unlock() is here.
>
>> ubifs_release_budget(c,&req);
>> insert_inode_hash(inode);
>> +
>> + err = ubifs_init_security(dir, inode,&dentry->d_name);
>> + if (err) {
>> + ubifs_err("cannot initialize extended attribute, error %d",
>> + err);
>> + goto out_cancel;
>> + }
>> +
>> d_instantiate(dentry, inode);
>> return 0;
> out_cancel:
> mutex_unlock() is also here...
>
> Same for the rest.
I will double check, my tree is based off linux-rc2, may be I messed up
things.
>> diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
>> index 85b2722..49c426a 100644
>> --- a/fs/ubifs/xattr.c
>> +++ b/fs/ubifs/xattr.c
>> @@ -568,3 +600,91 @@ 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';
>> + }
> If (list&& total_len> list_size), the caller will see total_len
> but no data is copied to list. Is it OK?
the above condition you referred to implies that caller passed in buffer
that was too short, in that case we don't need to copy the data. Typical
usage would be caller call listxattr with null and list size of zero, we
return size of the buffer etc.
man listxattr
ssize_t listxattr (const char *path, char *list, size_t size);
An empty buffer of size zero can be passed into these calls to
return the current size of the list of extende attribute names, which
can be used to estimate the size of a buffer which is sufficiently large
to hold the list of names.
so the above code should be okay?
>> + return total_len;
>> +}
>> +static int ubifs_initxattrs(struct inode *inode,
>> + const struct xattr *xattr_array, void *fs_info)
>> +{
>> + const struct xattr *xattr;
>> + char *name;
>> + int err = 0;
>> +
>> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
>> + name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
>> + strlen(xattr->name) + 1, GFP_NOFS);
> Maybe nice to have kstrdup2(const char *str1, const char *str2, gfp_t flags).
I will modify that.
>> + if (!name) {
>> + err = -ENOMEM;
>> + break;
>> + }
>> + strcpy(name, XATTR_SECURITY_PREFIX);
>> + strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
>> + err = __ubifs_setxattr(inode, name, xattr->value,
>> + xattr->value_len, 0);
>> + kfree(name);
>> + if (err< 0)
>> + break;
>> + }
>> + return err;
>> +}
-Subodh
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
@ 2012-05-11 14:25 ` Subodh Nijsure
0 siblings, 0 replies; 19+ messages in thread
From: Subodh Nijsure @ 2012-05-11 14:25 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: linux-security-module, linux-mtd, linux-kernel
On 05/11/2012 05:01 AM, Tetsuo Handa wrote:
> Subodh Nijsure wrote:
>> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
>> index ad6e550..cec3ffab 100644
>> --- a/fs/ubifs/dir.c
>> +++ b/fs/ubifs/dir.c
>> @@ -292,6 +292,14 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
>>
> According to 3.4-rc6,
>
> mutex_unlock() is here.
>
>> ubifs_release_budget(c,&req);
>> insert_inode_hash(inode);
>> +
>> + err = ubifs_init_security(dir, inode,&dentry->d_name);
>> + if (err) {
>> + ubifs_err("cannot initialize extended attribute, error %d",
>> + err);
>> + goto out_cancel;
>> + }
>> +
>> d_instantiate(dentry, inode);
>> return 0;
> out_cancel:
> mutex_unlock() is also here...
>
> Same for the rest.
I will double check, my tree is based off linux-rc2, may be I messed up
things.
>> diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
>> index 85b2722..49c426a 100644
>> --- a/fs/ubifs/xattr.c
>> +++ b/fs/ubifs/xattr.c
>> @@ -568,3 +600,91 @@ 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';
>> + }
> If (list&& total_len> list_size), the caller will see total_len
> but no data is copied to list. Is it OK?
the above condition you referred to implies that caller passed in buffer
that was too short, in that case we don't need to copy the data. Typical
usage would be caller call listxattr with null and list size of zero, we
return size of the buffer etc.
man listxattr
ssize_t listxattr (const char *path, char *list, size_t size);
An empty buffer of size zero can be passed into these calls to
return the current size of the list of extende attribute names, which
can be used to estimate the size of a buffer which is sufficiently large
to hold the list of names.
so the above code should be okay?
>> + return total_len;
>> +}
>> +static int ubifs_initxattrs(struct inode *inode,
>> + const struct xattr *xattr_array, void *fs_info)
>> +{
>> + const struct xattr *xattr;
>> + char *name;
>> + int err = 0;
>> +
>> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
>> + name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
>> + strlen(xattr->name) + 1, GFP_NOFS);
> Maybe nice to have kstrdup2(const char *str1, const char *str2, gfp_t flags).
I will modify that.
>> + if (!name) {
>> + err = -ENOMEM;
>> + break;
>> + }
>> + strcpy(name, XATTR_SECURITY_PREFIX);
>> + strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
>> + err = __ubifs_setxattr(inode, name, xattr->value,
>> + xattr->value_len, 0);
>> + kfree(name);
>> + if (err< 0)
>> + break;
>> + }
>> + return err;
>> +}
-Subodh
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
2012-05-10 23:17 ` Subodh Nijsure
@ 2012-05-11 12:01 ` Tetsuo Handa
-1 siblings, 0 replies; 19+ messages in thread
From: Tetsuo Handa @ 2012-05-11 12:01 UTC (permalink / raw)
To: snijsure; +Cc: linux-mtd, linux-security-module, linux-kernel
Subodh Nijsure wrote:
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index ad6e550..cec3ffab 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -292,6 +292,14 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
>
According to 3.4-rc6,
mutex_unlock() is here.
> ubifs_release_budget(c, &req);
> insert_inode_hash(inode);
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> d_instantiate(dentry, inode);
> return 0;
out_cancel:
mutex_unlock() is also here...
Same for the rest.
> diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
> index 85b2722..49c426a 100644
> --- a/fs/ubifs/xattr.c
> +++ b/fs/ubifs/xattr.c
> @@ -568,3 +600,91 @@ 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';
> + }
If (list && total_len > list_size), the caller will see total_len
but no data is copied to list. Is it OK?
> + return total_len;
> +}
> +static int ubifs_initxattrs(struct inode *inode,
> + const struct xattr *xattr_array, void *fs_info)
> +{
> + const struct xattr *xattr;
> + char *name;
> + int err = 0;
> +
> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
> + name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
> + strlen(xattr->name) + 1, GFP_NOFS);
Maybe nice to have kstrdup2(const char *str1, const char *str2, gfp_t flags).
> + if (!name) {
> + err = -ENOMEM;
> + break;
> + }
> + strcpy(name, XATTR_SECURITY_PREFIX);
> + strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
> + err = __ubifs_setxattr(inode, name, xattr->value,
> + xattr->value_len, 0);
> + kfree(name);
> + if (err < 0)
> + break;
> + }
> + return err;
> +}
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3] Add security.* XATTR support for the UBIFS
@ 2012-05-11 12:01 ` Tetsuo Handa
0 siblings, 0 replies; 19+ messages in thread
From: Tetsuo Handa @ 2012-05-11 12:01 UTC (permalink / raw)
To: snijsure; +Cc: linux-security-module, linux-mtd, linux-kernel
Subodh Nijsure wrote:
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index ad6e550..cec3ffab 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -292,6 +292,14 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
>
According to 3.4-rc6,
mutex_unlock() is here.
> ubifs_release_budget(c, &req);
> insert_inode_hash(inode);
> +
> + err = ubifs_init_security(dir, inode, &dentry->d_name);
> + if (err) {
> + ubifs_err("cannot initialize extended attribute, error %d",
> + err);
> + goto out_cancel;
> + }
> +
> d_instantiate(dentry, inode);
> return 0;
out_cancel:
mutex_unlock() is also here...
Same for the rest.
> diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
> index 85b2722..49c426a 100644
> --- a/fs/ubifs/xattr.c
> +++ b/fs/ubifs/xattr.c
> @@ -568,3 +600,91 @@ 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';
> + }
If (list && total_len > list_size), the caller will see total_len
but no data is copied to list. Is it OK?
> + return total_len;
> +}
> +static int ubifs_initxattrs(struct inode *inode,
> + const struct xattr *xattr_array, void *fs_info)
> +{
> + const struct xattr *xattr;
> + char *name;
> + int err = 0;
> +
> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
> + name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
> + strlen(xattr->name) + 1, GFP_NOFS);
Maybe nice to have kstrdup2(const char *str1, const char *str2, gfp_t flags).
> + if (!name) {
> + err = -ENOMEM;
> + break;
> + }
> + strcpy(name, XATTR_SECURITY_PREFIX);
> + strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
> + err = __ubifs_setxattr(inode, name, xattr->value,
> + xattr->value_len, 0);
> + kfree(name);
> + if (err < 0)
> + break;
> + }
> + return err;
> +}
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3] Add security.* XATTR support for the UBIFS
@ 2012-05-10 23:17 ` Subodh Nijsure
0 siblings, 0 replies; 19+ messages in thread
From: Subodh Nijsure @ 2012-05-10 23:17 UTC (permalink / raw)
To: linux-mtd, linux-security-module; +Cc: linux-kernel, snijsure
Also fix couple of bugs in UBIFS extended attribute length calculation.
Changes in v3:
Remove #ifdef CONFIG_UBIFS_FS_XATTR
Changes in v2:
Instead of just handling security.selinux extended attribute handle
all security.* attributes.
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.
This change also allows one to set other security.* extended
attributesr, such as security.smack security.evm, security.ima
Ran integck test on UBI filesystem.
Signed-off-by: Subodh Nijsure <snijsure@grid-net.com>
---
fs/ubifs/dir.c | 31 +++++++++++
fs/ubifs/file.c | 4 ++
fs/ubifs/journal.c | 12 +++-
fs/ubifs/super.c | 1 +
fs/ubifs/ubifs.h | 9 +++
fs/ubifs/xattr.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++------
6 files changed, 190 insertions(+), 19 deletions(-)
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index ad6e550..cec3ffab 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -292,6 +292,14 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
d_instantiate(dentry, inode);
return 0;
@@ -753,6 +761,13 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
mutex_unlock(&dir_ui->ui_mutex);
ubifs_release_budget(c, &req);
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
d_instantiate(dentry, inode);
return 0;
@@ -830,6 +845,14 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry,
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
d_instantiate(dentry, inode);
return 0;
@@ -906,6 +929,14 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry,
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
d_instantiate(dentry, inode);
return 0;
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 0fe640c..8629da0 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1573,6 +1573,10 @@ const struct inode_operations ubifs_symlink_inode_operations = {
.follow_link = ubifs_follow_link,
.setattr = ubifs_setattr,
.getattr = ubifs_getattr,
+ .setxattr = ubifs_symlink_setxattr,
+ .getxattr = ubifs_symlink_getxattr,
+ .listxattr = ubifs_listxattr,
+ .removexattr = ubifs_removexattr,
};
const struct file_operations ubifs_file_operations = {
diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index bc75e9d..5086823 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..228c69d 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2061,6 +2061,7 @@ 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;
+ sb->s_xattr = ubifs_xattr_handlers;
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..49c426a 100644
--- a/fs/ubifs/xattr.c
+++ b/fs/ubifs/xattr.c
@@ -107,6 +107,7 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
.new_ino_d = ALIGN(size, 8), .dirtied_ino = 1,
.dirtied_ino_d = ALIGN(host_ui->data_len, 8) };
+ ubifs_assert(mutex_is_locked(&host_ui->ui_mutex));
if (host_ui->xattr_cnt >= MAX_XATTRS_PER_INODE)
return -ENOSPC;
/*
@@ -146,18 +147,14 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
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_cnt += 1;
host_ui->xattr_size += CALC_DENT_SIZE(nm->len);
host_ui->xattr_size += CALC_XATTR_BYTES(size);
host_ui->xattr_names += nm->len;
-
err = ubifs_jnl_update(c, host, nm, inode, 0, 1);
if (err)
goto out_cancel;
- mutex_unlock(&host_ui->ui_mutex);
-
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
iput(inode);
@@ -167,7 +164,6 @@ out_cancel:
host_ui->xattr_cnt -= 1;
host_ui->xattr_size -= CALC_DENT_SIZE(nm->len);
host_ui->xattr_size -= CALC_XATTR_BYTES(size);
- mutex_unlock(&host_ui->ui_mutex);
out_free:
make_bad_inode(inode);
iput(inode);
@@ -209,11 +205,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 +289,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 +350,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 +380,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 +429,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 +600,91 @@ 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, name, 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, name, value,
+ size, flags);
+}
+
+struct xattr_handler ubifs_xattr_security_handler = {
+ .prefix = XATTR_SECURITY_PREFIX,
+ .list = ubifs_security_listxattr,
+ .get = ubifs_security_getxattr,
+ .set = ubifs_security_setxattr,
+};
+
+const struct xattr_handler *ubifs_xattr_handlers[] = {
+ &ubifs_xattr_security_handler,
+ NULL
+};
+
+static int ubifs_initxattrs(struct inode *inode,
+ const struct xattr *xattr_array, void *fs_info)
+{
+ const struct xattr *xattr;
+ char *name;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
+ strlen(xattr->name) + 1, GFP_NOFS);
+ if (!name) {
+ err = -ENOMEM;
+ break;
+ }
+ strcpy(name, XATTR_SECURITY_PREFIX);
+ strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
+ err = __ubifs_setxattr(inode, name, xattr->value,
+ xattr->value_len, 0);
+ kfree(name);
+ if (err < 0)
+ break;
+ }
+ return err;
+}
+
+int
+ubifs_init_security(struct inode *dentry, struct inode *inode,
+ const struct qstr *qstr)
+{
+ struct ubifs_inode *dir_ui = ubifs_inode(inode);
+ int err = 0;
+
+ mutex_lock(&dir_ui->ui_mutex);
+ mutex_lock(&inode->i_mutex);
+
+ err = security_inode_init_security(inode, dentry, qstr,
+ &ubifs_initxattrs, 0);
+ mutex_unlock(&inode->i_mutex);
+ mutex_unlock(&dir_ui->ui_mutex);
+ return err;
+}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3] Add security.* XATTR support for the UBIFS
@ 2012-05-10 23:17 ` Subodh Nijsure
0 siblings, 0 replies; 19+ messages in thread
From: Subodh Nijsure @ 2012-05-10 23:17 UTC (permalink / raw)
To: linux-mtd, linux-security-module; +Cc: linux-kernel, snijsure
Also fix couple of bugs in UBIFS extended attribute length calculation.
Changes in v3:
Remove #ifdef CONFIG_UBIFS_FS_XATTR
Changes in v2:
Instead of just handling security.selinux extended attribute handle
all security.* attributes.
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.
This change also allows one to set other security.* extended
attributesr, such as security.smack security.evm, security.ima
Ran integck test on UBI filesystem.
Signed-off-by: Subodh Nijsure <snijsure@grid-net.com>
---
fs/ubifs/dir.c | 31 +++++++++++
fs/ubifs/file.c | 4 ++
fs/ubifs/journal.c | 12 +++-
fs/ubifs/super.c | 1 +
fs/ubifs/ubifs.h | 9 +++
fs/ubifs/xattr.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++------
6 files changed, 190 insertions(+), 19 deletions(-)
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index ad6e550..cec3ffab 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -292,6 +292,14 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
d_instantiate(dentry, inode);
return 0;
@@ -753,6 +761,13 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
mutex_unlock(&dir_ui->ui_mutex);
ubifs_release_budget(c, &req);
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
d_instantiate(dentry, inode);
return 0;
@@ -830,6 +845,14 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry,
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
d_instantiate(dentry, inode);
return 0;
@@ -906,6 +929,14 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry,
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
+
+ err = ubifs_init_security(dir, inode, &dentry->d_name);
+ if (err) {
+ ubifs_err("cannot initialize extended attribute, error %d",
+ err);
+ goto out_cancel;
+ }
+
d_instantiate(dentry, inode);
return 0;
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 0fe640c..8629da0 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1573,6 +1573,10 @@ const struct inode_operations ubifs_symlink_inode_operations = {
.follow_link = ubifs_follow_link,
.setattr = ubifs_setattr,
.getattr = ubifs_getattr,
+ .setxattr = ubifs_symlink_setxattr,
+ .getxattr = ubifs_symlink_getxattr,
+ .listxattr = ubifs_listxattr,
+ .removexattr = ubifs_removexattr,
};
const struct file_operations ubifs_file_operations = {
diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index bc75e9d..5086823 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..228c69d 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2061,6 +2061,7 @@ 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;
+ sb->s_xattr = ubifs_xattr_handlers;
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..49c426a 100644
--- a/fs/ubifs/xattr.c
+++ b/fs/ubifs/xattr.c
@@ -107,6 +107,7 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
.new_ino_d = ALIGN(size, 8), .dirtied_ino = 1,
.dirtied_ino_d = ALIGN(host_ui->data_len, 8) };
+ ubifs_assert(mutex_is_locked(&host_ui->ui_mutex));
if (host_ui->xattr_cnt >= MAX_XATTRS_PER_INODE)
return -ENOSPC;
/*
@@ -146,18 +147,14 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
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_cnt += 1;
host_ui->xattr_size += CALC_DENT_SIZE(nm->len);
host_ui->xattr_size += CALC_XATTR_BYTES(size);
host_ui->xattr_names += nm->len;
-
err = ubifs_jnl_update(c, host, nm, inode, 0, 1);
if (err)
goto out_cancel;
- mutex_unlock(&host_ui->ui_mutex);
-
ubifs_release_budget(c, &req);
insert_inode_hash(inode);
iput(inode);
@@ -167,7 +164,6 @@ out_cancel:
host_ui->xattr_cnt -= 1;
host_ui->xattr_size -= CALC_DENT_SIZE(nm->len);
host_ui->xattr_size -= CALC_XATTR_BYTES(size);
- mutex_unlock(&host_ui->ui_mutex);
out_free:
make_bad_inode(inode);
iput(inode);
@@ -209,11 +205,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 +289,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 +350,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 +380,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 +429,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 +600,91 @@ 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, name, 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, name, value,
+ size, flags);
+}
+
+struct xattr_handler ubifs_xattr_security_handler = {
+ .prefix = XATTR_SECURITY_PREFIX,
+ .list = ubifs_security_listxattr,
+ .get = ubifs_security_getxattr,
+ .set = ubifs_security_setxattr,
+};
+
+const struct xattr_handler *ubifs_xattr_handlers[] = {
+ &ubifs_xattr_security_handler,
+ NULL
+};
+
+static int ubifs_initxattrs(struct inode *inode,
+ const struct xattr *xattr_array, void *fs_info)
+{
+ const struct xattr *xattr;
+ char *name;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
+ strlen(xattr->name) + 1, GFP_NOFS);
+ if (!name) {
+ err = -ENOMEM;
+ break;
+ }
+ strcpy(name, XATTR_SECURITY_PREFIX);
+ strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
+ err = __ubifs_setxattr(inode, name, xattr->value,
+ xattr->value_len, 0);
+ kfree(name);
+ if (err < 0)
+ break;
+ }
+ return err;
+}
+
+int
+ubifs_init_security(struct inode *dentry, struct inode *inode,
+ const struct qstr *qstr)
+{
+ struct ubifs_inode *dir_ui = ubifs_inode(inode);
+ int err = 0;
+
+ mutex_lock(&dir_ui->ui_mutex);
+ mutex_lock(&inode->i_mutex);
+
+ err = security_inode_init_security(inode, dentry, qstr,
+ &ubifs_initxattrs, 0);
+ mutex_unlock(&inode->i_mutex);
+ mutex_unlock(&dir_ui->ui_mutex);
+ return err;
+}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-05-11 14:25 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-11 21:28 [PATCH v3] Add security.* XATTR support for the UBIFS subodh.nijsure
2012-04-11 21:28 ` subodh.nijsure
2012-04-11 23:27 ` Mimi Zohar
2012-04-11 23:27 ` Mimi Zohar
2012-04-11 23:45 ` Subodh Nijsure
2012-04-11 23:45 ` Subodh Nijsure
2012-04-12 0:08 ` Mimi Zohar
2012-04-12 0:08 ` Mimi Zohar
2012-04-12 7:11 ` Artem Bityutskiy
2012-04-12 7:11 ` Artem Bityutskiy
2012-04-12 13:10 ` Subodh Nijsure
2012-04-13 13:41 ` Stephen Smalley
2012-04-13 13:41 ` Stephen Smalley
2012-05-10 23:17 Subodh Nijsure
2012-05-10 23:17 ` Subodh Nijsure
2012-05-11 12:01 ` Tetsuo Handa
2012-05-11 12:01 ` Tetsuo Handa
2012-05-11 14:25 ` Subodh Nijsure
2012-05-11 14:25 ` Subodh Nijsure
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.