On 12/26/23 14:43, [email protected] wrote:
Date: Tue, 26 Dec 2023 06:42:55 +0000
From: HAGIO KAZUHITO(萩尾 一仁)<[email protected]>
Subject: [Crash-utility] Re: [PATCH] x86_64: check bt->bptr before
calculate framesize
To: Tao Liu<[email protected]>,"[email protected]"
<[email protected]>
Message-ID:<[email protected]>
Content-Type: text/plain; charset="utf-8"
On 2023/12/26 10:19, Tao Liu wrote:
Previously the value of bt->bptr is not checked, which may led to a
wrong prev_sp and framesize. As a result, bt->stackbuf[] will be
accessed out of range, and segfault.
Before:
crash> set debug 1
crash> bt
...snip...
--- <NMI exception stack> ---
#8 [ffffffff9a603e10] __switch_to_asm at ffffffff99800214
rsp: ffffffff9a603e10 textaddr: ffffffff99800214 -> spo: 0 bpo: 0 spr: 0 bpr: 0
type: 0 end: 0
#9 [ffffffff9a603e40] __schedule at ffffffff9960dfb1
rsp: ffffffff9a603e40 textaddr: ffffffff9960dfb1 -> spo: 16 bpo: -16 spr: 4
bpr: 1 type: 0 end: 0
rsp: ffffffff9a603e40 rbp: ffffb9ca076e7ca8 prev_sp: ffffb9ca076e7cb8
framesize: 1829650024
Segmentation fault (core dumped)
(gdb) p/x bt->stackbase
$1 = 0xffffffff9a600000
(gdb) p/x bt->stacktop
$2 = 0xffffffff9a604000
After:
crash> set debug 1
crash> bt
...snip...
--- <NMI exception stack> ---
#8 [ffffffff9a603e10] __switch_to_asm at ffffffff99800214
rsp: ffffffff9a603e10 textaddr: ffffffff99800214 -> spo: 0 bpo: 0 spr: 0 bpr: 0
type: 0 end: 0
#9 [ffffffff9a603e40] __schedule at ffffffff9960dfb1
rsp: ffffffff9a603e40 textaddr: ffffffff9960dfb1 -> spo: 16 bpo: -16 spr: 4
bpr: 1 type: 0 end: 0
#10 [ffffffff9a603e98] schedule_idle at ffffffff9960e87c
rsp: ffffffff9a603e98 textaddr: ffffffff9960e87c -> spo: 8 bpo: 0 spr: 5 bpr: 0
type: 0 end: 0
rsp: ffffffff9a603e98 prev_sp: ffffffff9a603ea8 framesize: 0
...snip...
This patch will check bt->bptr value before calculate framesize. Only bt->bptr
falls into the range of bt->stackbase and bt->stacktop will be
regarded as valid.
Signed-off-by: Tao Liu<[email protected]>
---
x86_64.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/x86_64.c b/x86_64.c
index 8abe39d..ff1ba6e 100644
--- a/x86_64.c
+++ b/x86_64.c
@@ -8707,7 +8707,8 @@ x86_64_get_framesize(struct bt_info *bt, ulong textaddr,
ulong rsp, char *stack_
if (CRASHDEBUG(1))
fprintf(fp, "rsp: %lx prev_sp: %lx
framesize: %d\n",
rsp, prev_sp,
framesize);
- } else if ((korc->sp_reg == ORC_REG_BP) && bt->bptr) {
+ } else if ((korc->sp_reg == ORC_REG_BP) && bt->bptr &&
+ bt->bptr >= bt->stackbase && bt->bptr <=
bt->stacktop) {
Thank you for looking into this! Looks good to me.
Just a nit, INSTACK() can be used?
The INSTACK(bt->bptr, bt) is better, let's go with this, Kazu. Tao is on
PTO.
For the patch with the above change: Ack
Thanks.
Lianbo
Thanks,
Kazu
prev_sp = bt->bptr + korc->sp_offset;
framesize = (prev_sp - (rsp + 8) - 8);
if (CRASHDEBUG(1))
--
Crash-utility mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
