linux-kernel - [PATCH] Revert "x86/unwind/orc: Don't skip the first frame for inactive tasks"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220727031506.59322-1-chenzhongjin@huawei.com>
Date:   Wed, 27 Jul 2022 11:15:06 +0800
From:   Chen Zhongjin <chenzhongjin@...wei.com>
To:     <linux-kernel@...r.kernel.org>, <x86@...nel.org>,
        <linux-arch@...r.kernel.org>
CC:     <jpoimboe@...nel.org>, <peterz@...radead.org>,
        <tglx@...utronix.de>, <mingo@...hat.com>, <bp@...en8.de>,
        <dave.hansen@...ux.intel.com>, <hpa@...or.com>, <mbenes@...e.cz>,
        <chenzhongjin@...wei.com>
Subject: [PATCH] Revert "x86/unwind/orc: Don't skip the first frame for inactive tasks"

This reverts commit f1d9a2abff66aa8156fbc1493abed468db63ea48.

When CONFIG_GCOV_PROFILE_ALL is enabled, show_stack() and related
functions (e.g. dump_stack) will break for x86 ORC unwinder.

Call Trace:
 <TASK>
 ? dump_stack_lvl+0x83/0xb7
 ? schedule+0x1/0x190
 ? dump_stack+0x13/0x1f
 ? handler_pre0+0x3f/0x53 [kp_unwind]
 ...

show_trace_log_lvl() searches text address on stack to validate
whether unwind results are reliable. The code:

    for (; stack < stack_info.end; stack++) {
        ...
        if (stack == ret_addr_p)
            reliable = 1;
        ...
        if (!reliable)
            continue;
        ...
    }

This requires:

    *stack* <= ret_addr_p

So that the first ret_addr_p can be found when stack++.

In normal cases the frame of show_stack() should be optimized out.
However if it is not optimized such as CONFIG_GCOV_PROFILE_ALL=y,
unwind_start() will stop at show_stack(), where:

    state->sp == first_frame == *stack*

And this will causes:

    ret_addr_p = unwind_get_return_address_ptr = state->sp - 1
    => *stack* > ret_addr_p

Then reliable check will ignore all unwind because first ret_addr_p
can't be found.

'f1d9a2abff66 ("x86/unwind/orc: Don't skip the first frame for inactive tasks")'

This patch removed the equal condition when state->sp == first_frame
which makes frame of show_stack() not be skipped. But the reason to
do that is not established now:

'f2ac57a4c49d ("x86/unwind/orc: Fix inactive tasks with stack pointer in %sp on GCC 10 compiled kernels")'

    state->sp = first_frame + sizeof(*frame),

state->sp and first_frame can't be equal for inactive stack any more.

Regard this equal condition doesn't involve other cases now,
revert it to fix above problem.

After revert, stack can be printed right:

Call Trace:
 <TASK>
 dump_stack_lvl+0x83/0xb7
 ? schedule+0x1/0x190
 dump_stack+0x13/0x1f
 handler_pre0+0x3f/0x53 [kp_unwind]
 ...

Fixes: f1d9a2abff66 ("x86/unwind/orc: Don't skip the first frame for inactive tasks")
Signed-off-by: Chen Zhongjin <chenzhongjin@...wei.com>
---
 arch/x86/kernel/unwind_orc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 38185aedf7d1..514dc9ef99fe 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -708,7 +708,7 @@ void __unwind_start(struct unwind_state *state, struct task_struct *task,
 	/* Otherwise, skip ahead to the user-specified starting frame: */
 	while (!unwind_done(state) &&
 	       (!on_stack(&state->stack_info, first_frame, sizeof(long)) ||
-			state->sp < (unsigned long)first_frame))
+			state->sp <= (unsigned long)first_frame))
 		unwind_next_frame(state);

 	return;
-- 
2.17.1