From 91a76958e4a8a9fb67ac61166ff36e8dc961b3b9 Mon Sep 17 00:00:00 2001 From: HATAYAMA Daisuke Date: Wed, 7 Jun 2023 18:37:33 +0900 Subject: [PATCH 10/13] Revert "Fix segfault in arm64_is_kernel_exception_frame() when corrupt stack pointer address is given" This reverts commit 9868ebc8e648e5791764a51567a23efae7170d9b. The commit 9868ebc8e648e5791764a51567a23efae7170d9b causes the issue that bt command fails to show backtraces for the tasks that is running in the user mode at the moment of the kernel panic as follows: crash> bt 1734 PID: 1734 TASK: ffff000000392200 CPU: 4 COMMAND: "insmod" bt: invalid stack pointer is given The root cause is that while the commit added a sanity check into STACK_OFFSET_TYPE() to validate if a given candidate address of any interrupt or exception frame is contained within the range of the corresponding kernel stack, the premise that the STACK_OFFSET_TYPE() should not return out-of-the-buffer address, is wrong. Reexamining the relevant surrounding part of the backtracing code, it looks to me now that the STACK_OFFSET_TYPE() is originally expected to return an out-of-the-buffer address, like the address of the top of the corresponding kernel stack, e.g. at here: static int arm64_in_kdump_text(struct bt_info *bt, struct arm64_stackframe *frame) { ... if (bt->flags & BT_USER_SPACE) start = (ulong *)&bt->stackbuf[(ulong)(STACK_OFFSET_TYPE(bt->stacktop))]; else { Note that the above bt 1734 aborts here. Hence, the current implementation policy around STACK_OFFSET_TYPE() looks that the caller side is responsible for understanding the fact in advance and for avoiding making buffer overrun carefully. To fix this issue, revert the commit. Signed-off-by: HATAYAMA Daisuke Signed-off-by: Lianbo Jiang --- defs.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/defs.h b/defs.h index bfda0c48d37b..3e7d6cfbc6a8 100644 --- a/defs.h +++ b/defs.h @@ -976,10 +976,7 @@ struct bt_info { #define STACK_OFFSET_TYPE(OFF) \ (((ulong)(OFF) > STACKSIZE()) ? \ - (((ulong)(OFF) < (ulong)(bt->stackbase) || (ulong)(OFF) >= (ulong)(bt->stackbase) + STACKSIZE()) ? \ - error(FATAL, "invalid stack pointer is given\n") : \ - (ulong)((ulong)(OFF) - (ulong)(bt->stackbase))) : \ - (ulong)(OFF)) + (ulong)((ulong)(OFF) - (ulong)(bt->stackbase)) : (ulong)(OFF)) #define GET_STACK_ULONG(OFF) \ *((ulong *)((char *)(&bt->stackbuf[(ulong)(STACK_OFFSET_TYPE(OFF))]))) -- 2.37.1