From owner-svn-src-all@freebsd.org Tue Dec 1 17:17:23 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 9344F4AEFBB; Tue, 1 Dec 2020 17:17:23 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Clpgl3qLXz4ZGp; Tue, 1 Dec 2020 17:17:23 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 718A61940C; Tue, 1 Dec 2020 17:17:23 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 0B1HHNJO013860; Tue, 1 Dec 2020 17:17:23 GMT (envelope-from jhb@FreeBSD.org) Received: (from jhb@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 0B1HHNr1013859; Tue, 1 Dec 2020 17:17:23 GMT (envelope-from jhb@FreeBSD.org) Message-Id: <202012011717.0B1HHNr1013859@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: jhb set sender to jhb@FreeBSD.org using -f From: John Baldwin Date: Tue, 1 Dec 2020 17:17:23 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r368241 - head/sys/mips/mips X-SVN-Group: head X-SVN-Commit-Author: jhb X-SVN-Commit-Paths: head/sys/mips/mips X-SVN-Commit-Revision: 368241 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Dec 2020 17:17:23 -0000 Author: jhb Date: Tue Dec 1 17:17:22 2020 New Revision: 368241 URL: https://svnweb.freebsd.org/changeset/base/368241 Log: Make stack_save*() more robust on MIPS. - Validate any stack addresses read from against td_kstack before reading. If an unwind operation would attempt to read outside the bounds of td_kstack, abort the unwind instead. - For stack_save_td(), don't use the PC and SP from the current thread, instead read the PC and SP from pcb_context[]. - For stack_save(), use the current PC and SP of the current thread, not the values from pcb_regs (the horribly named td_frame of the outermost trapframe). The result was that stack_trace() never logged _any_ kernel frames but only the frame from the saved userspace registers on entry from the kernel. - Inline the one use of stack_register_fetch(). - Add a VALID_PC() helper macro and simplify types to remove excessive casts in stack_capture(). - Fix stack_capture() to work on compilers written in this century. Don't treat function epilogues as function prologues by skipping additions to SP when searching for a function start. - Add some comments to stack_capture() and fix some style bugs. Reviewed by: arichardson Obtained from: CheriBSD Sponsored by: DARPA Differential Revision: https://reviews.freebsd.org/D27358 Modified: head/sys/mips/mips/stack_machdep.c Modified: head/sys/mips/mips/stack_machdep.c ============================================================================== --- head/sys/mips/mips/stack_machdep.c Tue Dec 1 17:04:46 2020 (r368240) +++ head/sys/mips/mips/stack_machdep.c Tue Dec 1 17:17:22 2020 (r368241) @@ -41,30 +41,33 @@ __FBSDID("$FreeBSD$"); #include #include -static u_register_t -stack_register_fetch(u_register_t sp, u_register_t stack_pos) -{ - u_register_t * stack = - ((u_register_t *)(intptr_t)sp + (size_t)stack_pos/sizeof(u_register_t)); +#define VALID_PC(addr) ((addr) >= (uintptr_t)btext && (addr) % 4 == 0) - return *stack; -} - static void -stack_capture(struct stack *st, u_register_t pc, u_register_t sp) +stack_capture(struct stack *st, struct thread *td, uintptr_t pc, uintptr_t sp) { - u_register_t ra = 0, i, stacksize; - short ra_stack_pos = 0; + u_register_t ra; + uintptr_t i, ra_addr; + int ra_stack_pos, stacksize; InstFmt insn; stack_zero(st); for (;;) { - stacksize = 0; - if (pc <= (u_register_t)(intptr_t)btext) + if (!VALID_PC(pc)) break; - for (i = pc; i >= (u_register_t)(intptr_t)btext; i -= sizeof (insn)) { - bcopy((void *)(intptr_t)i, &insn, sizeof insn); + + /* + * Walk backward from the PC looking for the function + * start. Assume a subtraction from SP is the start + * of a function. Hope that we find the store of RA + * into the stack frame along the way and save the + * offset of the saved RA relative to SP. + */ + ra_stack_pos = -1; + stacksize = 0; + for (i = pc; VALID_PC(i); i -= sizeof(insn)) { + bcopy((void *)i, &insn, sizeof(insn)); switch (insn.IType.op) { case OP_ADDI: case OP_ADDIU: @@ -72,6 +75,17 @@ stack_capture(struct stack *st, u_register_t pc, u_reg case OP_DADDIU: if (insn.IType.rs != SP || insn.IType.rt != SP) break; + + /* + * Ignore stack fixups in "early" + * returns in a function, or if the + * call was from an unlikely branch + * moved after the end of the normal + * return. + */ + if ((short)insn.IType.imm > 0) + break; + stacksize = -(short)insn.IType.imm; break; @@ -85,36 +99,49 @@ stack_capture(struct stack *st, u_register_t pc, u_reg break; } - if (stacksize) + if (stacksize != 0) break; } if (stack_put(st, pc) == -1) break; - for (i = pc; !ra; i += sizeof (insn)) { - bcopy((void *)(intptr_t)i, &insn, sizeof insn); + if (ra_stack_pos == -1) + break; + /* + * Walk forward from the PC to find the function end + * (jr RA). If eret is hit instead, stop unwinding. + */ + ra_addr = sp + ra_stack_pos; + ra = 0; + for (i = pc; VALID_PC(i); i += sizeof(insn)) { + bcopy((void *)i, &insn, sizeof(insn)); + switch (insn.IType.op) { case OP_SPECIAL: if (insn.RType.func == OP_JR) { - if (ra >= (u_register_t)(intptr_t)btext) - break; if (insn.RType.rs != RA) break; - ra = stack_register_fetch(sp, - ra_stack_pos); - if (!ra) + if (!kstack_contains(td, ra_addr, + sizeof(ra))) goto done; + ra = *(u_register_t *)ra_addr; + if (ra == 0) + goto done; ra -= 8; } break; default: break; } + /* eret */ if (insn.word == 0x42000018) goto done; + + if (ra != 0) + break; } if (pc == ra && stacksize == 0) @@ -122,7 +149,6 @@ stack_capture(struct stack *st, u_register_t pc, u_reg sp += stacksize; pc = ra; - ra = 0; } done: return; @@ -131,7 +157,7 @@ done: int stack_save_td(struct stack *st, struct thread *td) { - u_register_t pc, sp; + uintptr_t pc, sp; THREAD_LOCK_ASSERT(td, MA_OWNED); KASSERT(!TD_IS_SWAPPED(td), @@ -140,21 +166,19 @@ stack_save_td(struct stack *st, struct thread *td) if (TD_IS_RUNNING(td)) return (EOPNOTSUPP); - pc = td->td_pcb->pcb_regs.pc; - sp = td->td_pcb->pcb_regs.sp; - stack_capture(st, pc, sp); + pc = td->td_pcb->pcb_context[PCB_REG_RA]; + sp = td->td_pcb->pcb_context[PCB_REG_SP]; + stack_capture(st, td, pc, sp); return (0); } void stack_save(struct stack *st) { - u_register_t pc, sp; + uintptr_t pc, sp; - if (curthread == NULL) - panic("stack_save: curthread == NULL"); - - pc = curthread->td_pcb->pcb_regs.pc; - sp = curthread->td_pcb->pcb_regs.sp; - stack_capture(st, pc, sp); + pc = (uintptr_t)&&here; + sp = (uintptr_t)__builtin_frame_address(0); +here: + stack_capture(st, curthread, pc, sp); }