Re: [PATCH 2/2] Fix again segfault in arm64_is_kernel_exception_frame() when corrupt stack pointer address is given

[lijiang <lijiang@xxxxxxxxxx>]

On Wed, Jun 7, 2023 at 8:00 PM <crash-utility-request@xxxxxxxxxx> wrote:

Date: Wed,  7 Jun 2023 18:37:34 +0900
From: HATAYAMA Daisuke <d.hatayama@xxxxxxxxxxx>
To: crash-utility@xxxxxxxxxx
Cc: d.hatayama@xxxxxxxxxxx
Subject: [PATCH 2/2] Fix again segfault in
        arm64_is_kernel_exception_frame() when corrupt stack pointer address
        is given
Message-ID: <20230607093734.247-2-d.hatayama@xxxxxxxxxxx>
Content-Type: text/plain; charset="US-ASCII"; x-default=true

This is the second trial from the commit
9868ebc8e648e5791764a51567a23efae7170d9b that was reverted at the
previous commit.

As described in the previous commit, result of STACK_OFFSET_TYPE() can
be an address out of bt->stackbuf and hence the address needs to be
checked prior to being referred to as an pt_regs object.

So, to fix the issue, let's check if stkptr points to within the range
of the kernel stack first.

Signed-off-by: HATAYAMA Daisuke <d.hatayama@xxxxxxxxxxx>
---
 arm64.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arm64.c b/arm64.c
index efbdccb..ca63fb5 100644
--- a/arm64.c
+++ b/arm64.c
@@ -2381,6 +2381,9 @@ arm64_is_kernel_exception_frame(struct bt_info *bt, ulong stkptr)
         struct arm64_pt_regs *regs;
        struct machine_specific *ms = machdep->machspec;

+       if (stkptr > STACKSIZE() && !INSTACK(stkptr, bt))
+               return FALSE;
+

I still have one question: Why does this one only need to be fixed, but the others are not needed(it won't be out of range)? The STACK_OFFSET_TYPE() is invoked multiple times in arm64.c, and similar calls can be seen on other arches(grep -nr "GET_STACK_ULONG" *.c or grep -nr "GET_STACK_DATA" *.c). 

# grep -nr "STACK_OFFSET_TYPE" *.c
arm64.c:2384:        regs = (struct arm64_pt_regs *)&bt->stackbuf[(ulong)(STACK_OFFSET_TYPE(stkptr))];
arm64.c:2821: ptregs = (struct arm64_pt_regs *)&bt->stackbuf[(ulong)(STACK_OFFSET_TYPE(orig_sp))];
arm64.c:3476: base = (ulong *)&bt->stackbuf[(ulong)(STACK_OFFSET_TYPE(bt->stackbase))];
arm64.c:3478: start = (ulong *)&bt->stackbuf[(ulong)(STACK_OFFSET_TYPE(bt->stacktop))];
arm64.c:3481: start = (ulong *)&bt->stackbuf[(ulong)(STACK_OFFSET_TYPE(frame->fp))];
arm64.c:3483: start = (ulong *)&bt->stackbuf[(ulong)(STACK_OFFSET_TYPE(bt->stacktop))];
arm64.c:3801: &bt->stackbuf[(ulong)(STACK_OFFSET_TYPE(sp))];
arm64.c:3822:       &bt->stackbuf[(ulong)(STACK_OFFSET_TYPE(pt_regs))];
x86.c:1075: if (STACK_OFFSET_TYPE(ep->eframe_addr) > STACKSIZE())
[root@hpe-apollo-cn99xx-13-vm-01 crash]# grep -nr "STACK_OFFSET_TYPE" *.h
defs.h:977:#define STACK_OFFSET_TYPE(OFF) \
defs.h:985: *((ulong *)((char *)(&bt->stackbuf[(ulong)(STACK_OFFSET_TYPE(OFF))])))
defs.h:988:    (void *)(&bt->stackbuf[(ulong)STACK_OFFSET_TYPE(OFF)]), (size_t)(SZ))

Thanks.

Lianbo

         regs = (struct arm64_pt_regs *)&bt->stackbuf[(ulong)(STACK_OFFSET_TYPE(stkptr))];

        if (INSTACK(regs->sp, bt) && INSTACK(regs->regs[29], bt) &&
--
2.25.1

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki