[Crash-utility] Re: [PATCH v2] arm64: fix a potential segfault when unwind frame

Date: Wed, 24 Jul 2024 17:21:37 +1200 From: Tao Liu <ltao(a)redhat.com&gt; Subject: [Crash-utility] Re: [PATCH v2] arm64: fix a potential segfault when unwind frame To: qiwu.chen(a)transsion.com Cc: devel(a)lists.crash-utility.osci.io Message-ID: < CAO7dBbVRpMcyUcFumkeVUkE8qwBwb88qQhpwOVv8YbxdoCOymA(a)mail.gmail.com&gt; Content-Type: text/plain; charset="UTF-8" Hi qiwu, The patch LGTM, so ack.

Thanks, Tao Liu On Wed, Jul 24, 2024 at 1:41 PM <qiwu.chen(a)transsion.com&gt; wrote: > > The range of frame->fp is checked insufficiently, which may lead to a wrong > next fp. As a result, bt->stackbuf will be accessed out of range, and segfault. > > crash> bt > [Detaching after fork from child process 11409] > PID: 7661 TASK: ffffff81858aa500 CPU: 4 COMMAND: "sh" > #0 [ffffffc008003f50] local_cpu_stop at ffffffdd7669444c > > Thread 1 "crash" received signal SIGSEGV, Segmentation fault. > 0x00005555558266cc in arm64_unwind_frame (bt=0x7fffffffd8f0, frame=0x7fffffffd080) at > arm64.c:2821 > 2821 frame->fp = GET_STACK_ULONG(fp); > (gdb) bt > arm64.c:2821 > out>) at main.c:1338 > gdb_interface.c:81 > (gdb) p /x *(struct bt_info*) 0x7fffffffd8f0 > $3 = {task = 0xffffff81858aa500, flags = 0x0, instptr = 0xffffffdd76694450, stkptr = > 0xffffffc008003f40, bptr = 0x0, stackbase = 0xffffffc027288000, > stacktop = 0xffffffc02728c000, stackbuf = 0x555556115a40, tc = 0x55559d16fdc0, hp = 0x0, > textlist = 0x0, ref = 0x0, frameptr = 0xffffffc008003f50, > call_target = 0x0, machdep = 0x0, debug = 0x0, eframe_ip = 0x0, radix = 0x0, cpumask = > 0x0} > (gdb) p /x *(struct arm64_stackframe*) 0x7fffffffd080 > $4 = {fp = 0xffffffc008003f50, sp = 0xffffffc008003f60, pc = 0xffffffdd76694450} > crash> bt -S 0xffffffc008003f50 > PID: 7661 TASK: ffffff81858aa500 CPU: 4 COMMAND: "sh" > bt: non-process stack address for this task: ffffffc008003f50 > (valid range: ffffffc027288000 - ffffffc02728c000) > > Check frame->fp value sufficiently before access it. Only frame->fp within > the range of bt->stackbase and bt->stacktop will be regarded as valid. > > Signed-off-by: qiwu.chen <qiwu.chen(a)transsion.com&gt; > --- > arm64.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arm64.c b/arm64.c > index b3040d7..624dba2 100644 > --- a/arm64.c > +++ b/arm64.c > @@ -2814,7 +2814,7 @@ arm64_unwind_frame(struct bt_info *bt, struct arm64_stackframe *frame) > low = frame->sp; > high = (low + stack_mask) & ~(stack_mask); > > - if (fp < low || fp > high || fp & 0xf) > + if (fp < low || fp > high || fp & 0xf || !INSTACK(fp, bt)) > return FALSE; > > frame->sp = fp + 0x10; > @@ -3024,7 +3024,7 @@ arm64_unwind_frame_v2(struct bt_info *bt, struct arm64_stackframe *frame, > low = frame->sp; > high = (low + stack_mask) & ~(stack_mask); > > - if (fp < low || fp > high || fp & 0xf) > + if (fp < low || fp > high || fp & 0xf || !INSTACK(fp, bt)) > return FALSE; > > if (CRASHDEBUG(1)) > -- > 2.25.1