Module Name:    src
Committed By:   snj
Date:           Thu Jul  6 15:18:23 UTC 2017

Modified Files:
        src/sys/kern [netbsd-6-0]: subr_xcall.c

Log Message:
Pull up following revision(s) (requested by ozaki-r in ticket #1419):
        sys/kern/subr_xcall.c: revision 1.19
Fix a race condition of low priority xcall
xc_lowpri and xc_thread are racy and xc_wait may return during/before
executing all xcall callbacks, resulting in a kernel panic at worst.
xc_lowpri serializes multiple jobs by a mutex and a cv. If all xcall
callbacks are done, xc_wait returns and also xc_lowpri accepts a next job.
The problem is that a counter that counts the number of finished xcall
callbacks is incremented *before* actually executing a xcall callback
(see xc_tailp++ in xc_thread). So xc_lowpri accepts a next job before
all xcall callbacks complete and a next job begins to run its xcall callbacks.
Even worse the counter is global and shared between jobs, so if a xcall
callback of the next job completes, the shared counter is incremented,
which confuses wc_wait of the previous job as all xcall callbacks of the
previous job are done and wc_wait of the previous job returns during/before
executing its xcall callbacks.
How to fix: there are actually two counters that count the number of finished
xcall callbacks for low priority xcall for historical reasons (I guess):
xc_tailp and xc_low_pri.xc_donep. xc_low_pri.xc_donep is incremented correctly
while xc_tailp is incremented wrongly, i.e., before executing a xcall callback.
We can fix the issue by dropping xc_tailp and using only xc_low_pri.xc_donep.
PR kern/51632


To generate a diff of this commit:
cvs rdiff -u -r1.13.16.1 -r1.13.16.2 src/sys/kern/subr_xcall.c

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

Modified files:

Index: src/sys/kern/subr_xcall.c
diff -u src/sys/kern/subr_xcall.c:1.13.16.1 src/sys/kern/subr_xcall.c:1.13.16.2
--- src/sys/kern/subr_xcall.c:1.13.16.1	Sat Apr 20 10:05:44 2013
+++ src/sys/kern/subr_xcall.c	Thu Jul  6 15:18:23 2017
@@ -1,4 +1,4 @@
-/*	$NetBSD: subr_xcall.c,v 1.13.16.1 2013/04/20 10:05:44 bouyer Exp $	*/
+/*	$NetBSD: subr_xcall.c,v 1.13.16.2 2017/07/06 15:18:23 snj Exp $	*/
 
 /*-
  * Copyright (c) 2007-2010 The NetBSD Foundation, Inc.
@@ -74,7 +74,7 @@
  */
  
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: subr_xcall.c,v 1.13.16.1 2013/04/20 10:05:44 bouyer Exp $");
+__KERNEL_RCSID(0, "$NetBSD: subr_xcall.c,v 1.13.16.2 2017/07/06 15:18:23 snj Exp $");
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -101,7 +101,6 @@ typedef struct {
 
 /* Low priority xcall structures. */
 static xc_state_t	xc_low_pri	__cacheline_aligned;
-static uint64_t		xc_tailp	__cacheline_aligned;
 
 /* High priority xcall structures. */
 static xc_state_t	xc_high_pri	__cacheline_aligned;
@@ -131,7 +130,6 @@ xc_init(void)
 	memset(xclo, 0, sizeof(xc_state_t));
 	mutex_init(&xclo->xc_lock, MUTEX_DEFAULT, IPL_NONE);
 	cv_init(&xclo->xc_busy, "xclocv");
-	xc_tailp = 0;
 
 	memset(xchi, 0, sizeof(xc_state_t));
 	mutex_init(&xchi->xc_lock, MUTEX_DEFAULT, IPL_SOFTCLOCK);
@@ -253,7 +251,7 @@ xc_lowpri(xcfunc_t func, void *arg1, voi
 	uint64_t where;
 
 	mutex_enter(&xc->xc_lock);
-	while (xc->xc_headp != xc_tailp) {
+	while (xc->xc_headp != xc->xc_donep) {
 		cv_wait(&xc->xc_busy, &xc->xc_lock);
 	}
 	xc->xc_arg1 = arg1;
@@ -274,7 +272,7 @@ xc_lowpri(xcfunc_t func, void *arg1, voi
 		ci->ci_data.cpu_xcall_pending = true;
 		cv_signal(&ci->ci_data.cpu_xcall);
 	}
-	KASSERT(xc_tailp < xc->xc_headp);
+	KASSERT(xc->xc_donep < xc->xc_headp);
 	where = xc->xc_headp;
 	mutex_exit(&xc->xc_lock);
 
@@ -299,7 +297,7 @@ xc_thread(void *cookie)
 	mutex_enter(&xc->xc_lock);
 	for (;;) {
 		while (!ci->ci_data.cpu_xcall_pending) {
-			if (xc->xc_headp == xc_tailp) {
+			if (xc->xc_headp == xc->xc_donep) {
 				cv_broadcast(&xc->xc_busy);
 			}
 			cv_wait(&ci->ci_data.cpu_xcall, &xc->xc_lock);
@@ -309,7 +307,6 @@ xc_thread(void *cookie)
 		func = xc->xc_func;
 		arg1 = xc->xc_arg1;
 		arg2 = xc->xc_arg2;
-		xc_tailp++;
 		mutex_exit(&xc->xc_lock);
 
 		KASSERT(func != NULL);

Reply via email to