git: b0f71f1bc5a4 - main - amd64: Add MD bits for KMSAN
Mark Johnston
markj at FreeBSD.org
Wed Aug 11 01:31:07 UTC 2021
The branch main has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=b0f71f1bc5a4b111523b73455fd012f54f7879f9
commit b0f71f1bc5a4b111523b73455fd012f54f7879f9
Author: Mark Johnston <markj at FreeBSD.org>
AuthorDate: 2021-08-10 21:14:47 +0000
Commit: Mark Johnston <markj at FreeBSD.org>
CommitDate: 2021-08-11 01:27:53 +0000
amd64: Add MD bits for KMSAN
Interrupt and exception handlers must call kmsan_intr_enter() prior to
calling any C code. This is because the KMSAN runtime maintains some
TLS in order to track initialization state of function parameters and
return values across function calls. Then, to ensure that this state is
kept consistent in the face of asynchronous kernel-mode excpeptions, the
runtime uses a stack of TLS blocks, and kmsan_intr_enter() and
kmsan_intr_leave() push and pop that stack, respectively.
Use these functions in amd64 interrupt and exception handlers. Note
that handlers for user->kernel transitions need not be annotated.
Also ensure that trap frames pushed by the CPU and by handlers are
marked as initialized before they are used.
Reviewed by: kib
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D31467
---
sys/amd64/amd64/apic_vector.S | 22 ++++++++++++++++++++++
sys/amd64/amd64/atpic_vector.S | 2 ++
sys/amd64/amd64/exception.S | 12 ++++++++++--
sys/amd64/amd64/machdep.c | 2 ++
sys/amd64/amd64/trap.c | 5 +++++
sys/amd64/ia32/ia32_syscall.c | 3 +++
sys/amd64/include/asmacros.h | 22 ++++++++++++++++++++++
sys/kern/kern_fork.c | 2 ++
sys/kern/subr_trap.c | 3 +++
sys/x86/isa/atpic.c | 3 ++-
sys/x86/x86/local_apic.c | 6 ++++--
11 files changed, 77 insertions(+), 5 deletions(-)
diff --git a/sys/amd64/amd64/apic_vector.S b/sys/amd64/amd64/apic_vector.S
index 21696a93fc5f..cea9b79a0ec8 100644
--- a/sys/amd64/amd64/apic_vector.S
+++ b/sys/amd64/amd64/apic_vector.S
@@ -81,6 +81,7 @@ as_lapic_eoi:
*/
.macro ISR_VEC index, vec_name
INTR_HANDLER \vec_name
+ KMSAN_ENTER
cmpl $0,x2apic_mode
je 1f
movl $(MSR_APIC_ISR0 + \index),%ecx
@@ -97,6 +98,7 @@ as_lapic_eoi:
movl %eax, %edi /* pass the IRQ */
call lapic_handle_intr
3:
+ KMSAN_LEAVE
jmp doreti
.endm
@@ -125,22 +127,28 @@ IDTVEC(spuriousint)
* Local APIC periodic timer handler.
*/
INTR_HANDLER timerint
+ KMSAN_ENTER
movq %rsp, %rdi
call lapic_handle_timer
+ KMSAN_LEAVE
jmp doreti
/*
* Local APIC CMCI handler.
*/
INTR_HANDLER cmcint
+ KMSAN_ENTER
call lapic_handle_cmc
+ KMSAN_LEAVE
jmp doreti
/*
* Local APIC error interrupt handler.
*/
INTR_HANDLER errorint
+ KMSAN_ENTER
call lapic_handle_error
+ KMSAN_LEAVE
jmp doreti
#ifdef XENHVM
@@ -149,8 +157,10 @@ IDTVEC(spuriousint)
* Only used when the hypervisor supports direct vector callbacks.
*/
INTR_HANDLER xen_intr_upcall
+ KMSAN_ENTER
movq %rsp, %rdi
call xen_intr_handle_upcall
+ KMSAN_LEAVE
jmp doreti
#endif
@@ -165,8 +175,10 @@ IDTVEC(spuriousint)
* IPI handler for cache and TLB shootdown
*/
INTR_HANDLER invlop
+ KMSAN_ENTER
call invlop_handler
call as_lapic_eoi
+ KMSAN_LEAVE
jmp ld_regs
/*
@@ -174,7 +186,9 @@ IDTVEC(spuriousint)
*/
INTR_HANDLER ipi_intr_bitmap_handler
call as_lapic_eoi
+ KMSAN_ENTER
call ipi_bitmap_handler
+ KMSAN_LEAVE
jmp doreti
/*
@@ -182,15 +196,19 @@ IDTVEC(spuriousint)
*/
INTR_HANDLER cpustop
call as_lapic_eoi
+ KMSAN_ENTER
call cpustop_handler
+ KMSAN_LEAVE
jmp doreti
/*
* Executed by a CPU when it receives an IPI_SUSPEND from another CPU.
*/
INTR_HANDLER cpususpend
+ KMSAN_ENTER
call cpususpend_handler
call as_lapic_eoi
+ KMSAN_LEAVE
jmp doreti
/*
@@ -198,7 +216,9 @@ IDTVEC(spuriousint)
*/
INTR_HANDLER ipi_swi
call as_lapic_eoi
+ KMSAN_ENTER
call ipi_swi_handler
+ KMSAN_LEAVE
jmp doreti
/*
@@ -212,8 +232,10 @@ IDTVEC(spuriousint)
movq ipi_rendezvous_counts(,%rax,8), %rax
incq (%rax)
#endif
+ KMSAN_ENTER
call smp_rendezvous_action
call as_lapic_eoi
+ KMSAN_LEAVE
jmp doreti
/*
diff --git a/sys/amd64/amd64/atpic_vector.S b/sys/amd64/amd64/atpic_vector.S
index d76331a887ad..c6471999c238 100644
--- a/sys/amd64/amd64/atpic_vector.S
+++ b/sys/amd64/amd64/atpic_vector.S
@@ -44,9 +44,11 @@
*/
.macro INTR irq_num, vec_name
INTR_HANDLER \vec_name
+ KMSAN_ENTER
movq %rsp, %rsi
movl $\irq_num, %edi /* pass the IRQ */
call atpic_handle_intr
+ KMSAN_LEAVE
jmp doreti
.endm
diff --git a/sys/amd64/amd64/exception.S b/sys/amd64/amd64/exception.S
index 4716ca8cd7c2..d1e49faa40e7 100644
--- a/sys/amd64/amd64/exception.S
+++ b/sys/amd64/amd64/exception.S
@@ -282,8 +282,10 @@ alltraps_pushregs_no_rax:
.globl calltrap
.type calltrap, at function
calltrap:
- movq %rsp,%rdi
+ KMSAN_ENTER
+ movq %rsp, %rdi
call trap_check
+ KMSAN_LEAVE
jmp doreti /* Handle any pending ASTs */
/*
@@ -352,8 +354,10 @@ IDTVEC(dblfault)
cmpq $~0,%rax
je 2f
movq %rax,%cr3
-2: movq %rsp,%rdi
+2: KMSAN_ENTER
+ movq %rsp,%rdi
call dblfault_handler
+ KMSAN_LEAVE
3: hlt
jmp 3b
@@ -856,8 +860,10 @@ nmi_fromuserspace:
3:
/* Note: this label is also used by ddb and gdb: */
nmi_calltrap:
+ KMSAN_ENTER
movq %rsp,%rdi
call trap
+ KMSAN_LEAVE
#ifdef HWPMC_HOOKS
/*
* Capture a userspace callchain if needed.
@@ -1043,8 +1049,10 @@ mchk_fromuserspace:
1: call handle_ibrs_entry
/* Note: this label is also used by ddb and gdb: */
mchk_calltrap:
+ KMSAN_ENTER
movq %rsp,%rdi
call mca_intr
+ KMSAN_LEAVE
testl %ebx,%ebx /* %ebx != 0 => return to userland */
jnz doreti_exit
/*
diff --git a/sys/amd64/amd64/machdep.c b/sys/amd64/amd64/machdep.c
index 8599dc2fa8f6..e49dcaa576e8 100644
--- a/sys/amd64/amd64/machdep.c
+++ b/sys/amd64/amd64/machdep.c
@@ -77,6 +77,7 @@ __FBSDID("$FreeBSD$");
#include <sys/lock.h>
#include <sys/malloc.h>
#include <sys/memrange.h>
+#include <sys/msan.h>
#include <sys/msgbuf.h>
#include <sys/mutex.h>
#include <sys/pcpu.h>
@@ -1943,6 +1944,7 @@ hammer_time(u_int64_t modulep, u_int64_t physfree)
thread0.td_critnest = 0;
kasan_init();
+ kmsan_init();
TSEXIT();
diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c
index b08495f3f139..ff4bccebed5b 100644
--- a/sys/amd64/amd64/trap.c
+++ b/sys/amd64/amd64/trap.c
@@ -63,6 +63,7 @@ __FBSDID("$FreeBSD$");
#include <sys/kernel.h>
#include <sys/ktr.h>
#include <sys/lock.h>
+#include <sys/msan.h>
#include <sys/mutex.h>
#include <sys/resourcevar.h>
#include <sys/signalvar.h>
@@ -229,6 +230,7 @@ trap(struct trapframe *frame)
dr6 = 0;
kasan_mark(frame, sizeof(*frame), sizeof(*frame), 0);
+ kmsan_mark(frame, sizeof(*frame), KMSAN_STATE_INITED);
VM_CNT_INC(v_trap);
type = frame->tf_trapno;
@@ -973,6 +975,7 @@ trap_user_dtrace(struct trapframe *frame, int (**hookp)(struct trapframe *))
void
dblfault_handler(struct trapframe *frame)
{
+ kmsan_mark(frame, sizeof(*frame), KMSAN_STATE_INITED);
#ifdef KDTRACE_HOOKS
if (dtrace_doubletrap_func != NULL)
(*dtrace_doubletrap_func)();
@@ -1177,6 +1180,8 @@ amd64_syscall(struct thread *td, int traced)
{
ksiginfo_t ksi;
+ kmsan_mark(td->td_frame, sizeof(*td->td_frame), KMSAN_STATE_INITED);
+
#ifdef DIAGNOSTIC
if (!TRAPF_USERMODE(td->td_frame)) {
panic("syscall");
diff --git a/sys/amd64/ia32/ia32_syscall.c b/sys/amd64/ia32/ia32_syscall.c
index 9294ef8ce741..4e95d056d7fa 100644
--- a/sys/amd64/ia32/ia32_syscall.c
+++ b/sys/amd64/ia32/ia32_syscall.c
@@ -55,6 +55,7 @@ __FBSDID("$FreeBSD$");
#include <sys/kernel.h>
#include <sys/ktr.h>
#include <sys/lock.h>
+#include <sys/msan.h>
#include <sys/mutex.h>
#include <sys/proc.h>
#include <sys/ptrace.h>
@@ -208,6 +209,8 @@ ia32_syscall(struct trapframe *frame)
register_t orig_tf_rflags;
ksiginfo_t ksi;
+ kmsan_mark(frame, sizeof(*frame), KMSAN_STATE_INITED);
+
orig_tf_rflags = frame->tf_rflags;
td = curthread;
td->td_frame = frame;
diff --git a/sys/amd64/include/asmacros.h b/sys/amd64/include/asmacros.h
index 438f4ec26f61..973a1c761c6b 100644
--- a/sys/amd64/include/asmacros.h
+++ b/sys/amd64/include/asmacros.h
@@ -213,6 +213,28 @@ X\vec_name:
movq TF_R15(%rsp),%r15
.endm
+#ifdef KMSAN
+/*
+ * The KMSAN runtime relies on a TLS block to track initialization and origin
+ * state for function parameters and return values. To keep this state
+ * consistent in the face of asynchronous kernel-mode traps, the runtime
+ * maintains a stack of blocks: when handling an exception or interrupt,
+ * kmsan_intr_enter() pushes the new block to be used until the handler is
+ * complete, at which point kmsan_intr_leave() restores the previous block.
+ *
+ * Thus, KMSAN_ENTER/LEAVE hooks are required only in handlers for events that
+ * may have happened while in kernel-mode. In particular, they are not required
+ * around amd64_syscall() or ast() calls. Otherwise, kmsan_intr_enter() can be
+ * called unconditionally, without distinguishing between entry from user-mode
+ * or kernel-mode.
+ */
+#define KMSAN_ENTER callq kmsan_intr_enter
+#define KMSAN_LEAVE callq kmsan_intr_leave
+#else
+#define KMSAN_ENTER
+#define KMSAN_LEAVE
+#endif
+
#endif /* LOCORE */
#ifdef __STDC__
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index b71a00adb62e..7b8a95333868 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -1057,6 +1057,8 @@ fork_exit(void (*callout)(void *, struct trapframe *), void *arg,
struct thread *td;
struct thread *dtd;
+ kmsan_mark(frame, sizeof(*frame), KMSAN_STATE_INITED);
+
td = curthread;
p = td->td_proc;
KASSERT(p->p_state == PRS_NORMAL, ("executing process is still new"));
diff --git a/sys/kern/subr_trap.c b/sys/kern/subr_trap.c
index d0f616d037c5..edeeded09911 100644
--- a/sys/kern/subr_trap.c
+++ b/sys/kern/subr_trap.c
@@ -55,6 +55,7 @@ __FBSDID("$FreeBSD$");
#include <sys/capsicum.h>
#include <sys/kernel.h>
#include <sys/lock.h>
+#include <sys/msan.h>
#include <sys/mutex.h>
#include <sys/pmckern.h>
#include <sys/proc.h>
@@ -216,6 +217,8 @@ ast(struct trapframe *framep)
int flags, sig;
bool resched_sigs;
+ kmsan_mark(framep, sizeof(*framep), KMSAN_STATE_INITED);
+
td = curthread;
p = td->td_proc;
diff --git a/sys/x86/isa/atpic.c b/sys/x86/isa/atpic.c
index 28c10ee7009f..de72780c100b 100644
--- a/sys/x86/isa/atpic.c
+++ b/sys/x86/isa/atpic.c
@@ -43,6 +43,7 @@ __FBSDID("$FreeBSD$");
#include <sys/kernel.h>
#include <sys/lock.h>
#include <sys/module.h>
+#include <sys/msan.h>
#include <machine/cpufunc.h>
#include <machine/frame.h>
@@ -523,8 +524,8 @@ atpic_handle_intr(u_int vector, struct trapframe *frame)
{
struct intsrc *isrc;
- /* The frame may have been written into a poisoned region. */
kasan_mark(frame, sizeof(*frame), sizeof(*frame), 0);
+ kmsan_mark(frame, sizeof(*frame), KMSAN_STATE_INITED);
KASSERT(vector < NUM_ISA_IRQS, ("unknown int %u\n", vector));
isrc = &atintrs[vector].at_intsrc;
diff --git a/sys/x86/x86/local_apic.c b/sys/x86/x86/local_apic.c
index 9708121e0829..04de3ae47461 100644
--- a/sys/x86/x86/local_apic.c
+++ b/sys/x86/x86/local_apic.c
@@ -48,6 +48,7 @@ __FBSDID("$FreeBSD$");
#include <sys/kernel.h>
#include <sys/lock.h>
#include <sys/malloc.h>
+#include <sys/msan.h>
#include <sys/mutex.h>
#include <sys/pcpu.h>
#include <sys/proc.h>
@@ -1306,8 +1307,9 @@ lapic_handle_intr(int vector, struct trapframe *frame)
{
struct intsrc *isrc;
- /* The frame may have been written into a poisoned region. */
kasan_mark(frame, sizeof(*frame), sizeof(*frame), 0);
+ kmsan_mark(&vector, sizeof(vector), KMSAN_STATE_INITED);
+ kmsan_mark(frame, sizeof(*frame), KMSAN_STATE_INITED);
isrc = intr_lookup_source(apic_idt_to_irq(PCPU_GET(apic_id),
vector));
@@ -1324,8 +1326,8 @@ lapic_handle_timer(struct trapframe *frame)
/* Send EOI first thing. */
lapic_eoi();
- /* The frame may have been written into a poisoned region. */
kasan_mark(frame, sizeof(*frame), sizeof(*frame), 0);
+ kmsan_mark(frame, sizeof(*frame), KMSAN_STATE_INITED);
#if defined(SMP) && !defined(SCHED_ULE)
/*
More information about the dev-commits-src-all
mailing list