Date: Wed, 11 Aug 2021 01:31:06 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: b0f71f1bc5a4 - main - amd64: Add MD bits for KMSAN Message-ID: <202108110131.17B1V6pX051059@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=b0f71f1bc5a4b111523b73455fd012f54f7879f9 commit b0f71f1bc5a4b111523b73455fd012f54f7879f9 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2021-08-10 21:14:47 +0000 Commit: Mark Johnston <markj@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,@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) /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202108110131.17B1V6pX051059>