linux-kernel - Re: [RFC v3 04/45] vfs: Shrink struct posix

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]

Date:	Sat, 25 Apr 2015 09:56:01 +0200
From:	Rasmus Villemoes <linux@...musvillemoes.dk>
To:	Andreas Gruenbacher <andreas.gruenbacher@...il.com>
Cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	linux-nfs@...r.kernel.org
Subject: Re: [RFC v3 04/45] vfs: Shrink struct posix_acl

On Fri, Apr 24 2015, Andreas Gruenbacher <andreas.gruenbacher@...il.com> wrote:

> There is a hole in struct posix_acl because its struct rcu_head member is too
> large; at least on on 64-bit architectures, the hole cannot be closed by
> changing the definition of struct posix_acl. So instead, remove the struct
> rcu_head member from struct posix_acl, make sure that acls are always big
> enough to fit a struct rcu_head, and cast to struct rcu_head * when disposing
> of an acl.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@...hat.com>
> ---
>  fs/posix_acl.c            | 5 +++--
>  include/linux/posix_acl.h | 7 ++-----
>  2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> index 3a48bb7..efe983e 100644
> --- a/fs/posix_acl.c
> +++ b/fs/posix_acl.c
> @@ -140,8 +140,9 @@ EXPORT_SYMBOL(posix_acl_init);
>  struct posix_acl *
>  posix_acl_alloc(int count, gfp_t flags)
>  {
> -	const size_t size = sizeof(struct posix_acl) +
> -	                    count * sizeof(struct posix_acl_entry);
> +	const size_t size = max(sizeof(struct rcu_head),
> +		sizeof(struct posix_acl) +
> +		count * sizeof(struct posix_acl_entry));
>  	struct posix_acl *acl = kmalloc(size, flags);
>  	if (acl)
>  		posix_acl_init(acl, count);
> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
> index 3e96a6a..66cf477 100644
> --- a/include/linux/posix_acl.h
> +++ b/include/linux/posix_acl.h
> @@ -43,10 +43,7 @@ struct posix_acl_entry {
>  };
>  
>  struct posix_acl {
> -	union {
> -		atomic_t		a_refcount;
> -		struct rcu_head		a_rcu;
> -	};
> +	atomic_t		a_refcount;
>  	unsigned int		a_count;
>  	struct posix_acl_entry	a_entries[0];
>  };
> @@ -73,7 +70,7 @@ static inline void
>  posix_acl_release(struct posix_acl *acl)
>  {
>  	if (acl && atomic_dec_and_test(&acl->a_refcount))
> -		kfree_rcu(acl, a_rcu);
> +		__kfree_rcu((struct rcu_head *)acl, 0);
>  }

This doesn't seem right. Wouldn't that scribble over the a_count and
part of the first struct posix_acl_entry while there might still be rcu
users? That might blow up in interesting ways...

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/