Module Name:    src
Committed By:   ad
Date:           Sat Jan 25 15:41:52 UTC 2020

Modified Files:
        src/lib/libpthread: pthread.c
        src/sys/compat/netbsd32: netbsd32_lwp.c
        src/sys/kern: sys_lwp.c
        src/sys/sys: lwp.h

Log Message:
- 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.


To generate a diff of this commit:
cvs rdiff -u -r1.154 -r1.155 src/lib/libpthread/pthread.c
cvs rdiff -u -r1.19 -r1.20 src/sys/compat/netbsd32/netbsd32_lwp.c
cvs rdiff -u -r1.71 -r1.72 src/sys/kern/sys_lwp.c
cvs rdiff -u -r1.197 -r1.198 src/sys/sys/lwp.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/lib/libpthread/pthread.c
diff -u src/lib/libpthread/pthread.c:1.154 src/lib/libpthread/pthread.c:1.155
--- src/lib/libpthread/pthread.c:1.154	Mon Jan 13 18:22:56 2020
+++ src/lib/libpthread/pthread.c	Sat Jan 25 15:41:52 2020
@@ -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 @@ pthread__create_tramp(void *cookie)
 	 * 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);
 

Index: src/sys/compat/netbsd32/netbsd32_lwp.c
diff -u src/sys/compat/netbsd32/netbsd32_lwp.c:1.19 src/sys/compat/netbsd32/netbsd32_lwp.c:1.20
--- src/sys/compat/netbsd32/netbsd32_lwp.c:1.19	Fri Apr 21 15:10:34 2017
+++ src/sys/compat/netbsd32/netbsd32_lwp.c	Sat Jan 25 15:41:52 2020
@@ -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 @@ netbsd32__lwp_create(struct lwp *l, cons
 	} */
 	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 @@ netbsd32__lwp_create(struct lwp *l, cons
 	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;
 }

Index: src/sys/kern/sys_lwp.c
diff -u src/sys/kern/sys_lwp.c:1.71 src/sys/kern/sys_lwp.c:1.72
--- src/sys/kern/sys_lwp.c:1.71	Sat Nov 23 19:42:52 2019
+++ src/sys/kern/sys_lwp.c	Sat Jan 25 15:41:52 2020
@@ -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 @@ mi_startlwp(void *arg)
 }
 
 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 @@ do_lwp_create(lwp_t *l, void *arg, u_lon
 		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 @@ sys__lwp_create(struct lwp *l, const str
 	} */
 	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 @@ sys__lwp_create(struct lwp *l, const str
 
 	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;
 }

Index: src/sys/sys/lwp.h
diff -u src/sys/sys/lwp.h:1.197 src/sys/sys/lwp.h:1.198
--- src/sys/sys/lwp.h:1.197	Tue Jan 21 20:31:57 2020
+++ src/sys/sys/lwp.h	Sat Jan 25 15:41:52 2020
@@ -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 @@ struct lwp {
 	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_need_userret(lwp_t *);
 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);

Reply via email to