Validate if the value written into {FS,GS}.base is a canonical

address, writting non-canonical address can cause kernel a panic,
by restricting base values to 0..VM_MAXUSER_ADDRESS, ensuring
only canonical values get written to the registers.

Reviewed by: peter, Josepha Koshy < joseph.koshy at gmail dot com >
Approved by: re (scottl)

2 files changed, 44 insertions, 13 deletions

diff --git a/sys/amd64/amd64/sys_machdep.c b/sys/amd64/amd64/sys_machdep.c
index 25fbb6f..7f022d0 100644
--- a/sys/amd64/amd64/sys_machdep.c
+++ b/sys/amd64/amd64/sys_machdep.c
@@ -42,6 +42,10 @@ __FBSDID("$FreeBSD$");
 #include <machine/sysarch.h>
 #include <machine/pcb.h>
 
+#include <vm/vm.h>
+#include <vm/pmap.h>
+#include <machine/vmparam.h>
+
 #ifndef _SYS_SYSPROTO_H_
 struct sysarch_args {
 	int op;
@@ -57,6 +61,7 @@ sysarch(td, uap)
 	int error = 0;
 	struct pcb *pcb = curthread->td_pcb;
 	uint32_t i386base;
+	uint64_t a64base;
 
 	switch(uap->op) {
 	case I386_GET_FSBASE:
@@ -65,9 +70,12 @@ sysarch(td, uap)
 		break;
 	case I386_SET_FSBASE:
 		error = copyin(uap->parms, &i386base, sizeof(i386base));
-		pcb->pcb_fsbase = i386base;
-		if (!error)
-			wrmsr(MSR_FSBASE, pcb->pcb_fsbase);
+		if (!error) {
+			critical_enter();
+			wrmsr(MSR_FSBASE, i386base);
+			pcb->pcb_fsbase = i386base;
+			critical_exit();
+		}
 		break;
 	case I386_GET_GSBASE:
 		i386base = pcb->pcb_gsbase;
@@ -75,18 +83,29 @@ sysarch(td, uap)
 		break;
 	case I386_SET_GSBASE:
 		error = copyin(uap->parms, &i386base, sizeof(i386base));
-		pcb->pcb_gsbase = i386base;
-		if (!error)
-			wrmsr(MSR_KGSBASE, pcb->pcb_gsbase);
+		if (!error) {
+			critical_enter();
+			wrmsr(MSR_KGSBASE, i386base);
+			pcb->pcb_gsbase = i386base;
+			critical_exit();
+		}
 		break;
 	case AMD64_GET_FSBASE:
 		error = copyout(&pcb->pcb_fsbase, uap->parms, sizeof(pcb->pcb_fsbase));
 		break;
 		
 	case AMD64_SET_FSBASE:
-		error = copyin(uap->parms, &pcb->pcb_fsbase, sizeof(pcb->pcb_fsbase));
-		if (!error)
-			wrmsr(MSR_FSBASE, pcb->pcb_fsbase);
+		error = copyin(uap->parms, &a64base, sizeof(a64base));
+		if (!error) {
+			if (a64base < VM_MAXUSER_ADDRESS) {
+				critical_enter();
+				wrmsr(MSR_FSBASE, a64base);
+				pcb->pcb_fsbase = a64base;
+				critical_exit();
+			} else {
+				error = EINVAL;
+			}
+		}
 		break;
 
 	case AMD64_GET_GSBASE:
@@ -94,9 +113,17 @@ sysarch(td, uap)
 		break;
 
 	case AMD64_SET_GSBASE:
-		error = copyin(uap->parms, &pcb->pcb_gsbase, sizeof(pcb->pcb_gsbase));
-		if (!error)
-			wrmsr(MSR_KGSBASE, pcb->pcb_gsbase);
+		error = copyin(uap->parms, &a64base, sizeof(a64base));
+		if (!error) {
+			if (a64base < VM_MAXUSER_ADDRESS) {
+				critical_enter();
+				wrmsr(MSR_KGSBASE, a64base);
+				pcb->pcb_gsbase = a64base;
+				critical_exit();
+			} else {
+				error = EINVAL;
+			}
+		}
 		break;
 
 	default:
diff --git a/sys/amd64/amd64/vm_machdep.c b/sys/amd64/amd64/vm_machdep.c
index 5acc1c0..424adaf 100644
--- a/sys/amd64/amd64/vm_machdep.c
+++ b/sys/amd64/amd64/vm_machdep.c
@@ -341,10 +341,13 @@ cpu_set_upcall_kse(struct thread *td, void (*entry)(void *), void *arg,
 	td->td_frame->tf_rdi = (register_t)arg;
 }
 
-void
+int
 cpu_set_user_tls(struct thread *td, void *tls_base)
 {
 
+	if ((u_int64_t)tls_base >= VM_MAXUSER_ADDRESS)
+		return (EINVAL);
+
 	if (td == curthread) {
 		critical_enter();
 		td->td_pcb->pcb_fsbase = (register_t)tls_base;
@@ -353,6 +356,7 @@ cpu_set_user_tls(struct thread *td, void *tls_base)
 	} else {
 		td->td_pcb->pcb_fsbase = (register_t)tls_base;
 	}
+	return (0);
 }
 
 #ifdef SMP