shithub: riscv

Info  •  Files  •  Log  •  Branches

kernel: fix tsleep()/twakeup()/tsemacquire() race

tsleep() used to cancel the timer with:

if(up->tt != nil)
	timerdel(up);

which still can result in twakeup() to fire after tsleep()
returns (because we set Timer.tt to nil *before* we call the tfn).
in most cases, this is not an issue as the Rendez*
usually is just &up->sleep, but when it is dynamically allocated
or on the stack like in tsemacquire(), twakeup() will call
wakeup() on a potentially garbage Rendez structure!

to fix the race, we execute the wakup() with the Timer lock
held, and set p->trend to nil only after we called wakeup().

that way, the timerdel(); which unconditionally locks the Timer;
can act as a proper barrier and use up->trend == nil as the
condition if the timer has already fired.

--- a/sys/src/9/port/proc.c

+++ b/sys/src/9/port/proc.c

@@ -822,24 +822,37 @@

 	return up->trend == nil || up->tfn(arg);

 }

-void

+static void

 twakeup(Ureg*, Timer *t)

 {

 	Proc *p;

 	Rendez *trend;

+	ilock(t);

 	p = t->ta;

 	trend = p->trend;

-	p->trend = nil;

-	if(trend != nil)

+	if(trend != nil){

 		wakeup(trend);

+		p->trend = nil;

+	}

+	iunlock(t);

 }

+static void

+stoptimer(void)

+{

+	if(up->trend != nil){

+		up->trend = nil;

+		timerdel(up);

+	}

+}

+

 void

 tsleep(Rendez *r, int (*fn)(void*), void *arg, ulong ms)

 {

 	if(up->tt != nil){

-		print("tsleep: timer active: mode %d, tf %#p\n", up->tmode, up->tf);

+		print("%s %lux: tsleep timer active: mode %d, tf %#p, pc %#p\n",

+			up->text, up->pid, up->tmode, up->tf, getcallerpc(&r));

 		timerdel(up);

 	}

 	up->tns = MS2NS(ms);

@@ -851,13 +864,11 @@

 	timeradd(up);

 	if(waserror()){

-		timerdel(up);

+		stoptimer();

 		nexterror();

 	}

 	sleep(r, tfn, arg);

-	if(up->tt != nil)

-		timerdel(up);

-	up->twhen = 0;

+	stoptimer();

 	poperror();

 }