shithub: riscv

Download patch

ref: 5851d6fe391a1169d595721dd33afe77165bd84b
parent: 6d4540130cbcd79ee11747037735db6c5f812821
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;
-- 
⑨