From 0c1110ace2f247d2605599bb02f2866aee2ab1c6 Mon Sep 17 00:00:00 2001
From: cinap_lenrek <cinap_lenrek@felloff.net>
Date: Wed, 29 Mar 2017 00:30:53 +0200
Subject: kernel: fix twakeup()/timerdel() race condition

timerdel() did not make sure that the timer function
is not active (on another cpu). just acquiering the
Timer lock in the timer function only blocks the caller
of timerdel()/timeradd() but not the other way arround
(on a multiprocessor).

this changes the timer code to track activity of
the timer function, having timerdel() wait until
the timer has finished executing.
---
 sys/src/9/port/edf.c       | 36 ++++++++++++++++++------------------
 sys/src/9/port/portclock.c | 15 +++++++++++++++
 sys/src/9/port/portdat.h   |  1 +
 sys/src/9/port/portfns.h   |  1 +
 sys/src/9/port/proc.c      | 24 +++++++-----------------
 5 files changed, 42 insertions(+), 35 deletions(-)

diff --git a/sys/src/9/port/edf.c b/sys/src/9/port/edf.c
index a9fc4b854..99bbf1f61 100644
--- a/sys/src/9/port/edf.c
+++ b/sys/src/9/port/edf.c
@@ -207,7 +207,7 @@ release(Proc *p)
 }
 
 static void
-releaseintr(Ureg*, Timer *t)
+releaseintr(Ureg *u, Timer *t)
 {
 	Proc *p;
 	extern int panicking;
@@ -254,9 +254,7 @@ releaseintr(Ureg*, Timer *t)
 	case Wakeme:
 		release(p);
 		edfunlock();
-		if(p->trend)
-			wakeup(p->trend);
-		p->trend = nil;
+		twakeup(u, t);
 		if(up){
 			up->delaysched++;
 			delayedscheds++;
@@ -445,8 +443,7 @@ edfstop(Proc *p)
 		if(p->trace && (pt = proctrace))
 			pt(p, SExpel, 0);
 		e->flags &= ~Admitted;
-		if(e->tt)
-			timerdel(e);
+		timerdel(e);
 		edfunlock();
 	}
 }
@@ -479,20 +476,23 @@ edfyield(void)
 	e->r = e->t;
 	e->flags |= Yield;
 	e->d = now;
-	if (up->tt == nil){
-		n = e->t - now;
-		if(n < 20)
-			n = 20;
-		up->tns = 1000LL * n;
-		up->tf = releaseintr;
-		up->tmode = Trelative;
-		up->ta = up;
-		up->trend = &up->sleep;
-		timeradd(up);
-	}else if(up->tf != releaseintr)
-		print("edfyield: surprise! %#p\n", up->tf);
+	n = e->t - now;
+	if(n < 20)
+		n = 20;
+	up->tns = 1000LL * n;
+	up->tf = releaseintr;
+	up->tmode = Trelative;
+	up->ta = up;
+	up->trend = &up->sleep;
+	timeradd(up);
 	edfunlock();
+	if(waserror()){
+		up->trend = nil;
+		timerdel(up);
+		nexterror();
+	}
 	sleep(&up->sleep, yfn, nil);
+	poperror();
 }
 
 int
diff --git a/sys/src/9/port/portclock.c b/sys/src/9/port/portclock.c
index 4b52f6d30..47f85f8e1 100644
--- a/sys/src/9/port/portclock.c
+++ b/sys/src/9/port/portclock.c
@@ -112,9 +112,13 @@ timeradd(Timer *nt)
 void
 timerdel(Timer *dt)
 {
+	Mach *mp;
 	Timers *tt;
 	uvlong when;
 
+	/* avoid Tperiodic getting re-added */
+	dt->tmode = Trelative;
+
 	ilock(dt);
 	if(tt = dt->tt){
 		ilock(tt);
@@ -123,7 +127,16 @@ timerdel(Timer *dt)
 			timerset(tt->head->twhen);
 		iunlock(tt);
 	}
+	if((mp = dt->tactive) == nil || mp->machno == m->machno){
+		iunlock(dt);
+		return;
+	}
 	iunlock(dt);
+
+	/* rare, but tf can still be active on another cpu */
+	while(dt->tactive == mp && dt->tt == nil)
+		if(up->nlocks == 0 && islo())
+			sched();
 }
 
 void
