linux-kernel - Re: [PATCH v2 2/4] kernfs: Add removed_size out param for simple_xattr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <C17D34QG2G67.2DUOCLCXQI4RP@dlxu-fedora-R90QNFJV>
Date:   Tue, 10 Mar 2020 11:20:44 -0700
From:   "Daniel Xu" <dxu@...uu.xyz>
To:     "Daniel Xu" <dxu@...uu.xyz>, <cgroups@...r.kernel.org>,
        <tj@...nel.org>, <lizefan@...wei.com>, <hannes@...xchg.org>,
        <viro@...iv.linux.org.uk>
Cc:     "Daniel Xu" <dxu@...uu.xyz>, <shakeelb@...gle.com>,
        <linux-kernel@...r.kernel.org>, <gregkh@...uxfoundation.org>,
        <kernel-team@...com>
Subject: Re: [PATCH v2 2/4] kernfs: Add removed_size out param for
 simple_xattr_set

On Thu Mar 5, 2020 at 1:16 PM, Daniel Xu wrote:
> This helps set up size accounting in the next commit. Without this out
> param, it's difficult to find out the removed xattr size without taking
> a lock for longer and walking the xattr linked list twice.
>
> 
> Signed-off-by: Daniel Xu <dxu@...uu.xyz>
> ---
> fs/kernfs/inode.c | 2 +-
> fs/xattr.c | 11 ++++++++++-
> include/linux/xattr.h | 3 ++-
> mm/shmem.c | 2 +-
> 4 files changed, 14 insertions(+), 4 deletions(-)
>
> 
> diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> index d0f7a5abd9a9..5f10ae95fbfa 100644
> --- a/fs/kernfs/inode.c
> +++ b/fs/kernfs/inode.c
> @@ -303,7 +303,7 @@ int kernfs_xattr_set(struct kernfs_node *kn, const
> char *name,
> if (!attrs)
> return -ENOMEM;
>  
> - return simple_xattr_set(&attrs->xattrs, name, value, size, flags);
> + return simple_xattr_set(&attrs->xattrs, name, value, size, flags,
> NULL);
> }
>  
> static int kernfs_vfs_xattr_get(const struct xattr_handler *handler,
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 0d3c9b4d1914..e13265e65871 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -860,6 +860,7 @@ int simple_xattr_get(struct simple_xattrs *xattrs,
> const char *name,
> * @value: value of the xattr. If %NULL, will remove the attribute.
> * @size: size of the new xattr
> * @flags: %XATTR_{CREATE|REPLACE}
> + * @removed_size: returns size of the removed xattr, -1 if none removed
> *
> * %XATTR_CREATE is set, the xattr shouldn't exist already; otherwise
> fails
> * with -EEXIST. If %XATTR_REPLACE is set, the xattr should exist;
> @@ -868,7 +869,8 @@ int simple_xattr_get(struct simple_xattrs *xattrs,
> const char *name,
> * Returns 0 on success, -errno on failure.
> */
> int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
> - const void *value, size_t size, int flags)
> + const void *value, size_t size, int flags,
> + ssize_t *removed_size)
> {
> struct simple_xattr *xattr;
> struct simple_xattr *new_xattr = NULL;
> @@ -895,8 +897,12 @@ int simple_xattr_set(struct simple_xattrs *xattrs,
> const char *name,
> err = -EEXIST;
> } else if (new_xattr) {
> list_replace(&xattr->list, &new_xattr->list);
> + if (removed_size)
> + *removed_size = xattr->size;
> } else {
> list_del(&xattr->list);
> + if (removed_size)
> + *removed_size = xattr->size;
> }
> goto out;
> }
> @@ -908,6 +914,9 @@ int simple_xattr_set(struct simple_xattrs *xattrs,
> const char *name,
> list_add(&new_xattr->list, &xattrs->head);
> xattr = NULL;
> }
> +
> + if (removed_size)
> + *removed_size = -1;
> out:
> spin_unlock(&xattrs->lock);
> if (xattr) {
> diff --git a/include/linux/xattr.h b/include/linux/xattr.h
> index 6dad031be3c2..4cf6e11f4a3c 100644
> --- a/include/linux/xattr.h
> +++ b/include/linux/xattr.h
> @@ -102,7 +102,8 @@ struct simple_xattr *simple_xattr_alloc(const void
> *value, size_t size);
> int simple_xattr_get(struct simple_xattrs *xattrs, const char *name,
> void *buffer, size_t size);
> int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
> - const void *value, size_t size, int flags);
> + const void *value, size_t size, int flags,
> + ssize_t *removed_size);
> ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs
> *xattrs, char *buffer,
> size_t size);
> void simple_xattr_list_add(struct simple_xattrs *xattrs,
> diff --git a/mm/shmem.c b/mm/shmem.c
> index aad3ba74b0e9..f47347cb30f6 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -3243,7 +3243,7 @@ static int shmem_xattr_handler_set(const struct
> xattr_handler *handler,
> struct shmem_inode_info *info = SHMEM_I(inode);
>  
> name = xattr_full_name(handler, name);
> - return simple_xattr_set(&info->xattrs, name, value, size, flags);
> + return simple_xattr_set(&info->xattrs, name, value, size, flags,
> NULL);
> }
>  
> static const struct xattr_handler shmem_security_xattr_handler = {
> --
> 2.21.1
>
> 

Adding Al Viro, who I forgot to add in the initial send. Will remember
on future sends.

Daniel