2024-02-26 10:15:14

by Rui Qi

[permalink] [raw]
Subject: [PATCH 0/3] Support intra-function call validation

Since kernel version 5.4.250 LTS, there has been an issue with the kernel live patching feature becoming unavailable. When compiling the sample code for kernel live patching, the following message is displayed when enabled:

livepatch: klp_check_stack: kworker/u256:6:23490 has an unreliable stack

After investigation, it was found that this is due to objtool not supporting intra-function calls, resulting in incorrect orc entry generation.

This patchset adds support for intra-function calls, allowing the kernel live patching feature to work correctly.

Alexandre Chartre (2):
objtool: is_fentry_call() crashes if call has no destination
objtool: Add support for intra-function calls

Rui Qi (1):
x86/speculation: Support intra-function call validation

arch/x86/include/asm/nospec-branch.h | 7 ++
include/linux/frame.h | 11 ++++
.../Documentation/stack-validation.txt | 8 +++
tools/objtool/arch/x86/decode.c | 6 ++
tools/objtool/check.c | 64 +++++++++++++++++--
5 files changed, 91 insertions(+), 5 deletions(-)

--
2.39.2 (Apple Git-143)



2024-02-26 10:25:33

by Rui Qi

[permalink] [raw]
Subject: [PATCH 2/3] objtool: Add support for intra-function calls

From: Alexandre Chartre <[email protected]>

commit 8aa8eb2a8f5b3305a95f39957dd2b715fa668e21 upstream.

Change objtool to support intra-function calls. On x86, an intra-function
call is represented in objtool as a push onto the stack (of the return
address), and a jump to the destination address. That way the stack
information is correctly updated and the call flow is still accurate.

Signed-off-by: Alexandre Chartre <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Miroslav Benes <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Rui Qi <[email protected]>
---
include/linux/frame.h | 11 ++++
.../Documentation/stack-validation.txt | 8 +++
tools/objtool/arch/x86/decode.c | 6 ++
tools/objtool/check.c | 62 +++++++++++++++++--
4 files changed, 83 insertions(+), 4 deletions(-)

diff --git a/include/linux/frame.h b/include/linux/frame.h
index 02d3ca2d9598..303cda600e56 100644
--- a/include/linux/frame.h
+++ b/include/linux/frame.h
@@ -15,9 +15,20 @@
static void __used __section(.discard.func_stack_frame_non_standard) \
*__func_stack_frame_non_standard_##func = func

+/*
+ * This macro indicates that the following intra-function call is valid.
+ * Any non-annotated intra-function call will cause objtool to issue a warning.
+ */
+#define ANNOTATE_INTRA_FUNCTION_CALL \
+ 999: \
+ .pushsection .discard.intra_function_calls; \
+ .long 999b; \
+ .popsection;
+
#else /* !CONFIG_STACK_VALIDATION */

#define STACK_FRAME_NON_STANDARD(func)
+#define ANNOTATE_INTRA_FUNCTION_CALL

#endif /* CONFIG_STACK_VALIDATION */

diff --git a/tools/objtool/Documentation/stack-validation.txt b/tools/objtool/Documentation/stack-validation.txt
index de094670050b..ee26bb382b70 100644
--- a/tools/objtool/Documentation/stack-validation.txt
+++ b/tools/objtool/Documentation/stack-validation.txt
@@ -290,6 +290,14 @@ they mean, and suggestions for how to fix them.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646


+11. file.o: warning: unannotated intra-function call
+
+ This warning means that a direct call is done to a destination which
+ is not at the beginning of a function. If this is a legit call, you
+ can remove this warning by putting the ANNOTATE_INTRA_FUNCTION_CALL
+ directive right before the call.
+
+
If the error doesn't seem to make sense, it could be a bug in objtool.
Feel free to ask the objtool maintainer for help.

diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index a62e032863a8..c3ff62c085c8 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -437,6 +437,12 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,

case 0xe8:
*type = INSN_CALL;
+ /*
+ * For the impact on the stack, a CALL behaves like
+ * a PUSH of an immediate value (the return address).
+ */
+ op->src.type = OP_SRC_CONST;
+ op->dest.type = OP_DEST_PUSH;
break;

case 0xfc:
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 71a24fd46dbd..0fa414869f45 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -645,6 +645,7 @@ static int add_jump_destinations(struct objtool_file *file)
return 0;
}

+
/*
* Find the destination instructions for all calls.
*/
@@ -666,10 +667,7 @@ static int add_call_destinations(struct objtool_file *file)
dest_off);

if (!insn->call_dest && !insn->ignore) {
- WARN_FUNC("unsupported intra-function call",
- insn->sec, insn->offset);
- if (retpoline)
- WARN("If this is a retpoline, please patch it in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE.");
+ WARN_FUNC("unannotated intra-function call", insn->sec, insn->offset);
return -1;
}

@@ -1291,6 +1289,58 @@ static int read_retpoline_hints(struct objtool_file *file)
return 0;
}