@@ -190,12 +203,14 @@ timerintr(Ureg *u, Tval)
 		tt->head = t->tnext;
 		assert(t->tt == tt);
 		t->tt = nil;
+		t->tactive = MACHP(m->machno);
 		fcallcount[m->machno]++;
 		iunlock(tt);
 		if(t->tf)
 			(*t->tf)(u, t);
 		else
 			callhzclock++;
+		t->tactive = nil;
 		ilock(tt);
 		if(t->tmode == Tperiodic)
 			tadd(tt, t);
diff --git a/sys/src/9/port/portdat.h b/sys/src/9/port/portdat.h
index 00519897e..c91e7a29c 100644
--- a/sys/src/9/port/portdat.h
+++ b/sys/src/9/port/portdat.h
@@ -556,6 +556,7 @@ struct Timer
 	void	*ta;
 	/* Internal */
 	Lock;
+	Mach	*tactive;	/* The cpu that tf is active on */
 	Timers	*tt;		/* Timers queue this timer runs on */
 	Tval	tticks;		/* tns converted to ticks */
 	Tval	twhen;		/* ns represented in fastticks */
diff --git a/sys/src/9/port/portfns.h b/sys/src/9/port/portfns.h
index 5564ce0c8..14ff9ddaa 100644
--- a/sys/src/9/port/portfns.h
+++ b/sys/src/9/port/portfns.h
@@ -348,6 +348,7 @@ void		todinit(void);
 void		todset(vlong, vlong, int);
 Block*		trimblock(Block*, int, int);
 void		tsleep(Rendez*, int (*)(void*), void*, ulong);
+void		twakeup(Ureg*, Timer *);
 int		uartctl(Uart*, char*);
 int		uartgetc(void);
 void		uartkick(void*);
diff --git a/sys/src/9/port/proc.c b/sys/src/9/port/proc.c
index b39fc0942..e9a299c20 100644
--- a/sys/src/9/port/proc.c
+++ b/sys/src/9/port/proc.c
@@ -822,28 +822,17 @@ tfn(void *arg)
 	return up->trend == nil || up->tfn(arg);
 }
 
-static void
+void
 twakeup(Ureg*, Timer *t)
 {
 	Proc *p;
 	Rendez *trend;
 
-	ilock(t);
 	p = t->ta;
 	trend = p->trend;
 	if(trend != nil){
-		wakeup(trend);
 		p->trend = nil;
-	}
-	iunlock(t);
-}
-
-static void
-stoptimer(void)
-{
-	if(up->trend != nil){
-		up->trend = nil;
-		timerdel(up);
+		wakeup(trend);
 	}
 }
 
@@ -864,11 +853,13 @@ tsleep(Rendez *r, int (*fn)(void*), void *arg, ulong ms)
 	timeradd(up);
 
 	if(waserror()){
-		stoptimer();
+		up->trend = nil;
+		timerdel(up);
 		nexterror();
 	}
 	sleep(r, tfn, arg);
-	stoptimer();
+	up->trend = nil;
+	timerdel(up);
 	poperror();
 }
 
@@ -1102,8 +1093,7 @@ pexit(char *exitstr, int freemem)
 	void (*pt)(Proc*, int, vlong);
 
 	up->alarm = 0;
-	if(up->tt != nil)
-		timerdel(up);
+	timerdel(up);
 	pt = proctrace;
 	if(pt != nil)
 		pt(up, SDead, 0);
-- 
cgit v1.2.3