linux-kernel - Re: [PATCH 1/3] ipc: convert ipc_namespace.count from atomic_t to refcount

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 for Android: free password hash cracker in your pocket

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

Date:   Sun, 09 Jul 2017 16:59:55 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Elena Reshetova <elena.reshetova@...el.com>
Cc:     linux-kernel@...r.kernel.org, peterz@...radead.org,
        gregkh@...uxfoundation.org, akpm@...ux-foundation.org,
        mingo@...hat.com, adobriyan@...il.com, serge@...lyn.com,
        arozansk@...hat.com, dave@...olabs.net, keescook@...omium.org,
        Hans Liljestrand <ishkamiel@...il.com>,
        David Windsor <dwindsor@...il.com>
Subject: Re: [PATCH 1/3] ipc: convert ipc_namespace.count from atomic_t to refcount_t

Elena Reshetova <elena.reshetova@...el.com> writes:

> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.

In this patch you can see all of the uses of the count.
What accidental refcount overflows are possible?

Eric

> Signed-off-by: Elena Reshetova <elena.reshetova@...el.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@...il.com>
> Signed-off-by: Kees Cook <keescook@...omium.org>
> Signed-off-by: David Windsor <dwindsor@...il.com>
> ---
>  include/linux/ipc_namespace.h | 5 +++--
>  ipc/msgutil.c                 | 2 +-
>  ipc/namespace.c               | 4 ++--
>  3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
> index 65327ee..e81445c 100644
> --- a/include/linux/ipc_namespace.h
> +++ b/include/linux/ipc_namespace.h
> @@ -7,6 +7,7 @@
>  #include <linux/notifier.h>
>  #include <linux/nsproxy.h>
>  #include <linux/ns_common.h>
> +#include <linux/refcount.h>
>  
>  struct user_namespace;
>  
> @@ -19,7 +20,7 @@ struct ipc_ids {
>  };
>  
>  struct ipc_namespace {
> -	atomic_t	count;
> +	refcount_t	count;
>  	struct ipc_ids	ids[3];
>  
>  	int		sem_ctls[4];
> @@ -118,7 +119,7 @@ extern struct ipc_namespace *copy_ipcs(unsigned long flags,
>  static inline struct ipc_namespace *get_ipc_ns(struct ipc_namespace *ns)
>  {
>  	if (ns)
> -		atomic_inc(&ns->count);
> +		refcount_inc(&ns->count);
>  	return ns;
>  }
>  
> diff --git a/ipc/msgutil.c b/ipc/msgutil.c
> index bf74eaa..8459802 100644
> --- a/ipc/msgutil.c
> +++ b/ipc/msgutil.c
> @@ -29,7 +29,7 @@ DEFINE_SPINLOCK(mq_lock);
>   * and not CONFIG_IPC_NS.
>   */
>  struct ipc_namespace init_ipc_ns = {
> -	.count		= ATOMIC_INIT(1),
> +	.count		= REFCOUNT_INIT(1),
>  	.user_ns = &init_user_ns,
>  	.ns.inum = PROC_IPC_INIT_INO,
>  #ifdef CONFIG_IPC_NS
> diff --git a/ipc/namespace.c b/ipc/namespace.c
> index b4d80f9..7af6e6b 100644
> --- a/ipc/namespace.c
> +++ b/ipc/namespace.c
> @@ -50,7 +50,7 @@ static struct ipc_namespace *create_ipc_ns(struct user_namespace *user_ns,
>  		goto fail_free;
>  	ns->ns.ops = &ipcns_operations;
>  
> -	atomic_set(&ns->count, 1);
> +	refcount_set(&ns->count, 1);
>  	ns->user_ns = get_user_ns(user_ns);
>  	ns->ucounts = ucounts;
>  
> @@ -144,7 +144,7 @@ static void free_ipc_ns(struct ipc_namespace *ns)
>   */
>  void put_ipc_ns(struct ipc_namespace *ns)
>  {
> -	if (atomic_dec_and_lock(&ns->count, &mq_lock)) {
> +	if (refcount_dec_and_lock(&ns->count, &mq_lock)) {
>  		mq_clear_sbinfo(ns);
>  		spin_unlock(&mq_lock);
>  		mq_put_mnt(ns);