Quoting David P. Quigley (dpquigl@xxxxxxxxxxxxx):
> This patch modifies the interface to inode_getsecurity to have the function
> return a buffer containing the security blob and its length via parameters
> instead of relying on the calling function to give it an appropriately sized
> buffer. Security blobs obtained with this function should be freed using the
> release_secctx LSM hook. This alleviates the problem of the caller having to
> guess a length and preallocate a buffer for this function allowing it to be
> used elsewhere for Labeled NFS. The patch also removed the unused err
> parameter. The conversion is similar to the one performed by Al Viro for the
> security_getprocattr hook.
>
> Signed-off-by: David P. Quigley <dpquigl@xxxxxxxxxxxxx>
Looks good. Looks like it's already hit -mm, but anyway
Acked-by: Serge Hallyn <serue@xxxxxxxxxx>
thanks,
-serge
> ---
> fs/xattr.c | 30 ++++++++++++++++++++++++++++--
> include/linux/security.h | 21 +++++++++------------
> include/linux/xattr.h | 1 +
> mm/shmem.c | 3 +--
> security/dummy.c | 2 +-
> security/security.c | 4 ++--
> security/selinux/hooks.c | 45 ++++++++++++++++-----------------------------
> 7 files changed, 58 insertions(+), 48 deletions(-)
>
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 6645b73..56b5b88 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -105,6 +105,33 @@ out:
> EXPORT_SYMBOL_GPL(vfs_setxattr);
>
> ssize_t
> +xattr_getsecurity(struct inode *inode, const char *name, void *value,
> + size_t size)
> +{
> + void *buffer = NULL;
> + ssize_t len;
> +
> + if (!value || !size) {
> + len = security_inode_getsecurity(inode, name, &buffer, false);
> + goto out_noalloc;
> + }
> +
> + len = security_inode_getsecurity(inode, name, &buffer, true);
> + if (len < 0)
> + return len;
> + if (size < len) {
> + len = -ERANGE;
> + goto out;
> + }
> + memcpy(value, buffer, len);
> +out:
> + security_release_secctx(buffer, len);
> +out_noalloc:
> + return len;
> +}
> +EXPORT_SYMBOL_GPL(xattr_getsecurity);
> +
> +ssize_t
> vfs_getxattr(struct dentry *dentry, char *name, void *value, size_t size)
> {
> struct inode *inode = dentry->d_inode;
> @@ -126,8 +153,7 @@ vfs_getxattr(struct dentry *dentry, char *name, void *value, size_t size)
> if (!strncmp(name, XATTR_SECURITY_PREFIX,
> XATTR_SECURITY_PREFIX_LEN)) {
> const char *suffix = name + XATTR_SECURITY_PREFIX_LEN;
> - int ret = security_inode_getsecurity(inode, suffix, value,
> - size, error);
> + int ret = xattr_getsecurity(inode, suffix, value, size);
> /*
> * Only overwrite the return value if a security module
> * is actually active.
> diff --git a/include/linux/security.h b/include/linux/security.h
> index ac05083..3c4c91e 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -404,15 +404,12 @@ struct request_sock;
> * identified by @name for @dentry.
> * Return 0 if permission is granted.
> * @inode_getsecurity:
> - * Copy the extended attribute representation of the security label
> - * associated with @name for @inode into @buffer. @buffer may be
> - * NULL to request the size of the buffer required. @size indicates
> - * the size of @buffer in bytes. Note that @name is the remainder
> - * of the attribute name after the security. prefix has been removed.
> - * @err is the return value from the preceding fs getxattr call,
> - * and can be used by the security module to determine whether it
> - * should try and canonicalize the attribute value.
> - * Return number of bytes used/required on success.
> + * Retrieve a copy of the extended attribute representation of the
> + * security label associated with @name for @inode via @buffer. Note that
> + * @name is the remainder of the attribute name after the security prefix
> + * has been removed. @alloc is used to specify of the call should return a
> + * value via the buffer or just the value length Return size of buffer on
> + * success.
> * @inode_setsecurity:
> * Set the security label associated with @name for @inode from the
> * extended attribute value @value. @size indicates the size of the
> @@ -1275,7 +1272,7 @@ struct security_operations {
> int (*inode_removexattr) (struct dentry *dentry, char *name);
> int (*inode_need_killpriv) (struct dentry *dentry);
> int (*inode_killpriv) (struct dentry *dentry);
> - int (*inode_getsecurity)(const struct inode *inode, const char *name, void *buffer, size_t size, int err);
> + int (*inode_getsecurity)(const struct inode *inode, const char *name, void **buffer, bool alloc);
> int (*inode_setsecurity)(struct inode *inode, const char *name, const void *value, size_t size, int flags);
> int (*inode_listsecurity)(struct inode *inode, char *buffer, size_t buffer_size);
>
> @@ -1529,7 +1526,7 @@ int security_inode_listxattr(struct dentry *dentry);
> int security_inode_removexattr(struct dentry *dentry, char *name);
> int security_inode_need_killpriv(struct dentry *dentry);
> int security_inode_killpriv(struct dentry *dentry);
> -int security_inode_getsecurity(const struct inode *inode, const char *name, void *buffer, size_t size, int err);
> +int security_inode_getsecurity(const struct inode *inode, const char *name, void **buffer, bool alloc);
> int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags);
> int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size);
> int security_file_permission(struct file *file, int mask);
> @@ -1933,7 +1930,7 @@ static inline int security_inode_killpriv(struct dentry *dentry)
> return cap_inode_killpriv(dentry);
> }
>
> -static inline int security_inode_getsecurity(const struct inode *inode, const char *name, void *buffer, size_t size, int err)
> +static inline int security_inode_getsecurity(const struct inode *inode, const char *name, void **buffer, bool alloc)
> {
> return -EOPNOTSUPP;
> }
> diff --git a/include/linux/xattr.h b/include/linux/xattr.h
> index def131a..df6b95d 100644
> --- a/include/linux/xattr.h
> +++ b/include/linux/xattr.h
> @@ -46,6 +46,7 @@ struct xattr_handler {
> size_t size, int flags);
> };
>
> +ssize_t xattr_getsecurity(struct inode *, const char *, void *, size_t);
> ssize_t vfs_getxattr(struct dentry *, char *, void *, size_t);
> ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
> int vfs_setxattr(struct dentry *, char *, void *, size_t, int);
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 404e53b..4c53460 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1980,8 +1980,7 @@ static int shmem_xattr_security_get(struct inode *inode, const char *name,
> {
> if (strcmp(name, "") == 0)
> return -EINVAL;
> - return security_inode_getsecurity(inode, name, buffer, size,
> - -EOPNOTSUPP);
> + return xattr_getsecurity(inode, name, buffer, size);
> }
>
> static int shmem_xattr_security_set(struct inode *inode, const char *name,
> diff --git a/security/dummy.c b/security/dummy.c
> index 6d895ad..7de65dc 100644
> --- a/security/dummy.c
> +++ b/security/dummy.c
> @@ -384,7 +384,7 @@ static int dummy_inode_killpriv(struct dentry *dentry)
> return 0;
> }
>
> -static int dummy_inode_getsecurity(const struct inode *inode, const char *name, void *buffer, size_t size, int err)
> +static int dummy_inode_getsecurity(const struct inode *inode, const char *name, void **buffer, bool alloc)
> {
> return -EOPNOTSUPP;
> }
> diff --git a/security/security.c b/security/security.c
> index 0e1f1f1..39de3f4 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -478,11 +478,11 @@ int security_inode_killpriv(struct dentry *dentry)
> return security_ops->inode_killpriv(dentry);
> }
>
> -int security_inode_getsecurity(const struct inode *inode, const char *name, void *buffer, size_t size, int err)
> +int security_inode_getsecurity(const struct inode *inode, const char *name, void **buffer, bool alloc)
> {
> if (unlikely(IS_PRIVATE(inode)))
> return 0;
> - return security_ops->inode_getsecurity(inode, name, buffer, size, err);
> + return security_ops->inode_getsecurity(inode, name, buffer, alloc);
> }
>
> int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9f3124b..2ae7766 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -127,32 +127,6 @@ static DEFINE_SPINLOCK(sb_security_lock);
>
> static struct kmem_cache *sel_inode_cache;
>
> -/* Return security context for a given sid or just the context
> - length if the buffer is null or length is 0 */
> -static int selinux_getsecurity(u32 sid, void *buffer, size_t size)
> -{
> - char *context;
> - unsigned len;
> - int rc;
> -
> - rc = security_sid_to_context(sid, &context, &len);
> - if (rc)
> - return rc;
> -
> - if (!buffer || !size)
> - goto getsecurity_exit;
> -
> - if (size < len) {
> - len = -ERANGE;
> - goto getsecurity_exit;
> - }
> - memcpy(buffer, context, len);
> -
> -getsecurity_exit:
> - kfree(context);
> - return len;
> -}
> -
> /* Allocate and free functions for each kind of security blob. */
>
> static int task_alloc_security(struct task_struct *task)
> @@ -2416,14 +2390,27 @@ static int selinux_inode_removexattr (struct dentry *dentry, char *name)
> *
> * Permission check is handled by selinux_inode_getxattr hook.
> */
> -static int selinux_inode_getsecurity(const struct inode *inode, const char *name, void *buffer, size_t size, int err)
> +static int selinux_inode_getsecurity(const struct inode *inode, const char *name, void **buffer, bool alloc)
> {
> + u32 size;
> + int error;
> + char *context = NULL;
> struct inode_security_struct *isec = inode->i_security;
>
> if (strcmp(name, XATTR_SELINUX_SUFFIX))
> return -EOPNOTSUPP;
> -
> - return selinux_getsecurity(isec->sid, buffer, size);
> +
> + error = security_sid_to_context(isec->sid, &context, &size);
> + if (error)
> + return error;
> + error = size;
> + if (alloc) {
> + *buffer = context;
> + goto out_nofree;
> + }
> + kfree(context);
> +out_nofree:
> + return error;
> }
>
> static int selinux_inode_setsecurity(struct inode *inode, const char *name,
> --
> 1.5.3.4
>
-
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