+
+static int read_intra_function_calls(struct objtool_file *file)
+{
+ struct instruction *insn;
+ struct section *sec;
+ struct rela *rela;
+
+ sec = find_section_by_name(file->elf, ".rela.discard.intra_function_calls");
+ if (!sec)
+ return 0;
+
+ list_for_each_entry(rela, &sec->rela_list, list) {
+ unsigned long dest_off;
+
+ if (rela->sym->type != STT_SECTION) {
+ WARN("unexpected relocation symbol type in %s",
+ sec->name);
+ return -1;
+ }
+
+ insn = find_insn(file, rela->sym->sec, rela->addend);
+ if (!insn) {
+ WARN("bad .discard.intra_function_call entry");
+ return -1;
+ }
+
+ if (insn->type != INSN_CALL) {
+ WARN_FUNC("intra_function_call not a direct call",
+ insn->sec, insn->offset);
+ return -1;
+ }
+
+ /*
+ * Treat intra-function CALLs as JMPs, but with a stack_op.
+ * See add_call_destinations(), which strips stack_ops from
+ * normal CALLs.
+ */
+ insn->type = INSN_JUMP_UNCONDITIONAL;
+
+ dest_off = insn->offset + insn->len + insn->immediate;
+ insn->jump_dest = find_insn(file, insn->sec, dest_off);
+ if (!insn->jump_dest) {
+ WARN_FUNC("can't find call dest at %s+0x%lx",
+ insn->sec, insn->offset,
+ insn->sec->name, dest_off);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
static void mark_rodata(struct objtool_file *file)
{
struct section *sec;
@@ -1346,6 +1396,10 @@ static int decode_sections(struct objtool_file *file)
if (ret)
return ret;

+ ret = read_intra_function_calls(file);
+ if (ret)
+ return ret;
+
ret = add_call_destinations(file);
if (ret)
return ret;
--
2.39.2 (Apple Git-143)


2024-02-26 11:34:53

by Rui Qi

[permalink] [raw]
Subject: Re: [PATCH 0/3] Support intra-function call validation

This issue only occurs in 5.4 LTS versions after LTS 5.4.250 (inclusive), and this patchset is based on commit 6e1f54a4985b63bc1b55a09e5e75a974c5d6719b (Linux 5.4.269)

On 2/26/24 5:49 PM, Rui Qi wrote:
> Since kernel version 5.4.250 LTS, there has been an issue with the kernel live patching feature becoming unavailable. When compiling the sample code for kernel live patching, the following message is displayed when enabled:
>
> livepatch: klp_check_stack: kworker/u256:6:23490 has an unreliable stack
>
> After investigation, it was found that this is due to objtool not supporting intra-function calls, resulting in incorrect orc entry generation.
>
> This patchset adds support for intra-function calls, allowing the kernel live patching feature to work correctly.
>
> Alexandre Chartre (2):
> objtool: is_fentry_call() crashes if call has no destination
> objtool: Add support for intra-function calls
>
> Rui Qi (1):
> x86/speculation: Support intra-function call validation
>
> arch/x86/include/asm/nospec-branch.h | 7 ++
> include/linux/frame.h | 11 ++++
> .../Documentation/stack-validation.txt | 8 +++
> tools/objtool/arch/x86/decode.c | 6 ++
> tools/objtool/check.c | 64 +++++++++++++++++--
> 5 files changed, 91 insertions(+), 5 deletions(-)
>

2024-02-26 17:28:55

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH 0/3] Support intra-function call validation

On Mon, Feb 26, 2024 at 07:33:53PM +0800, qirui wrote:
> This issue only occurs in 5.4 LTS versions after LTS 5.4.250
> (inclusive), and this patchset is based on commit
> 6e1f54a4985b63bc1b55a09e5e75a974c5d6719b (Linux 5.4.269)

Does the bug also exist in mainline? If not, why?

--
Josh

2024-02-28 02:51:14

by Rui Qi

[permalink] [raw]
Subject: Re: [External] Re: [PATCH 0/3] Support intra-function call validation

I tested the mainline kernel v6.8-rc5 without this problem, as I said before, this problem only occurs in 5.4 LTS, to be precise, it can occur from v5.4.217, with CONFIG_RETPOLINE and CONFIG_LIVEPATCH enabled

BTW: The patch for V2 version has been sent out. We can discuss based on that. Thank you!
https://lore.kernel.org/stable/[email protected]/T/#t

On 2/27/24 1:28 AM, Josh Poimboeuf wrote:
> On Mon, Feb 26, 2024 at 07:33:53PM +0800, qirui wrote:
>> This issue only occurs in 5.4 LTS versions after LTS 5.4.250
>> (inclusive), and this patchset is based on commit
>> 6e1f54a4985b63bc1b55a09e5e75a974c5d6719b (Linux 5.4.269)
>
> Does the bug also exist in mainline? If not, why?
>