From nobody Fri Sep 01 22:55:49 2023 X-Original-To: dev-commits-src-main@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 4Rctfs5Gn6z4rrXb; Fri, 1 Sep 2023 22:55:49 +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 4Rctfs4jdRz3BpS; Fri, 1 Sep 2023 22:55:49 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1693608949; 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=gO7GJazsnaAH34DMmhV+5OTSntLjm/wtTGB/7RORcd8=; b=NuqEFH636jH1yRbct/DVcLuWGyciIq/JaJNgCKT1qDncMBT8krGSEXm3cgt+jPKQ1AXRxI himrrpSbxLquGVLmRSe10FQJ4HIS0TJ62ZCvkzrECO2q3IPmYQahLgjmSzdh4vI4CcJkfx YdsTDJbZFbiZFr9l1dkFzsSdy4gPb6h1t58IbnBDD9xLbbtO1b88+PlL7MgVW0KkvlR5Fu W7xiFnC3PPO7i6PL5MHcw6o9NRLHq1zpTcfwSe5D6YMlqmJkzLkBJPgG1M0l8sYHZb3Svw 2aSskKPJjjCSyYBWJa2PbIGOChamW6rHxz1XqxReuPUpommuRCxXGPfJtWqb5g== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1693608949; a=rsa-sha256; cv=none; b=tozD7LNbrZmJXzEAoTME4zLWI2T9QCR/Eg+9S6yVCj5IpiwhnxgxoJyZPTo5jrjaJuirS1 qWSUz4WJIXh82VLtGyYxqerG7wOE4FZ1uzZVXc13C5g6X/vU/z5FkmhYrgbexTmsFeyGJ+ TY8+wy5i1lzGh3tYEcoLpl8eWqh0x5aJ76Px8WFltUOdamL6/C0OOIh1Jh0OlVhlMndyVv VWNEvPH2cYmpUZwx4wZnse1wHa3yxEgN8rZAeiwIkjh3+Jz4uH481Y768D4tIrjgxl8s8F mifiM6ZPRBpYOv+Hd0Hh0Ko/lkT2oqIgC8ictujehpcjmzmqBLqtQfn0pMlAog== 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=1693608949; 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=gO7GJazsnaAH34DMmhV+5OTSntLjm/wtTGB/7RORcd8=; b=rlMK/RGNr2cDirMBQmj+Q2HobZY+ZVcpRltja7ijVV8bJvqraqHeFZZJ7lpMXP5DTu4xpv mCXLS4wJYCZQl0B5I9bGZzIIGGjyWKZ86E3sLlqQJabdGocAaLVndNUC8MwVbgo33Bv3pf sVCv34zm2/txHzRqjOreXGILUcL2HI0Cc39jvnXFokTR4o4s6VTWnDX/2S51E8R9ZBkzjX NF6xceUpQFD3P0BpOYguid2et47IyjcF7H7yrGkxIi6B1c2+awPDWszrN8L7r6zN71ej4c BleWohQFyiWtS1p7qMr6n/0YptIG6SdSL1Sm1tNenO3Ct61H2nXNGPyEK2VAqg== 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 4Rctfs3n1yz488; Fri, 1 Sep 2023 22:55:49 +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 381MtnIc068111; Fri, 1 Sep 2023 22:55:49 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 381Mtnnn068108; Fri, 1 Sep 2023 22:55:49 GMT (envelope-from git) Date: Fri, 1 Sep 2023 22:55:49 GMT Message-Id: <202309012255.381Mtnnn068108@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: John Baldwin Subject: git: 4a9cd9fc22d7 - main - amd64 db_trace: Reject unaligned frame pointers List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@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/main X-Git-Reftype: branch X-Git-Commit: 4a9cd9fc22d7f87a27ccd3d41b93a0356cd7061c Auto-Submitted: auto-generated The branch main has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=4a9cd9fc22d7f87a27ccd3d41b93a0356cd7061c commit 4a9cd9fc22d7f87a27ccd3d41b93a0356cd7061c Author: John Baldwin AuthorDate: 2023-09-01 22:55:05 +0000 Commit: John Baldwin CommitDate: 2023-09-01 22:55:37 +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 --- 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 14bd893b6cc4..f9a0c2b6a03e 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 __nosanitizeaddress __nosanitizememory -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