[Ocfs2-devel] [PATCH] xattr: Constify ->name member of "struct xattr".
Joel Becker
jlbec at evilplan.org
Sun Jun 9 02:07:23 PDT 2013
I can't really comment on the concept, but if security folks agree, the
ocfs2 part looks fine.
Reviewed-by: Joel Becker <jlbec at evilplan.org>
On Sat, Jun 08, 2013 at 09:54:56PM +0900, Tetsuo Handa wrote:
> >From 96df8b4be7f8702913a0245b2312958e2eea6caf Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel at I-love.SAKURA.ne.jp>
> Date: Sat, 8 Jun 2013 21:46:58 +0900
> Subject: [PATCH] xattr: Constify ->name member of "struct xattr".
>
> Since everybody sets kstrdup()ed constant string to "struct xattr"->name but
> nobody modifies "struct xattr"->name , we can omit kstrdup() and its failure
> checking by constifying ->name member of "struct xattr".
>
> Signed-off-by: Tetsuo Handa <penguin-kernel at I-love.SAKURA.ne.jp>
> ---
> fs/ocfs2/xattr.h | 2 +-
> include/linux/security.h | 8 ++++----
> include/linux/xattr.h | 2 +-
> include/uapi/linux/reiserfs_xattr.h | 2 +-
> security/capability.c | 2 +-
> security/integrity/evm/evm_main.c | 2 +-
> security/security.c | 8 +++-----
> security/selinux/hooks.c | 17 ++++++-----------
> security/smack/smack_lsm.c | 9 +++------
> 9 files changed, 21 insertions(+), 31 deletions(-)
>
> diff --git a/fs/ocfs2/xattr.h b/fs/ocfs2/xattr.h
> index e5c7f15..19f134e 100644
> --- a/fs/ocfs2/xattr.h
> +++ b/fs/ocfs2/xattr.h
> @@ -32,7 +32,7 @@ enum ocfs2_xattr_type {
>
> struct ocfs2_security_xattr_info {
> int enable;
> - char *name;
> + const char *name;
> void *value;
> size_t value_len;
> };
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 40560f4..2e64d73 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1466,7 +1466,7 @@ struct security_operations {
> int (*inode_alloc_security) (struct inode *inode);
> void (*inode_free_security) (struct inode *inode);
> int (*inode_init_security) (struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len);
> int (*inode_create) (struct inode *dir,
> struct dentry *dentry, umode_t mode);
> @@ -1737,7 +1737,7 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> const struct qstr *qstr,
> initxattrs initxattrs, void *fs_data);
> int security_old_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len);
> int security_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode);
> int security_inode_link(struct dentry *old_dentry, struct inode *dir,
> @@ -2048,8 +2048,8 @@ static inline int security_inode_init_security(struct inode *inode,
> static inline int security_old_inode_init_security(struct inode *inode,
> struct inode *dir,
> const struct qstr *qstr,
> - char **name, void **value,
> - size_t *len)
> + const char **name,
> + void **value, size_t *len)
> {
> return -EOPNOTSUPP;
> }
> diff --git a/include/linux/xattr.h b/include/linux/xattr.h
> index fdbafc6..91b0a68 100644
> --- a/include/linux/xattr.h
> +++ b/include/linux/xattr.h
> @@ -31,7 +31,7 @@ struct xattr_handler {
> };
>
> struct xattr {
> - char *name;
> + const char *name;
> void *value;
> size_t value_len;
> };
> diff --git a/include/uapi/linux/reiserfs_xattr.h b/include/uapi/linux/reiserfs_xattr.h
> index d8ce17c..38fdd64 100644
> --- a/include/uapi/linux/reiserfs_xattr.h
> +++ b/include/uapi/linux/reiserfs_xattr.h
> @@ -16,7 +16,7 @@ struct reiserfs_xattr_header {
> };
>
> struct reiserfs_security_handle {
> - char *name;
> + const char *name;
> void *value;
> size_t length;
> };
> diff --git a/security/capability.c b/security/capability.c
> index 1728d4e..b311ff0 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -119,7 +119,7 @@ static void cap_inode_free_security(struct inode *inode)
> }
>
> static int cap_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len)
> {
> return -EOPNOTSUPP;
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index cdbde17..2787080 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -405,7 +405,7 @@ int evm_inode_init_security(struct inode *inode,
>
> evm_xattr->value = xattr_data;
> evm_xattr->value_len = sizeof(*xattr_data);
> - evm_xattr->name = kstrdup(XATTR_EVM_SUFFIX, GFP_NOFS);
> + evm_xattr->name = XATTR_EVM_SUFFIX;
> return 0;
> out:
> kfree(xattr_data);
> diff --git a/security/security.c b/security/security.c
> index a3dce87..8849691 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -335,10 +335,10 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> if (unlikely(IS_PRIVATE(inode)))
> return 0;
>
> - memset(new_xattrs, 0, sizeof new_xattrs);
> if (!initxattrs)
> return security_ops->inode_init_security(inode, dir, qstr,
> NULL, NULL, NULL);
> + memset(new_xattrs, 0, sizeof(new_xattrs));
> lsm_xattr = new_xattrs;
> ret = security_ops->inode_init_security(inode, dir, qstr,
> &lsm_xattr->name,
> @@ -353,16 +353,14 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> goto out;
> ret = initxattrs(inode, new_xattrs, fs_data);
> out:
> - for (xattr = new_xattrs; xattr->name != NULL; xattr++) {
> - kfree(xattr->name);
> + for (xattr = new_xattrs; xattr->value != NULL; xattr++)
> kfree(xattr->value);
> - }
> return (ret == -EOPNOTSUPP) ? 0 : ret;
> }
> EXPORT_SYMBOL(security_inode_init_security);
>
> int security_old_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len)
> {
> if (unlikely(IS_PRIVATE(inode)))
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 5c6f2cd..16b8e51 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2516,7 +2516,8 @@ static void selinux_inode_free_security(struct inode *inode)
> }
>
> static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr,
> + const char **name,
> void **value, size_t *len)
> {
> const struct task_security_struct *tsec = current_security();
> @@ -2524,7 +2525,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> struct superblock_security_struct *sbsec;
> u32 sid, newsid, clen;
> int rc;
> - char *namep = NULL, *context;
> + char *context;
>
> dsec = dir->i_security;
> sbsec = dir->i_sb->s_security;
> @@ -2560,19 +2561,13 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> if (!ss_initialized || !(sbsec->flags & SE_SBLABELSUPP))
> return -EOPNOTSUPP;
>
> - if (name) {
> - namep = kstrdup(XATTR_SELINUX_SUFFIX, GFP_NOFS);
> - if (!namep)
> - return -ENOMEM;
> - *name = namep;
> - }
> + if (name)
> + *name = XATTR_SELINUX_SUFFIX;
>
> if (value && len) {
> rc = security_sid_to_context_force(newsid, &context, &clen);
> - if (rc) {
> - kfree(namep);
> + if (rc)
> return rc;
> - }
> *value = context;
> *len = clen;
> }
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index d52c780..46e5b47 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -554,7 +554,7 @@ static void smack_inode_free_security(struct inode *inode)
> * Returns 0 if it all works out, -ENOMEM if there's no memory
> */
> static int smack_inode_init_security(struct inode *inode, struct inode *dir,
> - const struct qstr *qstr, char **name,
> + const struct qstr *qstr, const char **name,
> void **value, size_t *len)
> {
> struct smack_known *skp;
> @@ -564,11 +564,8 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
> char *dsp = smk_of_inode(dir);
> int may;
>
> - if (name) {
> - *name = kstrdup(XATTR_SMACK_SUFFIX, GFP_NOFS);
> - if (*name == NULL)
> - return -ENOMEM;
> - }
> + if (name)
> + *name = XATTR_SMACK_SUFFIX;
>
> if (value) {
> skp = smk_find_entry(csp);
> --
> 1.7.1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
"Sometimes I think the surest sign intelligent
life exists elsewhere in the universe is that
none of it has tried to contact us."
-Calvin & Hobbes
http://www.jlbec.org/
jlbec at evilplan.org
More information about the Ocfs2-devel
mailing list