From nobody Wed Oct 11 19:06:30 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4S5Mgp38Cqz4wZ2b; Wed, 11 Oct 2023 19:06:30 +0000 (UTC) (envelope-from git@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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4S5Mgp2Tdmz4bX1; Wed, 11 Oct 2023 19:06:30 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1697051190; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=eV60LuRZUN7p6H7pQs3r9FcK4D1MIAw8+2mbD8Oejh4=; b=TRgkWfpGDfyo3qeRMkJYrXGKJzyzeABXQBX4AGbXQUOP4w1G68OoxmVWScLcc1I+Lrj4L+ RcTyQHi+BHm41j2OIBuIh1L3jMWMqkcDaXepw5x7Hufqjuij6SwtaG/iX5N0ScGtsX3tmR AKQhefm8ELF3FAfw8mjqT4DWxPH65tUu6aRcB/cyqlCzMknQN2zAYY+Qnt65o+V8mzPjhg 0N8iVS16duv2/dqu2rT0sGLG3IEZS+pF2yu2JXvV6/OOAJFG0zjhNqMR6PmiS1T8/oj27Q sdhr0PIvX3iBg1UGC1ahfIdDu+UHCAq6eyx5qMOdUTJlx6LiGhN323RRj9bfdQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1697051190; a=rsa-sha256; cv=none; b=N/dVDCnGbOdGGtASFeBY3v4dDwrgjClfWy01yacZiduJXV/DyeZ/X79EVx/e6AhdZ8Og/K AyKHjd28uGL4ZD4tlJvKXnz9kUmYlVzRgJd7H7kwHHjDwpsGAleLUHCstyS0rMy/VdrQxj UcpZrDTfYtKZLHjSOGIHcMyKNufkqvfac0rmyb5EJrN1PGWMEWH66Of8JJRjRc2FfwUb+b Iy07YHtC/dlBx8iZm8maL79B7A0spuNQYV/TmwrP2K4h2WUze13PifUGidawXq41IqzrSG /g3KEDu3zlRHA9pIORiJZ+UZV4tfk3sqqR84KenPJhu353P24g6J5gR7bpgj6Q== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1697051190; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=eV60LuRZUN7p6H7pQs3r9FcK4D1MIAw8+2mbD8Oejh4=; b=cq3dyU5QQRe97X9xPQIoBuJUokyDlWG4PeAzzBgPIG+oC6wZQB+w9w2vLipRwBdy1Fik/L I07OnShIxSSeD3cbfFqsT02SnQAuwPXLSsAjHNGSOzdp4Lzq9SUwdVnmrESv6txNv7iaRH 03YP32D8wq04biDij1DXV6MIMdx1gbFEhDX+/gd4OvDqVlUShER6t2fbIRoLOCwGdhAskt Y/LHtyNSaO/nW9+bZyTGHiUlQaoy2tKvByA0QrQZ/plAH7A2NMoaV3B+x7tcVHl90E+T2w Zj9XHS/wF5jI2aram0jF+9T1wohP2KuCVkcOl+TfoKpiv4D4J0dSWz+4sQwtSg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4S5Mgp1XYmzXxb; Wed, 11 Oct 2023 19:06:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 39BJ6UJP058895; Wed, 11 Oct 2023 19:06:30 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 39BJ6Ukp058892; Wed, 11 Oct 2023 19:06:30 GMT (envelope-from git) Date: Wed, 11 Oct 2023 19:06:30 GMT Message-Id: <202310111906.39BJ6Ukp058892@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: John Baldwin Subject: git: 21923b24c4cf - stable/13 - amd64 db_trace: Reject unaligned frame pointers List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: jhb X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 21923b24c4cfc42870041a5aaa4effb3abd85fce Auto-Submitted: auto-generated The branch stable/13 has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=21923b24c4cfc42870041a5aaa4effb3abd85fce commit 21923b24c4cfc42870041a5aaa4effb3abd85fce Author: John Baldwin AuthorDate: 2023-09-01 22:55:05 +0000 Commit: John Baldwin CommitDate: 2023-10-11 15:09:52 +0000 amd64 db_trace: Reject unaligned frame pointers Switch to using db_addr_t to hold frame pointer values until they are verified to be suitably aligned. Reviewed by: kib, markj Differential Revision: https://reviews.freebsd.org/D41532 (cherry picked from commit 4a9cd9fc22d7f87a27ccd3d41b93a0356cd7061c) --- sys/amd64/amd64/db_trace.c | 127 ++++++++++++++++++++++++--------------------- 1 file changed, 68 insertions(+), 59 deletions(-) diff --git a/sys/amd64/amd64/db_trace.c b/sys/amd64/amd64/db_trace.c index 62d02117016a..87a0c2aec7bf 100644 --- a/sys/amd64/amd64/db_trace.c +++ b/sys/amd64/amd64/db_trace.c @@ -121,17 +121,14 @@ db_frame(struct db_variable *vp, db_expr_t *valuep, int op) #define INTERRUPT 2 #define SYSCALL 3 -static void db_nextframe(struct amd64_frame **, db_addr_t *, struct thread *); -static void db_print_stack_entry(const char *, db_addr_t, void *); - static void -db_print_stack_entry(const char *name, db_addr_t callpc, void *frame) +db_print_stack_entry(const char *name, db_addr_t callpc, db_addr_t frame) { db_printf("%s() at ", name != NULL ? name : "??"); db_printsym(callpc, DB_STGY_PROC); - if (frame != NULL) - db_printf("/frame 0x%lx", (register_t)frame); + if (frame != 0) + db_printf("/frame %#lx", frame); db_printf("\n"); } @@ -139,17 +136,20 @@ db_print_stack_entry(const char *name, db_addr_t callpc, void *frame) * Figure out the next frame up in the call stack. */ static void -db_nextframe(struct amd64_frame **fp, db_addr_t *ip, struct thread *td) +db_nextframe(db_addr_t *fp, db_addr_t *ip, struct thread *td) { struct trapframe *tf; + db_addr_t tf_addr; int frame_type; long rip, rsp, rbp; db_expr_t offset; c_db_sym_t sym; const char *name; - rip = db_get_value((long) &(*fp)->f_retaddr, 8, FALSE); - rbp = db_get_value((long) &(*fp)->f_frame, 8, FALSE); + rip = db_get_value(*fp + offsetof(struct amd64_frame, f_retaddr), 8, + FALSE); + rbp = db_get_value(*fp + offsetof(struct amd64_frame, f_frame), 8, + FALSE); /* * Figure out frame type. We look at the address just before @@ -193,50 +193,55 @@ db_nextframe(struct amd64_frame **fp, db_addr_t *ip, struct thread *td) * Normal frames need no special processing. */ if (frame_type == NORMAL) { - *ip = (db_addr_t) rip; - *fp = (struct amd64_frame *) rbp; + *ip = rip; + *fp = rbp; return; } - db_print_stack_entry(name, rip, &(*fp)->f_frame); + db_print_stack_entry(name, rip, *fp); /* * Point to base of trapframe which is just above the * current frame. */ - tf = (struct trapframe *)((long)*fp + 16); - - if (INKERNEL((long) tf)) { - rsp = tf->tf_rsp; - rip = tf->tf_rip; - rbp = tf->tf_rbp; - switch (frame_type) { - case TRAP: - db_printf("--- trap %#r", tf->tf_trapno); - break; - case SYSCALL: - db_printf("--- syscall"); - db_decode_syscall(td, tf->tf_rax); - break; - case INTERRUPT: - db_printf("--- interrupt"); - break; - default: - panic("The moon has moved again."); - } - db_printf(", rip = %#lr, rsp = %#lr, rbp = %#lr ---\n", rip, - rsp, rbp); + tf_addr = *fp + 16; + + if (!__is_aligned(tf_addr, _Alignof(*tf)) || !INKERNEL(tf_addr)) { + db_printf("--- invalid trapframe %p\n", (void *)tf_addr); + *ip = 0; + *fp = 0; + return; } + tf = (struct trapframe *)tf_addr; + + rsp = tf->tf_rsp; + rip = tf->tf_rip; + rbp = tf->tf_rbp; + switch (frame_type) { + case TRAP: + db_printf("--- trap %#r", tf->tf_trapno); + break; + case SYSCALL: + db_printf("--- syscall"); + db_decode_syscall(td, tf->tf_rax); + break; + case INTERRUPT: + db_printf("--- interrupt"); + break; + default: + __assert_unreachable(); + } + db_printf(", rip = %#lr, rsp = %#lr, rbp = %#lr ---\n", rip, rsp, rbp); - *ip = (db_addr_t) rip; - *fp = (struct amd64_frame *) rbp; + *ip = rip; + *fp = rbp; } static int -db_backtrace(struct thread *td, struct trapframe *tf, struct amd64_frame *frame, +db_backtrace(struct thread *td, struct trapframe *tf, db_addr_t frame, db_addr_t pc, register_t sp, int count) { - struct amd64_frame *actframe; + db_addr_t actframe; const char *name; db_expr_t offset; c_db_sym_t sym; @@ -270,7 +275,7 @@ db_backtrace(struct thread *td, struct trapframe *tf, struct amd64_frame *frame, * jumped to a bogus location, so try and use * the return address to find our caller. */ - db_print_stack_entry(name, pc, NULL); + db_print_stack_entry(name, pc, 0); pc = db_get_value(sp, 8, FALSE); if (db_search_symbol(pc, DB_STGY_PROC, &offset) == C_DB_SYM_NULL) @@ -282,20 +287,20 @@ db_backtrace(struct thread *td, struct trapframe *tf, struct amd64_frame *frame, instr = db_get_value(pc, 4, FALSE); if ((instr & 0xffffffff) == 0xe5894855) { /* pushq %rbp; movq %rsp, %rbp */ - actframe = (void *)(tf->tf_rsp - 8); + actframe = tf->tf_rsp - 8; } else if ((instr & 0xffffff) == 0xe58948) { /* movq %rsp, %rbp */ - actframe = (void *)tf->tf_rsp; + actframe = tf->tf_rsp; if (tf->tf_rbp == 0) { /* Fake frame better. */ frame = actframe; } } else if ((instr & 0xff) == 0xc3) { /* ret */ - actframe = (void *)(tf->tf_rsp - 8); + actframe = tf->tf_rsp - 8; } else if (offset == 0) { /* Probably an assembler symbol. */ - actframe = (void *)(tf->tf_rsp - 8); + actframe = tf->tf_rsp - 8; } } else if (name != NULL && strcmp(name, "fork_trampoline") == 0) { @@ -312,20 +317,24 @@ db_backtrace(struct thread *td, struct trapframe *tf, struct amd64_frame *frame, if (actframe != frame) { /* `frame' belongs to caller. */ - pc = (db_addr_t) - db_get_value((long)&actframe->f_retaddr, 8, FALSE); + pc = db_get_value(actframe + + offsetof(struct amd64_frame, f_retaddr), 8, FALSE); continue; } db_nextframe(&frame, &pc, td); - - if (INKERNEL((long)pc) && !INKERNEL((long)frame)) { - sym = db_search_symbol(pc, DB_STGY_ANY, &offset); - db_symbol_values(sym, &name, NULL); - db_print_stack_entry(name, pc, frame); + if (frame == 0) break; - } - if (!INKERNEL((long) frame)) { + + if (!__is_aligned(frame, _Alignof(struct amd64_frame)) || + !INKERNEL(frame)) { + if (INKERNEL(pc)) { + sym = db_search_symbol(pc, DB_STGY_ANY, + &offset); + db_symbol_values(sym, &name, NULL); + db_print_stack_entry(name, pc, frame); + break; + } break; } } @@ -336,14 +345,14 @@ db_backtrace(struct thread *td, struct trapframe *tf, struct amd64_frame *frame, void db_trace_self(void) { - struct amd64_frame *frame; - db_addr_t callpc; + db_addr_t callpc, frame; register_t rbp; __asm __volatile("movq %%rbp,%0" : "=r" (rbp)); - frame = (struct amd64_frame *)rbp; - callpc = (db_addr_t)db_get_value((long)&frame->f_retaddr, 8, FALSE); - frame = frame->f_frame; + callpc = db_get_value(rbp + offsetof(struct amd64_frame, f_retaddr), 8, + FALSE); + frame = db_get_value(rbp + offsetof(struct amd64_frame, f_frame), 8, + FALSE); db_backtrace(curthread, NULL, frame, callpc, 0, -1); } @@ -355,8 +364,8 @@ db_trace_thread(struct thread *thr, int count) ctx = kdb_thr_ctx(thr); tf = thr == kdb_thread ? kdb_frame : NULL; - return (db_backtrace(thr, tf, (struct amd64_frame *)ctx->pcb_rbp, - ctx->pcb_rip, ctx->pcb_rsp, count)); + return (db_backtrace(thr, tf, ctx->pcb_rbp, ctx->pcb_rip, ctx->pcb_rsp, + count)); } void