git: 9front

Download patch

ref: a6964cfab636bbd7f30abc6ce1776d283ba75cf7
parent: 9e3dbd0b075d3b8f48c6279c79e35d3e5963ea20
author: cinap_lenrek <cinap_lenrek@felloff.net>
date: Mon Apr 7 18:07:22 EDT 2025 

kernel: fix noted() checks for all archs

Theres a potential race allowing to set arbitrary regs/flags,
when the user-space Ureg struct is in a shared segment.
Then another process could modify the flags after our
check passed.

The right way todo it is to use setregisters(), which
copies first and then adjusts flags. And all later checks
use the kernel-copy of the Ureg, ignoring the user-space
one.


--- a/sys/src/9/arm64/trap.c
+++ b/sys/src/9/arm64/trap.c
@@ -360,13 +360,12 @@
 		pexit("Suicide", 0);
 	}
 
-	nureg->psr = (nureg->psr & USPSRMASK) | (ureg->psr & ~USPSRMASK);
-	memmove(ureg, nureg, sizeof(Ureg));
+	setregisters(ureg, (char*)ureg, (char*)nureg, sizeof(Ureg));
 	
 	switch(arg0){
 	case NCONT: case NRSTR:
-		if(!okaddr(nureg->pc, BY2WD, 0) || !okaddr(nureg->sp, BY2WD, 0) ||
-				(nureg->pc & 3) != 0 || (nureg->sp & 7) != 0){
+		if(!okaddr(ureg->pc, BY2WD, 0) || !okaddr(ureg->sp, BY2WD, 0) ||
+				(ureg->pc & 3) != 0 || (ureg->sp & 7) != 0){
 			qunlock(&up->debug);
 			pprint("suicide: trap in noted\n");
 			pexit("Suicide", 0);
@@ -376,16 +375,16 @@
 		break;
 	
 	case NSAVE:
-		if(!okaddr(nureg->pc, BY2WD, 0) || !okaddr(nureg->sp, BY2WD, 0) ||
-				(nureg->pc & 3) != 0 || (nureg->sp & 7) != 0){
+		sp = oureg - 4 * BY2WD - ERRMAX;
+		ureg->sp = sp;
+		ureg->r0 = (uintptr) oureg;
+		if(!okaddr(ureg->pc, BY2WD, 0) || !okaddr(ureg->sp, 4 * BY2WD, 1) ||
+				(nureg->pc & 3) != 0 || (ureg->sp & 7) != 0){
 			qunlock(&up->debug);
 			pprint("suicide: trap in noted\n");
 			pexit("Suicide", 0);
 		}
 		qunlock(&up->debug);
-		sp = oureg - 4 * BY2WD - ERRMAX;
-		ureg->sp = sp;
-		ureg->r0 = (uintptr) oureg;
 		((uintptr *) sp)[1] = oureg;
 		((uintptr *) sp)[0] = 0;
 		break;
--- a/sys/src/9/cycv/trap.c
+++ b/sys/src/9/cycv/trap.c
@@ -378,14 +378,12 @@
 		pexit("Suicide", 0);
 	}
 	
-	nureg->psr = nureg->psr & 0xf80f0000 | ureg->psr & 0x07f0ffff;
+	setregisters(ureg, (char*)ureg, (char*)nureg, sizeof(Ureg));
 	
-	memmove(ureg, nureg, sizeof(Ureg));
-	
 	switch(arg0){
 	case NCONT: case NRSTR:
-		if(!okaddr(nureg->pc, BY2WD, 0) || !okaddr(nureg->sp, BY2WD, 0) ||
-				(nureg->pc & 3) != 0 || (nureg->sp & 3) != 0){
+		if(!okaddr(ureg->pc, BY2WD, 0) || !okaddr(ureg->sp, BY2WD, 0) ||
+				(ureg->pc & 3) != 0 || (ureg->sp & 3) != 0){
 			qunlock(&up->debug);
 			pprint("suicide: trap in noted\n");
 			pexit("Suicide", 0);
@@ -395,17 +393,16 @@
 		break;
 	
 	case NSAVE:
-		if(!okaddr(nureg->pc, BY2WD, 0) || !okaddr(nureg->sp, BY2WD, 0) ||
-				(nureg->pc & 3) != 0 || (nureg->sp & 3) != 0){
+		sp = oureg - 4 * BY2WD - ERRMAX;
+		ureg->sp = sp;
+		ureg->r0 = (uintptr) oureg;
+		if(!okaddr(ureg->pc, BY2WD, 0) || !okaddr(ureg->sp, 4*BY2WD, 1) ||
+				(ureg->pc & 3) != 0 || (ureg->sp & 3) != 0){
 			qunlock(&up->debug);
 			pprint("suicide: trap in noted\n");
 			pexit("Suicide", 0);
 		}
 		qunlock(&up->debug);
-		sp = oureg - 4 * BY2WD - ERRMAX;
-		splhi();
-		ureg->sp = sp;
-		ureg->r0 = (uintptr) oureg;
 		((ulong *) sp)[1] = oureg;
 		((ulong *) sp)[0] = 0;
 		break;
--- a/sys/src/9/mt7688/syscall.c
+++ b/sys/src/9/mt7688/syscall.c
@@ -219,11 +219,12 @@
 		pexit("Suicide", 0);
 	}
 
-	setregisters(kur, (char*)kur, (char*)up->ureg, sizeof(Ureg));
+	setregisters(kur, (char*)kur, (char*)nur, sizeof(Ureg));
+
 	switch(arg0) {
 	case NCONT:
 	case NRSTR:				/* only used by APE */
-		if(!okaddr(nur->pc, BY2WD, 0) || !okaddr(nur->usp, BY2WD, 0)){
+		if(!okaddr(kur->pc, BY2WD, 0) || !okaddr(kur->usp, BY2WD, 0)){
 			pprint("suicide: trap in noted\n");
 			qunlock(&up->debug);
 			pexit("Suicide", 0);
@@ -230,20 +231,18 @@
 		}
 		up->ureg = (Ureg*)(*(ulong*)(oureg-BY2WD));
 		qunlock(&up->debug);
-		splhi();
 		break;
 
 	case NSAVE:				/* only used by APE */
-		if(!okaddr(nur->pc, BY2WD, 0) || !okaddr(nur->usp, BY2WD, 0)){
+		sp = oureg-4*BY2WD-ERRMAX;
+		kur->r1 = oureg;		/* arg 1 is ureg* */
+		kur->usp = sp;
+		if(!okaddr(kur->pc, BY2WD, 0) || !okaddr(kur->usp, 4*BY2WD, 1)){
 			pprint("suicide: trap in noted\n");
 			qunlock(&up->debug);
 			pexit("Suicide", 0);
 		}
 		qunlock(&up->debug);
-		sp = oureg-4*BY2WD-ERRMAX;
-		splhi();
-		kur->sp = sp;
-		kur->r1 = oureg;		/* arg 1 is ureg* */
 		((ulong*)sp)[1] = oureg;	/* arg 1 0(FP) is ureg* */
 		((ulong*)sp)[0] = 0;		/* arg 0 is pc */
 		break;
--- a/sys/src/9/omap/syscall.c
+++ b/sys/src/9/omap/syscall.c
@@ -27,7 +27,6 @@
 noted(Ureg* cur, uintptr arg0)
 {
 	NFrame *nf;
-	Ureg *nur;
 
 	qlock(&up->debug);
 	if(arg0 != NRSTR && !up->notified){
@@ -50,17 +49,12 @@
 		pexit("Suicide", 0);
 	}
 
-	/* don't let user change system flags */
-	nur = &nf->ureg;
-	nur->psr &= PsrMask|PsrDfiq|PsrDirq;
-	nur->psr |= (cur->psr & ~(PsrMask|PsrDfiq|PsrDirq));
+	setregisters(cur, (char*)cur, (char*)&nf->ureg, sizeof(Ureg));
 
-	memmove(cur, nur, sizeof(Ureg));
-
 	switch((int)arg0){
 	case NCONT:
 	case NRSTR:
-		if(!okaddr(nur->pc, BY2WD, 0) || !okaddr(nur->sp, BY2WD, 0)){
+		if(!okaddr(cur->pc, BY2WD, 0) || !okaddr(cur->sp, BY2WD, 0)){
 			qunlock(&up->debug);
 			pprint("suicide: trap in noted\n");
 			pexit("Suicide", 0);
@@ -69,19 +63,17 @@
 		qunlock(&up->debug);
 		break;
 	case NSAVE:
-		if(!okaddr(nur->pc, BY2WD, 0) || !okaddr(nur->sp, BY2WD, 0)){
+		cur->sp = (uintptr)nf;
+		cur->r0 = (uintptr)&nf->ureg;
+		if(!okaddr(cur->pc, BY2WD, 0) || !okaddr(cur->sp, sizeof(NFrame), 1)){
 			qunlock(&up->debug);
 			pprint("suicide: trap in noted\n");
 			pexit("Suicide", 0);
 		}
 		qunlock(&up->debug);
-
-		splhi();
 		nf->arg1 = nf->msg;
 		nf->arg0 = &nf->ureg;
 		nf->ip = 0;
-		cur->sp = (uintptr)nf;
-		cur->r0 = (uintptr)nf->arg0;
 		break;
 	default:
 		up->lastnote->flag = NDebug;
@@ -160,6 +152,7 @@
 	ulong sp;
 	long ret;
 	int i, scallnr;
+	vlong startns, stopns;
 
 	if(!kenter(ureg))
 		panic("syscall: from kernel: pc %#lux r14 %#lux psr %#lux",
@@ -169,23 +162,25 @@
 	up->insyscall = 1;
 	up->pc = ureg->pc;
 
-	if(up->procctl == Proc_tracesyscall){
-		up->procctl = Proc_stopme;
-		procctl();
-	}
-
 	scallnr = ureg->r0;
 	up->scallnr = scallnr;
 	spllo();
-
 	sp = ureg->sp;
+
 	up->nerrlab = 0;
 	ret = -1;
 	if(!waserror()){
 		if(sp < (USTKTOP-BY2PG) || sp > (USTKTOP-sizeof(Sargs)-BY2WD))
 			validaddr(sp, sizeof(Sargs)+BY2WD, 0);
-
 		up->s = *((Sargs*)(sp+BY2WD));
+		if(up->procctl == Proc_tracesyscall){
+			syscallfmt(scallnr, ureg->pc, (va_list)up->s.args);
+			s = splhi();
+			up->procctl = Proc_stopme;
+			procctl();
+			splx(s);
+			todget(nil, &startns);
+		}
 		if(scallnr >= nsyscall || systab[scallnr] == nil){
 			postnote(up, 1, "sys: bad sys call", NDebug);
 			error(Ebadarg);
@@ -216,6 +211,8 @@
 	ureg->r0 = ret;
 
 	if(up->procctl == Proc_tracesyscall){
+		todget(nil, &stopns);
+		sysretfmt(scallnr, (va_list)up->s.args, ret, startns, stopns);
 		s = splhi();
 		up->procctl = Proc_stopme;
 		procctl();
@@ -226,7 +223,7 @@
 	up->psstate = 0;
 
 	if(scallnr == NOTED)
-		noted(ureg, *(ulong*)(sp+BY2WD));
+		noted(ureg, *((ulong*)up->s.args));
 
 	splhi();
 	if(scallnr != RFORK && (up->procctl || up->nnote))
@@ -237,7 +234,6 @@
 		sched();
 	kexit(ureg);
 }
-
 
 uintptr
 execregs(uintptr entry, ulong ssize, ulong nargs)
--- a/sys/src/9/pc/trap.c
+++ b/sys/src/9/pc/trap.c
@@ -639,19 +639,14 @@
 		pexit("Suicide", 0);
 	}
 
-	/* don't let user change system flags */
-	nureg->flags = (ureg->flags & ~0xCD5) | (nureg->flags & 0xCD5);
-	nureg->cs |= 3;
-	nureg->ss |= 3;
+	setregisters(ureg, (char*)ureg, (char*)nureg, sizeof(Ureg));
 
-	memmove(ureg, nureg, sizeof(Ureg));
-
 	switch(arg0){
 	case NCONT:
 	case NRSTR:
 if(0) print("%s %lud: noted %.8lux %.8lux\n",
 	up->text, up->pid, nureg->pc, nureg->usp);
-		if(!okaddr(nureg->pc, 1, 0) || !okaddr(nureg->usp, BY2WD, 0)){
+		if(!okaddr(ureg->pc, 1, 0) || !okaddr(ureg->usp, BY2WD, 0)){
 			qunlock(&up->debug);
 			pprint("suicide: trap in noted\n");
 			pexit("Suicide", 0);
@@ -661,16 +656,14 @@
 		break;
 
 	case NSAVE:
-		if(!okaddr(nureg->pc, BY2WD, 0)
-		|| !okaddr(nureg->usp, BY2WD, 0)){
+		sp = oureg-4*BY2WD-ERRMAX;
+		ureg->usp = sp;
+		if(!okaddr(ureg->pc, 1, 0) || !okaddr(ureg->usp, 4*BY2WD, 1)){
 			qunlock(&up->debug);
 			pprint("suicide: trap in noted\n");
 			pexit("Suicide", 0);
 		}
 		qunlock(&up->debug);
-		sp = oureg-4*BY2WD-ERRMAX;
-		splhi();
-		ureg->sp = sp;
 		((ulong*)sp)[1] = oureg;	/* arg 1 0(FP) is ureg* */
 		((ulong*)sp)[0] = 0;		/* arg 0 is pc */
 		break;
--- a/sys/src/9/sgi/trap.c
+++ b/sys/src/9/sgi/trap.c
@@ -548,11 +548,12 @@
 		pexit("Suicide", 0);
 	}
 
-	setregisters(kur, (char*)kur, (char*)up->ureg, sizeof(Ureg));
+	setregisters(kur, (char*)kur, (char*)nur, sizeof(Ureg));
+
 	switch(arg0) {
 	case NCONT:
 	case NRSTR:				/* only used by APE */
-		if(!okaddr(nur->pc, BY2WD, 0) || !okaddr(nur->usp, BY2WD, 0)){
+		if(!okaddr(kur->pc, BY2WD, 0) || !okaddr(kur->usp, BY2WD, 0)){
 			pprint("suicide: trap in noted\n");
 			qunlock(&up->debug);
 			pexit("Suicide", 0);
@@ -559,20 +560,18 @@
 		}
 		up->ureg = (Ureg*)(*(ulong*)(oureg-BY2WD));
 		qunlock(&up->debug);
-		splhi();
 		break;
 
 	case NSAVE:				/* only used by APE */
-		if(!okaddr(nur->pc, BY2WD, 0) || !okaddr(nur->usp, BY2WD, 0)){
+		sp = oureg-4*BY2WD-ERRMAX;
+		kur->r1 = oureg;		/* arg 1 is ureg* */
+		kur->usp = sp;
+		if(!okaddr(kur->pc, BY2WD, 0) || !okaddr(kur->usp, 4*BY2WD, 1)){
 			pprint("suicide: trap in noted\n");
 			qunlock(&up->debug);
 			pexit("Suicide", 0);
 		}
 		qunlock(&up->debug);
-		sp = oureg-4*BY2WD-ERRMAX;
-		splhi();
-		kur->sp = sp;
-		kur->r1 = oureg;		/* arg 1 is ureg* */
 		((ulong*)sp)[1] = oureg;	/* arg 1 0(FP) is ureg* */
 		((ulong*)sp)[0] = 0;		/* arg 0 is pc */
 		break;
--- a/sys/src/9/xen/trap.c
+++ b/sys/src/9/xen/trap.c
@@ -566,15 +566,12 @@
 		pexit("Suicide", 0);
 	}
 
-	/* don't let user change system flags */
-	nureg->flags = (ureg->flags & ~0xCD5) | (nureg->flags & 0xCD5);
+	setregisters(ureg, (char*)ureg, (char*)nureg, sizeof(Ureg));
 
-	memmove(ureg, nureg, sizeof(Ureg));
-
 	switch(arg0){
 	case NCONT:
 	case NRSTR:
-		if(!okaddr(nureg->pc, 1, 0) || !okaddr(nureg->usp, BY2WD, 0)){
+		if(!okaddr(ureg->pc, 1, 0) || !okaddr(ureg->usp, BY2WD, 0)){
 			qunlock(&up->debug);
 			pprint("suicide: trap in noted\n");
 			pexit("Suicide", 0);
@@ -584,16 +581,14 @@
 		break;
 
 	case NSAVE:
-		if(!okaddr(nureg->pc, BY2WD, 0)
-		|| !okaddr(nureg->usp, BY2WD, 0)){
+		sp = oureg-4*BY2WD-ERRMAX;
+		ureg->usp = sp;
+		if(!okaddr(ureg->pc, 1, 0) || !okaddr(ureg->usp, 4*BY2WD, 1)){
 			qunlock(&up->debug);
 			pprint("suicide: trap in noted\n");
 			pexit("Suicide", 0);
 		}
 		qunlock(&up->debug);
-		sp = oureg-4*BY2WD-ERRMAX;
-		splhi();
-		ureg->sp = sp;
 		((ulong*)sp)[1] = oureg;	/* arg 1 0(FP) is ureg* */
 		((ulong*)sp)[0] = 0;		/* arg 0 is pc */
 		break;
--- a/sys/src/9/zynq/trap.c
+++ b/sys/src/9/zynq/trap.c
@@ -415,15 +415,13 @@
 		pprint("bad ureg in noted or call to noted when not notified\n");
 		pexit("Suicide", 0);
 	}
+
+	setregisters(ureg, (char*)ureg, (char*)nureg, sizeof(Ureg));
 	
-	nureg->psr = nureg->psr & 0xf80f0000 | ureg->psr & 0x07f0ffff;
-	
-	memmove(ureg, nureg, sizeof(Ureg));
-	
 	switch(arg0){
 	case NCONT: case NRSTR:
-		if(!okaddr(nureg->pc, BY2WD, 0) || !okaddr(nureg->sp, BY2WD, 0) ||
-				(nureg->pc & 3) != 0 || (nureg->sp & 3) != 0){
+		if(!okaddr(ureg->pc, BY2WD, 0) || !okaddr(ureg->sp, BY2WD, 0) ||
+				(ureg->pc & 3) != 0 || (ureg->sp & 3) != 0){
 			qunlock(&up->debug);
 			pprint("suicide: trap in noted\n");
 			pexit("Suicide", 0);
@@ -433,17 +431,16 @@
 		break;
 	
 	case NSAVE:
-		if(!okaddr(nureg->pc, BY2WD, 0) || !okaddr(nureg->sp, BY2WD, 0) ||
-				(nureg->pc & 3) != 0 || (nureg->sp & 3) != 0){
+		sp = oureg - 4 * BY2WD - ERRMAX;
+		ureg->sp = sp;
+		ureg->r0 = (uintptr) oureg;
+		if(!okaddr(ureg->pc, BY2WD, 0) || !okaddr(ureg->sp, 4 * BY2WD, 1) ||
+				(ureg->pc & 3) != 0 || (ureg->sp & 3) != 0){
 			qunlock(&up->debug);
 			pprint("suicide: trap in noted\n");
 			pexit("Suicide", 0);
 		}
 		qunlock(&up->debug);
-		sp = oureg - 4 * BY2WD - ERRMAX;
-		splhi();
-		ureg->sp = sp;
-		ureg->r0 = (uintptr) oureg;
 		((ulong *) sp)[1] = oureg;
 		((ulong *) sp)[0] = 0;
 		break;
-- 
⑨