Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src - Fix a race between the kernel and libpthread, where a new ...



details:   https://anonhg.NetBSD.org/src/rev/4971a7183513
branches:  trunk
changeset: 1006726:4971a7183513
user:      ad <ad%NetBSD.org@localhost>
date:      Sat Jan 25 15:41:52 2020 +0000

description:
- Fix a race between the kernel and libpthread, where a new thread can start
  life without its self->pt_lid being filled in.

- Fix an error path in _lwp_create().  If the new LID can't be copied out,
  then get rid of the new LWP (i.e. either succeed or fail, not both).

- Mark l_dopreempt and l_nopreempt volatile in struct lwp.

diffstat:

 lib/libpthread/pthread.c           |  11 ++++-------
 sys/compat/netbsd32/netbsd32_lwp.c |  26 ++++++++++++++------------
 sys/kern/sys_lwp.c                 |  30 ++++++++++++++----------------
 sys/sys/lwp.h                      |  10 +++++-----
 4 files changed, 37 insertions(+), 40 deletions(-)

diffs (214 lines):

diff -r e6905fa87a44 -r 4971a7183513 lib/libpthread/pthread.c
--- a/lib/libpthread/pthread.c  Sat Jan 25 15:38:24 2020 +0000
+++ b/lib/libpthread/pthread.c  Sat Jan 25 15:41:52 2020 +0000
@@ -1,7 +1,8 @@
-/*     $NetBSD: pthread.c,v 1.154 2020/01/13 18:22:56 ad Exp $ */
+/*     $NetBSD: pthread.c,v 1.155 2020/01/25 15:41:52 ad Exp $ */
 
 /*-
- * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008 The NetBSD Foundation, Inc.
+ * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020
+ *     The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -30,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: pthread.c,v 1.154 2020/01/13 18:22:56 ad Exp $");
+__RCSID("$NetBSD: pthread.c,v 1.155 2020/01/25 15:41:52 ad Exp $");
 
 #define        __EXPOSE_STACK  1
 
@@ -571,10 +572,6 @@
         * thrash.  May help for SMT processors.  XXX We should not
         * be allocating stacks on fixed 2MB boundaries.  Needs a
         * thread register or decent thread local storage.
-        *
-        * Note that we may race with the kernel in _lwp_create(),
-        * and so pt_lid can be unset at this point, but we don't
-        * care.
         */
        (void)alloca(((unsigned)self->pt_lid & 7) << 8);
 
diff -r e6905fa87a44 -r 4971a7183513 sys/compat/netbsd32/netbsd32_lwp.c
--- a/sys/compat/netbsd32/netbsd32_lwp.c        Sat Jan 25 15:38:24 2020 +0000
+++ b/sys/compat/netbsd32/netbsd32_lwp.c        Sat Jan 25 15:41:52 2020 +0000
@@ -1,7 +1,7 @@
-/*     $NetBSD: netbsd32_lwp.c,v 1.19 2017/04/21 15:10:34 christos Exp $       */
+/*     $NetBSD: netbsd32_lwp.c,v 1.20 2020/01/25 15:41:52 ad Exp $     */
 
 /*
- *  Copyright (c) 2005, 2006, 2007 The NetBSD Foundation.
+ *  Copyright (c) 2005, 2006, 2007, 2020 The NetBSD Foundation.
  *  All rights reserved.
  *
  *  Redistribution and use in source and binary forms, with or without
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: netbsd32_lwp.c,v 1.19 2017/04/21 15:10:34 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: netbsd32_lwp.c,v 1.20 2020/01/25 15:41:52 ad Exp $");
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -55,7 +55,7 @@
        } */
        struct proc *p = l->l_proc;
        ucontext32_t *newuc = NULL;
-       lwpid_t lid;
+       lwp_t *l2;
        int error;
 
        KASSERT(p->p_emul->e_ucsize == sizeof(*newuc));
@@ -77,18 +77,20 @@
        const sigset_t *sigmask = newuc->uc_flags & _UC_SIGMASK ?
            &newuc->uc_sigmask : &l->l_sigmask;
 
-       error = do_lwp_create(l, newuc, SCARG(uap, flags), &lid, sigmask,
+       error = do_lwp_create(l, newuc, SCARG(uap, flags), &l2, sigmask,
            &SS_INIT);
-       if (error)
+       if (error != 0)
                goto fail;
 
-       /*
-        * do not free ucontext in case of an error here,
-        * the lwp will actually run and access it
-        */
-       return copyout(&lid, SCARG_P32(uap, new_lwp), sizeof(lid));
+       error = copyout(&l2->l_lid, SCARG_P32(uap, new_lwp),
+           sizeof(l2->l_lid));
+       if (error != 0)
+               lwp_exit(l2);
+       else
+               lwp_start(l2, SCARG(uap, flags));
+       return error;
 
-fail:
+ fail:
        kmem_free(newuc, sizeof(ucontext_t));
        return error;
 }
