On 17/06/11 17:46, Mimi Zohar wrote:
> Option 2:
>
> In preparation for supporting the initialization of multiple LSM xattrs
> and the EVM xattr, this patch defines an xattr structure and changes
> the security_inode_init_security API, adding an fs specific function
> callback parameter to write the xattrs.
>
> Initially the callback function walks the array of xattrs, calling the
> xattr 'set' function for each xattr. To optimize performance an FS could
> replace this, with one that writes the multiple xattrs in a single 'set'
> call.
>
> Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxx>
> ---
> fs/ext3/xattr_security.c | 36 ++++++++++++++++++++----------------
> fs/ext4/xattr_security.c | 36 ++++++++++++++++++++----------------
> include/linux/security.h | 20 +++++++++++++++-----
> include/linux/xattr.h | 6 ++++++
> mm/shmem.c | 4 ++--
> security/security.c | 36 +++++++++++++++++++++++++++++++-----
> 6 files changed, 94 insertions(+), 44 deletions(-)
>
> diff --git a/fs/ext3/xattr_security.c b/fs/ext3/xattr_security.c
> index b8d9f83..1ceaad6d 100644
> --- a/fs/ext3/xattr_security.c
> +++ b/fs/ext3/xattr_security.c
> @@ -48,26 +48,30 @@ ext3_xattr_security_set(struct dentry *dentry, const char *name,
> name, value, size, flags);
> }
>
> +int ext3_initxattrs(struct inode *inode, struct xattr *xattr_array, int flag,
> + void *fs_info)
> +{
> + handle_t *handle = fs_info;
> + struct xattr *xattr;
> + int err = 0;
> +
> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
> + err = ext3_xattr_set_handle(handle, inode,
> + EXT3_XATTR_INDEX_SECURITY,
> + xattr->name, xattr->value,
> + xattr->value_len, 0);
> + if (err < 0)
> + break;
> + }
> + return err;
> +}
> +
> int
> ext3_init_security(handle_t *handle, struct inode *inode, struct inode *dir,
> const struct qstr *qstr)
> {
> - int err;
> - size_t len;
> - void *value;
> - char *name;
> -
> - err = security_inode_init_security(inode, dir, qstr, &name, &value, &len);
> - if (err) {
> - if (err == -EOPNOTSUPP)
> - return 0;
> - return err;
> - }
> - err = ext3_xattr_set_handle(handle, inode, EXT3_XATTR_INDEX_SECURITY,
> - name, value, len, 0);
> - kfree(name);
> - kfree(value);
> - return err;
> + return security_inode_init_security(inode, dir, qstr,
> + &ext3_initxattrs, handle);
> }
>
> const struct xattr_handler ext3_xattr_security_handler = {
> diff --git a/fs/ext4/xattr_security.c b/fs/ext4/xattr_security.c
> index 007c3bf..3a97aaa 100644
> --- a/fs/ext4/xattr_security.c
> +++ b/fs/ext4/xattr_security.c
> @@ -48,26 +48,30 @@ ext4_xattr_security_set(struct dentry *dentry, const char *name,
> name, value, size, flags);
> }
>
> +int ext4_initxattrs(struct inode *inode, struct xattr *xattr_array, int flag,
> + void *fs_info)
> +{
> + handle_t *handle = fs_info;
> + struct xattr *xattr;
> + int err = 0;
> +
> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
> + err = ext4_xattr_set_handle(handle, inode,
> + EXT4_XATTR_INDEX_SECURITY,
> + xattr->name, xattr->value,
> + xattr->value_len, 0);
> + if (err < 0)
> + break;
> + }
> + return err;
> +}
> +
> int
> ext4_init_security(handle_t *handle, struct inode *inode, struct inode *dir,
> const struct qstr *qstr)
> {
> - int err;
> - size_t len;
> - void *value;
> - char *name;
> -
> - err = security_inode_init_security(inode, dir, qstr, &name, &value, &len);
> - if (err) {
> - if (err == -EOPNOTSUPP)
> - return 0;
> - return err;
> - }
> - err = ext4_xattr_set_handle(handle, inode, EXT4_XATTR_INDEX_SECURITY,
> - name, value, len, 0);
> - kfree(name);
> - kfree(value);
> - return err;
> + return security_inode_init_security(inode, dir, qstr,
> + &ext4_initxattrs, handle);
> }
>
> const struct xattr_handler ext4_xattr_security_handler = {
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 8ce59ef..92cbf8d 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -36,6 +36,7 @@
> #include <linux/key.h>
> #include <linux/xfrm.h>
> #include <linux/slab.h>
> +#include <linux/xattr.h>
> #include <net/flow.h>
>
> /* Maximum number of letters for an LSM name string */
> @@ -1704,8 +1705,12 @@ int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
> int security_inode_alloc(struct inode *inode);
> void security_inode_free(struct inode *inode);
> int security_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> - void **value, size_t *len);
> + const struct qstr *qstr,
> + int (*initxattrs) (struct inode *inode,
> + struct xattr *xattr_array, int flag,
> + void *fs_data),
> + void *fs_data
> + );
> int security_inode_create(struct inode *dir, struct dentry *dentry, int mode);
> int security_inode_link(struct dentry *old_dentry, struct inode *dir,
> struct dentry *new_dentry);
> @@ -2035,9 +2040,14 @@ static inline void security_inode_free(struct inode *inode)
> static inline int security_inode_init_security(struct inode *inode,
> struct inode *dir,
> const struct qstr *qstr,
> - char **name,
> - void **value,
> - size_t *len)
> + struct xattr **xattr_array)
> + int (*initxattrs)
> + (struct inode *inode,
> + struct xattr *xattr_array,
> + int flag,
> + void *fs_data),
> + void *fs_data
> + );
> {
> return -EOPNOTSUPP;
> }
> diff --git a/include/linux/xattr.h b/include/linux/xattr.h
> index aed54c5..7a37866 100644
> --- a/include/linux/xattr.h
> +++ b/include/linux/xattr.h
> @@ -67,6 +67,12 @@ struct xattr_handler {
> size_t size, int flags, int handler_flags);
> };
>
> +struct xattr {
> + char *name;
> + void *value;
> + size_t value_len;
> +};
> +
> ssize_t xattr_getsecurity(struct inode *, const char *, void *, size_t);
> ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t);
> ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
> diff --git a/mm/shmem.c b/mm/shmem.c
> index d221a1c..518f27c 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1877,7 +1877,7 @@ shmem_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
> inode = shmem_get_inode(dir->i_sb, dir, mode, dev, VM_NORESERVE);
> if (inode) {
> error = security_inode_init_security(inode, dir,
> - &dentry->d_name, NULL,
> + &dentry->d_name,
> NULL, NULL);
> if (error) {
> if (error != -EOPNOTSUPP) {
> @@ -2017,7 +2017,7 @@ static int shmem_symlink(struct inode *dir, struct dentry *dentry, const char *s
> if (!inode)
> return -ENOSPC;
>
> - error = security_inode_init_security(inode, dir, &dentry->d_name, NULL,
> + error = security_inode_init_security(inode, dir, &dentry->d_name,
> NULL, NULL);
> if (error) {
> if (error != -EOPNOTSUPP) {
> diff --git a/security/security.c b/security/security.c
> index 4ba6d4c..c1ad1d2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -18,6 +18,8 @@
> #include <linux/security.h>
> #include <linux/ima.h>
>
> +#define MAX_LSM_XATTR 1
> +
> /* Boot-time LSM user choice */
> static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
> CONFIG_DEFAULT_SECURITY;
> @@ -339,13 +341,37 @@ void security_inode_free(struct inode *inode)
> }
>
> int security_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> - void **value, size_t *len)
> -{
> + const struct qstr *qstr,
> + int (*initxattrs) (struct inode *inode,
> + struct xattr *xattr_array, int flag,
> + void *fs_data),
> + void *fs_data
> + )
> +{
> + struct xattr new_xattrs[MAX_LSM_XATTR + 1];
> + struct xattr *lsm_xattr;
> + int ret;
> +
> if (unlikely(IS_PRIVATE(inode)))
> return -EOPNOTSUPP;
> - return security_ops->inode_init_security(inode, dir, qstr, name, value,
> - len);
> +
> + memset(new_xattrs, 0, sizeof new_xattrs);
> + if (!initxattrs)
> + return security_ops->inode_init_security(inode, dir, qstr,
> + NULL, NULL, NULL);
> + lsm_xattr = new_xattrs;
> + ret = security_ops->inode_init_security(inode, dir, qstr,
> + &lsm_xattr->name,
> + &lsm_xattr->value,
> + &lsm_xattr->value_len);
> + if (ret)
> + goto out;
> + ret = initxattrs(inode, new_xattrs, 0, fs_data);
> +out:
> + kfree(lsm_xattr->name);
> + kfree(lsm_xattr->value);
> +
> + return (ret == -EOPNOTSUPP) ? 0 : ret;
> }
> EXPORT_SYMBOL(security_inode_init_security);
>
Hi,
Option 1 and 2 are about the same - both use xattr array approach and
change API.
But callback option looks as cleaner approach to me, because it provides
a function which is used to set multiple xattrs...
- Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
