linux-kernel - Re: [PATCH 2/3] ipc: convert sem_undo_list.refcnt 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAGXu5jK7VUUaSEERbiEgwe1OnhGe+j0nO83u7OPnUSV2O_aeMQ@mail.gmail.com>
Date:   Sat, 27 May 2017 12:44:20 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Manfred Spraul <manfred@...orfullife.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Elena Reshetova <elena.reshetova@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Ingo Molnar <mingo@...hat.com>,
        Alexey Dobriyan <adobriyan@...il.com>,
        "Serge E. Hallyn" <serge@...lyn.com>, arozansk@...hat.com,
        Davidlohr Bueso <dave@...olabs.net>,
        Hans Liljestrand <ishkamiel@...il.com>,
        David Windsor <dwindsor@...il.com>
Subject: Re: [PATCH 2/3] ipc: convert sem_undo_list.refcnt from atomic_t to refcount_t

On Mon, Feb 20, 2017 at 3:29 AM, Elena Reshetova
<elena.reshetova@...el.com> wrote:
> 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.
>
> 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>

Here's the 2/3 that should go via -mm, I think too.

-Kees

> ---
>  ipc/sem.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/ipc/sem.c b/ipc/sem.c
> index e468cd1..9063ffa 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -139,7 +139,7 @@ struct sem_undo {
>   * that may be shared among all a CLONE_SYSVSEM task group.
>   */
>  struct sem_undo_list {
> -       atomic_t                refcnt;
> +       refcount_t              refcnt;
>         spinlock_t              lock;
>         struct list_head        list_proc;
>  };
> @@ -1646,7 +1646,7 @@ static inline int get_undo_list(struct sem_undo_list **undo_listp)
>                 if (undo_list == NULL)
>                         return -ENOMEM;
>                 spin_lock_init(&undo_list->lock);
> -               atomic_set(&undo_list->refcnt, 1);
> +               refcount_set(&undo_list->refcnt, 1);
>                 INIT_LIST_HEAD(&undo_list->list_proc);
>
>                 current->sysvsem.undo_list = undo_list;
> @@ -2045,7 +2045,7 @@ int copy_semundo(unsigned long clone_flags, struct task_struct *tsk)
>                 error = get_undo_list(&undo_list);
>                 if (error)
>                         return error;
> -               atomic_inc(&undo_list->refcnt);
> +               refcount_inc(&undo_list->refcnt);
>                 tsk->sysvsem.undo_list = undo_list;
>         } else
>                 tsk->sysvsem.undo_list = NULL;
> @@ -2074,7 +2074,7 @@ void exit_sem(struct task_struct *tsk)
>                 return;
>         tsk->sysvsem.undo_list = NULL;
>
> -       if (!atomic_dec_and_test(&ulp->refcnt))
> +       if (!refcount_dec_and_test(&ulp->refcnt))
>                 return;
>
>         for (;;) {
> --
> 2.7.4
>



-- 
Kees Cook
Pixel Security