From: Serge Hallyn <serge.hallyn@ubuntu.com>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: ocfs2-devel@oss.oracle.com, reiserfs-devel@vger.kernel.org,
eparis@parisplace.org, casey@schaufler-ca.com, zohar@us.ibm.com,
serge.hallyn@canonical.com, linux-fsdevel@vger.kernel.org,
linux-security-module@vger.kernel.org
Subject: Re: [PATCH] xattr: Constify ->name member of "struct xattr".
Date: Mon, 10 Jun 2013 06:54:18 -0500 [thread overview]
Message-ID: <20130610115417.GB13953@tp> (raw)
In-Reply-To: <201306082154.BDJ95357.MFQFVOJLSOtHFO@I-love.SAKURA.ne.jp>
Quoting Tetsuo Handa (penguin-kernel@I-love.SAKURA.ne.jp):
> >From 96df8b4be7f8702913a0245b2312958e2eea6caf Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@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@I-love.SAKURA.ne.jp>
It seems good to me.
Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>
> ---
> 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
next prev parent reply other threads:[~2013-06-10 11:54 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-29 12:52 [RFC] xattr: Constify ->name member of "struct xattr" Tetsuo Handa
2013-06-08 12:54 ` [PATCH] " Tetsuo Handa
2013-06-09 9:07 ` Joel Becker
2013-06-09 12:09 ` Tetsuo Handa
2013-06-24 13:05 ` Tetsuo Handa
2013-06-10 11:54 ` Serge Hallyn [this message]
2013-06-10 15:53 ` Casey Schaufler
2013-07-23 18:27 ` Paul Moore
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130610115417.GB13953@tp \
--to=serge.hallyn@ubuntu.com \
--cc=casey@schaufler-ca.com \
--cc=eparis@parisplace.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=ocfs2-devel@oss.oracle.com \
--cc=penguin-kernel@I-love.SAKURA.ne.jp \
--cc=reiserfs-devel@vger.kernel.org \
--cc=serge.hallyn@canonical.com \
--cc=zohar@us.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).