From a8a9f7108db349ad7e22f6c7c5d6c783801552c2 Mon Sep 17 00:00:00 2001 From: maxv Date: Wed, 10 Jan 2018 20:51:11 +0000 Subject: [PATCH] Restrict the check: SMAP faults are always protection violations, as the SDM points out, so make sure we have PGEX_P. This way NULL dereferences - which are caused by an unmapped VA, and therefore are not protection violations - don't take this branch, and don't display a misleading "SMAP" in ddb. Adding a PGEX_P check, or not, does not essentially change anything from a security point of view, it's just a matter of what gets displayed when a fatal fault comes in. I didn't put PGEX_P until now, because initially when I wrote the SMAP implementation Qemu did not always receive the fault if the PGEX_P check was there, while a native i5 would. I'm unable to reproduce this issue with a recent Qemu, so I assume I did something wrong when testing in the first place. --- sys/arch/amd64/amd64/trap.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/sys/arch/amd64/amd64/trap.c b/sys/arch/amd64/amd64/trap.c index 0db4277f0b20..7fc61be3d8b2 100644 --- a/sys/arch/amd64/amd64/trap.c +++ b/sys/arch/amd64/amd64/trap.c @@ -1,4 +1,4 @@ -/* $NetBSD: trap.c,v 1.109 2017/12/09 00:52:41 christos Exp $ */ +/* $NetBSD: trap.c,v 1.110 2018/01/10 20:51:11 maxv Exp $ */ /* * Copyright (c) 1998, 2000, 2017 The NetBSD Foundation, Inc. @@ -64,7 +64,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: trap.c,v 1.109 2017/12/09 00:52:41 christos Exp $"); +__KERNEL_RCSID(0, "$NetBSD: trap.c,v 1.110 2018/01/10 20:51:11 maxv Exp $"); #include "opt_ddb.h" #include "opt_kgdb.h" @@ -562,13 +562,11 @@ trap(struct trapframe *frame) } } - if (cr2 < VM_MAXUSER_ADDRESS) { + if ((frame->tf_err & PGEX_P) && + cr2 < VM_MAXUSER_ADDRESS) { /* SMAP might have brought us here */ if (onfault_handler(pcb, frame) == NULL) { - panic("prevented %s %p (SMAP)", - (cr2 < PAGE_SIZE - ? "null pointer dereference at" - : "access to"), + panic("prevented access to %p (SMAP)", (void *)cr2); } }