diff -r e6905fa87a44 -r 4971a7183513 sys/kern/sys_lwp.c
--- a/sys/kern/sys_lwp.c        Sat Jan 25 15:38:24 2020 +0000
+++ b/sys/kern/sys_lwp.c        Sat Jan 25 15:41:52 2020 +0000
@@ -1,7 +1,7 @@
-/*     $NetBSD: sys_lwp.c,v 1.71 2019/11/23 19:42:52 ad Exp $  */
+/*     $NetBSD: sys_lwp.c,v 1.72 2020/01/25 15:41:52 ad Exp $  */
 
 /*-
- * Copyright (c) 2001, 2006, 2007, 2008, 2019 The NetBSD Foundation, Inc.
+ * Copyright (c) 2001, 2006, 2007, 2008, 2019, 2020 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sys_lwp.c,v 1.71 2019/11/23 19:42:52 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sys_lwp.c,v 1.72 2020/01/25 15:41:52 ad Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -97,11 +97,10 @@
 }
 
 int
-do_lwp_create(lwp_t *l, void *arg, u_long flags, lwpid_t *new_lwp,
+do_lwp_create(lwp_t *l, void *arg, u_long flags, lwp_t **l2,
     const sigset_t *sigmask, const stack_t *sigstk)
 {
        struct proc *p = l->l_proc;
-       struct lwp *l2;
        vaddr_t uaddr;
        int error;
 
@@ -112,14 +111,12 @@
                return ENOMEM;
 
        error = lwp_create(l, p, uaddr, flags & LWP_DETACHED, NULL, 0,
-           mi_startlwp, arg, &l2, l->l_class, sigmask, &lwp_ss_init);
+           mi_startlwp, arg, l2, l->l_class, sigmask, &lwp_ss_init);
        if (__predict_false(error)) {
                uvm_uarea_free(uaddr);
                return error;
        }
 
-       *new_lwp = l2->l_lid;
-       lwp_start(l2, flags);
        return 0;
 }
 
@@ -134,7 +131,7 @@
        } */
        struct proc *p = l->l_proc;
        ucontext_t *newuc;
-       lwpid_t lid;
+       lwp_t *l2;
        int error;
 
        newuc = kmem_alloc(sizeof(ucontext_t), KM_SLEEP);
@@ -153,18 +150,19 @@
 
        const sigset_t *sigmask = newuc->uc_flags & _UC_SIGMASK ?
            &newuc->uc_sigmask : &l->l_sigmask;
-       error = do_lwp_create(l, newuc, SCARG(uap, flags), &lid, sigmask,
+       error = do_lwp_create(l, newuc, SCARG(uap, flags), &l2, sigmask,
            &SS_INIT);
        if (error)
                goto fail;
 
-       /*
-        * do not free ucontext in case of an error here,
-        * the lwp will actually run and access it
-        */
-       return copyout(&lid, SCARG(uap, new_lwp), sizeof(lid));
+       error = copyout(&l2->l_lid, SCARG(uap, new_lwp), sizeof(l2->l_lid));
+       if (error != 0)
+               lwp_exit(l2);
+       else
+               lwp_start(l2, SCARG(uap, flags));
+       return error;
 
-fail:
+ fail:
        kmem_free(newuc, sizeof(ucontext_t));
        return error;
 }
diff -r e6905fa87a44 -r 4971a7183513 sys/sys/lwp.h
--- a/sys/sys/lwp.h     Sat Jan 25 15:38:24 2020 +0000
+++ b/sys/sys/lwp.h     Sat Jan 25 15:41:52 2020 +0000
@@ -1,7 +1,7 @@
-/*     $NetBSD: lwp.h,v 1.197 2020/01/21 20:31:57 ad Exp $     */
+/*     $NetBSD: lwp.h,v 1.198 2020/01/25 15:41:52 ad Exp $     */
 
 /*
- * Copyright (c) 2001, 2006, 2007, 2008, 2009, 2010, 2019
+ * Copyright (c) 2001, 2006, 2007, 2008, 2009, 2010, 2019, 2020
  *    The NetBSD Foundation, Inc.
  * All rights reserved.
  *
@@ -186,8 +186,8 @@
        u_short         l_exlocks;      /* !: lockdebug: excl. locks held */
        u_short         l_psrefs;       /* !: count of psref held */
        u_short         l_blcnt;        /* !: count of kernel_lock held */
-       int             l_nopreempt;    /* !: don't preempt me! */
-       u_int           l_dopreempt;    /* s: kernel preemption pending */
+       volatile int    l_nopreempt;    /* !: don't preempt me! */
+       volatile u_int  l_dopreempt;    /* s: kernel preemption pending */
        int             l_pflag;        /* !: LWP private flags */
        int             l_dupfd;        /* !: side return from cloning devs XXX */
        const struct sysent * volatile l_sysent;/* !: currently active syscall */
@@ -352,7 +352,7 @@
 void   lwp_free(lwp_t *, bool, bool);
 uint64_t lwp_pctr(void);
 int    lwp_setprivate(lwp_t *, void *);
-int    do_lwp_create(lwp_t *, void *, u_long, lwpid_t *, const sigset_t *,
+int    do_lwp_create(lwp_t *, void *, u_long, lwp_t **, const sigset_t *,
     const stack_t *);
 
 void   lwpinit_specificdata(void);



Home | Main Index | Thread Index | Old Index