* [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe
@ 2025-03-20 17:15 Song Liu
2025-03-20 17:15 ` [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable Song Liu
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: Song Liu @ 2025-03-20 17:15 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, linux-toolchains, live-patching
Cc: indu.bhagat, puranjay, wnliu, irogers, joe.lawrence, jpoimboe,
mark.rutland, peterz, roman.gushchin, rostedt, will, kernel-team,
song
There are recent efforts to enable livepatch for arm64, with sframe [1] or
without sframe [2]. This set tries to enable livepatch without sframe. Some
of the code, however, are from [1].
Although the sframe implementation is more promising in longer term, it
suffers from the following issues:
1. sframe is not yet supported in llvm;
2. There is still bug in binutil [3], so that we cannot yet use sframe
with gcc;
3. sframe unwinder hasn't been fully verified in the kernel.
On the other hand, arm64 processors have become more and more important in
the data center world. Therefore, it is getting critical to support
livepatching of arm64 kernels.
With recent change in arm64 unwinder [4], it is possible to reliably
livepatch arm64 kernels without sframe. This is because we do not need
arch_stack_walk_reliable() to get reliable stack trace in all scenarios.
Instead, we only need arch_stack_walk_reliable() to detect when the
stack trace is not reliable, then the livepatch logic can retry the patch
transition at a later time.
Given the increasing need of livepatching, and relatively long time before
sframe is fully ready (for both gcc and clang), we would like to enable
livepatch without sframe.
Thanks!
[1] https://lore.kernel.org/live-patching/20250127213310.2496133-1-wnliu@google.com/
[2] https://lore.kernel.org/live-patching/20250129232936.1795412-1-song@kernel.org/
[3] https://sourceware.org/bugzilla/show_bug.cgi?id=32589
[4] https://lore.kernel.org/linux-arm-kernel/20241017092538.1859841-1-mark.rutland@arm.com/
Changes v2 => v3:
1. Remove a redundant check for -ENOENT. (Josh Poimboeuf)
2. Add Tested-by and Acked-by on v1. (I forgot to add them in v2.)
v2: https://lore.kernel.org/live-patching/20250319213707.1784775-1-song@kernel.org/
Changes v1 => v2:
1. Rework arch_stack_walk_reliable().
v1: https://lore.kernel.org/live-patching/20250308012742.3208215-1-song@kernel.org/
Song Liu (2):
arm64: Implement arch_stack_walk_reliable
arm64: Implement HAVE_LIVEPATCH
arch/arm64/Kconfig | 3 ++
arch/arm64/include/asm/thread_info.h | 4 +-
arch/arm64/kernel/entry-common.c | 4 ++
arch/arm64/kernel/stacktrace.c | 66 +++++++++++++++++++++-------
4 files changed, 60 insertions(+), 17 deletions(-)
--
2.47.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable
2025-03-20 17:15 [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe Song Liu
@ 2025-03-20 17:15 ` Song Liu
2025-03-20 17:46 ` Weinan Liu
` (4 more replies)
2025-03-20 17:15 ` [PATCH v3 2/2] arm64: Implement HAVE_LIVEPATCH Song Liu
` (2 subsequent siblings)
3 siblings, 5 replies; 21+ messages in thread
From: Song Liu @ 2025-03-20 17:15 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, linux-toolchains, live-patching
Cc: indu.bhagat, puranjay, wnliu, irogers, joe.lawrence, jpoimboe,
mark.rutland, peterz, roman.gushchin, rostedt, will, kernel-team,
song
With proper exception boundary detection, it is possible to implment
arch_stack_walk_reliable without sframe.
Note that, arch_stack_walk_reliable does not guarantee getting reliable
stack in all scenarios. Instead, it can reliably detect when the stack
trace is not reliable, which is enough to provide reliable livepatching.
Signed-off-by: Song Liu <song@kernel.org>
---
arch/arm64/Kconfig | 2 +-
arch/arm64/kernel/stacktrace.c | 66 +++++++++++++++++++++++++---------
2 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 701d980ea921..31d5e1ee6089 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -276,6 +276,7 @@ config ARM64
select HAVE_SOFTIRQ_ON_OWN_STACK
select USER_STACKTRACE_SUPPORT
select VDSO_GETRANDOM
+ select HAVE_RELIABLE_STACKTRACE
help
ARM 64-bit (AArch64) Linux support.
@@ -2500,4 +2501,3 @@ endmenu # "CPU Power Management"
source "drivers/acpi/Kconfig"
source "arch/arm64/kvm/Kconfig"
-
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 1d9d51d7627f..7e07911d8694 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -56,6 +56,7 @@ struct kunwind_state {
enum kunwind_source source;
union unwind_flags flags;
struct pt_regs *regs;
+ bool end_on_unreliable;
};
static __always_inline void
@@ -230,8 +231,26 @@ kunwind_next_frame_record(struct kunwind_state *state)
new_fp = READ_ONCE(record->fp);
new_pc = READ_ONCE(record->lr);
- if (!new_fp && !new_pc)
- return kunwind_next_frame_record_meta(state);
+ if (!new_fp && !new_pc) {
+ int ret;
+
+ ret = kunwind_next_frame_record_meta(state);
+ if (ret < 0) {
+ /*
+ * This covers two different conditions:
+ * 1. ret == -ENOENT, unwinding is done.
+ * 2. ret == -EINVAL, unwinding hit error.
+ */
+ return ret;
+ }
+ /*
+ * Searching across exception boundaries. The stack is now
+ * unreliable.
+ */
+ if (state->end_on_unreliable)
+ return -EINVAL;
+ return 0;
+ }
unwind_consume_stack(&state->common, info, fp, sizeof(*record));
@@ -277,21 +296,24 @@ kunwind_next(struct kunwind_state *state)
typedef bool (*kunwind_consume_fn)(const struct kunwind_state *state, void *cookie);
-static __always_inline void
+static __always_inline int
do_kunwind(struct kunwind_state *state, kunwind_consume_fn consume_state,
void *cookie)
{
- if (kunwind_recover_return_address(state))
- return;
+ int ret;
- while (1) {
- int ret;
+ ret = kunwind_recover_return_address(state);
+ if (ret)
+ return ret;
+ while (1) {
if (!consume_state(state, cookie))
- break;
+ return -EINVAL;
ret = kunwind_next(state);
+ if (ret == -ENOENT)
+ return 0;
if (ret < 0)
- break;
+ return ret;
}
}
@@ -324,10 +346,10 @@ do_kunwind(struct kunwind_state *state, kunwind_consume_fn consume_state,
: stackinfo_get_unknown(); \
})
-static __always_inline void
+static __always_inline int
kunwind_stack_walk(kunwind_consume_fn consume_state,
void *cookie, struct task_struct *task,
- struct pt_regs *regs)
+ struct pt_regs *regs, bool end_on_unreliable)
{
struct stack_info stacks[] = {
stackinfo_get_task(task),
@@ -348,11 +370,12 @@ kunwind_stack_walk(kunwind_consume_fn consume_state,
.stacks = stacks,
.nr_stacks = ARRAY_SIZE(stacks),
},
+ .end_on_unreliable = end_on_unreliable,
};
if (regs) {
if (task != current)
- return;
+ return -EINVAL;
kunwind_init_from_regs(&state, regs);
} else if (task == current) {
kunwind_init_from_caller(&state);
@@ -360,7 +383,7 @@ kunwind_stack_walk(kunwind_consume_fn consume_state,
kunwind_init_from_task(&state, task);
}
- do_kunwind(&state, consume_state, cookie);
+ return do_kunwind(&state, consume_state, cookie);
}
struct kunwind_consume_entry_data {
@@ -384,7 +407,18 @@ noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry,
.cookie = cookie,
};
- kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs);
+ kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs, false);
+}
+
+noinline noinstr int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry,
+ void *cookie, struct task_struct *task)
+{
+ struct kunwind_consume_entry_data data = {
+ .consume_entry = consume_entry,
+ .cookie = cookie,
+ };
+
+ return kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, NULL, true);
}
struct bpf_unwind_consume_entry_data {
@@ -409,7 +443,7 @@ noinline noinstr void arch_bpf_stack_walk(bool (*consume_entry)(void *cookie, u6
.cookie = cookie,
};
- kunwind_stack_walk(arch_bpf_unwind_consume_entry, &data, current, NULL);
+ kunwind_stack_walk(arch_bpf_unwind_consume_entry, &data, current, NULL, false);
}
static const char *state_source_string(const struct kunwind_state *state)
@@ -456,7 +490,7 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk,
return;
printk("%sCall trace:\n", loglvl);
- kunwind_stack_walk(dump_backtrace_entry, (void *)loglvl, tsk, regs);
+ kunwind_stack_walk(dump_backtrace_entry, (void *)loglvl, tsk, regs, false);
put_task_stack(tsk);
}
--
2.47.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 2/2] arm64: Implement HAVE_LIVEPATCH
2025-03-20 17:15 [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe Song Liu
2025-03-20 17:15 ` [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable Song Liu
@ 2025-03-20 17:15 ` Song Liu
2025-03-31 9:07 ` Andrea della Porta
2025-03-25 12:53 ` [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe Petr Mladek
2025-04-10 15:17 ` Petr Mladek
3 siblings, 1 reply; 21+ messages in thread
From: Song Liu @ 2025-03-20 17:15 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, linux-toolchains, live-patching
Cc: indu.bhagat, puranjay, wnliu, irogers, joe.lawrence, jpoimboe,
mark.rutland, peterz, roman.gushchin, rostedt, will, kernel-team,
song, Suraj Jitindar Singh, Torsten Duwe, Miroslav Benes,
Breno Leitao
This is largely based on [1] by Suraj Jitindar Singh.
Test coverage:
- Passed manual tests with samples/livepatch.
- Passed all but test-kprobe.sh in selftests/livepatch.
test-kprobe.sh is expected to fail, because arm64 doesn't have
KPROBES_ON_FTRACE.
- Passed tests with kpatch-build [2]. (This version includes commits that
are not merged to upstream kpatch yet).
[1] https://lore.kernel.org/all/20210604235930.603-1-surajjs@amazon.com/
[2] https://github.com/liu-song-6/kpatch/tree/fb-6.13
Cc: Suraj Jitindar Singh <surajjs@amazon.com>
Cc: Torsten Duwe <duwe@suse.de>
Signed-off-by: Song Liu <song@kernel.org>
Acked-by: Miroslav Benes <mbenes@suse.cz>
Tested-by: Breno Leitao <leitao@debian.org>
---
arch/arm64/Kconfig | 3 +++
arch/arm64/include/asm/thread_info.h | 4 +++-
arch/arm64/kernel/entry-common.c | 4 ++++
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 31d5e1ee6089..dbd237b13b21 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -277,6 +277,7 @@ config ARM64
select USER_STACKTRACE_SUPPORT
select VDSO_GETRANDOM
select HAVE_RELIABLE_STACKTRACE
+ select HAVE_LIVEPATCH
help
ARM 64-bit (AArch64) Linux support.
@@ -2501,3 +2502,5 @@ endmenu # "CPU Power Management"
source "drivers/acpi/Kconfig"
source "arch/arm64/kvm/Kconfig"
+
+source "kernel/livepatch/Kconfig"
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 1114c1c3300a..4ac42e13032b 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -64,6 +64,7 @@ void arch_setup_new_exec(void);
#define TIF_UPROBE 4 /* uprobe breakpoint or singlestep */
#define TIF_MTE_ASYNC_FAULT 5 /* MTE Asynchronous Tag Check Fault */
#define TIF_NOTIFY_SIGNAL 6 /* signal notifications exist */
+#define TIF_PATCH_PENDING 7 /* pending live patching update */
#define TIF_SYSCALL_TRACE 8 /* syscall trace active */
#define TIF_SYSCALL_AUDIT 9 /* syscall auditing */
#define TIF_SYSCALL_TRACEPOINT 10 /* syscall tracepoint for ftrace */
@@ -92,6 +93,7 @@ void arch_setup_new_exec(void);
#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT)
#define _TIF_SECCOMP (1 << TIF_SECCOMP)
#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU)
+#define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING)
#define _TIF_UPROBE (1 << TIF_UPROBE)
#define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP)
#define _TIF_32BIT (1 << TIF_32BIT)
@@ -103,7 +105,7 @@ void arch_setup_new_exec(void);
#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
_TIF_NOTIFY_RESUME | _TIF_FOREIGN_FPSTATE | \
_TIF_UPROBE | _TIF_MTE_ASYNC_FAULT | \
- _TIF_NOTIFY_SIGNAL)
+ _TIF_NOTIFY_SIGNAL | _TIF_PATCH_PENDING)
#define _TIF_SYSCALL_WORK (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
_TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP | \
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index b260ddc4d3e9..b537af333b42 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -8,6 +8,7 @@
#include <linux/context_tracking.h>
#include <linux/kasan.h>
#include <linux/linkage.h>
+#include <linux/livepatch.h>
#include <linux/lockdep.h>
#include <linux/ptrace.h>
#include <linux/resume_user_mode.h>
@@ -144,6 +145,9 @@ static void do_notify_resume(struct pt_regs *regs, unsigned long thread_flags)
(void __user *)NULL, current);
}
+ if (thread_flags & _TIF_PATCH_PENDING)
+ klp_update_patch_state(current);
+
if (thread_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
do_signal(regs);
--
2.47.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable
2025-03-20 17:15 ` [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable Song Liu
@ 2025-03-20 17:46 ` Weinan Liu
2025-03-20 17:54 ` Song Liu
2025-03-21 7:11 ` Josh Poimboeuf
` (3 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Weinan Liu @ 2025-03-20 17:46 UTC (permalink / raw)
To: song
Cc: indu.bhagat, irogers, joe.lawrence, jpoimboe, kernel-team,
linux-arm-kernel, linux-kernel, linux-toolchains, live-patching,
mark.rutland, peterz, puranjay, roman.gushchin, rostedt, will,
wnliu
On Thu, Mar 20, 2025 at 10:16 AM Song Liu <song@kernel.org> wrote:
>
> static __always_inline void
> @@ -230,8 +231,26 @@ kunwind_next_frame_record(struct kunwind_state *state)
> new_fp = READ_ONCE(record->fp);
> new_pc = READ_ONCE(record->lr);
>
> - if (!new_fp && !new_pc)
> - return kunwind_next_frame_record_meta(state);
> + if (!new_fp && !new_pc) {
> + int ret;
> +
> + ret = kunwind_next_frame_record_meta(state);
The exception case kunwind_next_regs_pc() will return 0 when unwind success.
Should we return a different value for the success case of kunwind_next_regs_pc()?
> + if (ret < 0) {
> + /*
> + * This covers two different conditions:
> + * 1. ret == -ENOENT, unwinding is done.
> + * 2. ret == -EINVAL, unwinding hit error.
> + */
> + return ret;
> + }
> + /*
> + * Searching across exception boundaries. The stack is now
> + * unreliable.
> + */
> + if (state->end_on_unreliable)
> + return -EINVAL;
> + return 0;
> + }
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable
2025-03-20 17:46 ` Weinan Liu
@ 2025-03-20 17:54 ` Song Liu
0 siblings, 0 replies; 21+ messages in thread
From: Song Liu @ 2025-03-20 17:54 UTC (permalink / raw)
To: Weinan Liu
Cc: indu.bhagat, irogers, joe.lawrence, jpoimboe, kernel-team,
linux-arm-kernel, linux-kernel, linux-toolchains, live-patching,
mark.rutland, peterz, puranjay, roman.gushchin, rostedt, will
On Thu, Mar 20, 2025 at 10:46 AM Weinan Liu <wnliu@google.com> wrote:
>
> On Thu, Mar 20, 2025 at 10:16 AM Song Liu <song@kernel.org> wrote:
> >
> > static __always_inline void
> > @@ -230,8 +231,26 @@ kunwind_next_frame_record(struct kunwind_state *state)
> > new_fp = READ_ONCE(record->fp);
> > new_pc = READ_ONCE(record->lr);
> >
> > - if (!new_fp && !new_pc)
> > - return kunwind_next_frame_record_meta(state);
> > + if (!new_fp && !new_pc) {
> > + int ret;
> > +
> > + ret = kunwind_next_frame_record_meta(state);
>
> The exception case kunwind_next_regs_pc() will return 0 when unwind success.
> Should we return a different value for the success case of kunwind_next_regs_pc()?
I am assuming once the unwinder hits an exception boundary, the stack is not
100% reliable. This does mean we will return -EINVAL for some reliable stack
walk, but this is safer and good enough for livepatch. IIUC, SFrame based
unwinder should not have this limitation.
Thanks,
Song
>
> > + if (ret < 0) {
> > + /*
> > + * This covers two different conditions:
> > + * 1. ret == -ENOENT, unwinding is done.
> > + * 2. ret == -EINVAL, unwinding hit error.
> > + */
> > + return ret;
> > + }
> > + /*
> > + * Searching across exception boundaries. The stack is now
> > + * unreliable.
> > + */
> > + if (state->end_on_unreliable)
> > + return -EINVAL;
> > + return 0;
> > + }
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable
2025-03-20 17:15 ` [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable Song Liu
2025-03-20 17:46 ` Weinan Liu
@ 2025-03-21 7:11 ` Josh Poimboeuf
2025-03-26 13:48 ` Miroslav Benes
` (2 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: Josh Poimboeuf @ 2025-03-21 7:11 UTC (permalink / raw)
To: Song Liu
Cc: linux-arm-kernel, linux-kernel, linux-toolchains, live-patching,
indu.bhagat, puranjay, wnliu, irogers, joe.lawrence, mark.rutland,
peterz, roman.gushchin, rostedt, will, kernel-team
On Thu, Mar 20, 2025 at 10:15:58AM -0700, Song Liu wrote:
> With proper exception boundary detection, it is possible to implment
> arch_stack_walk_reliable without sframe.
>
> Note that, arch_stack_walk_reliable does not guarantee getting reliable
> stack in all scenarios. Instead, it can reliably detect when the stack
> trace is not reliable, which is enough to provide reliable livepatching.
>
> Signed-off-by: Song Liu <song@kernel.org>
Reviewed-by: Josh Poimboeuf <jpoimboe@kernel.org>
--
Josh
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe
2025-03-20 17:15 [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe Song Liu
2025-03-20 17:15 ` [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable Song Liu
2025-03-20 17:15 ` [PATCH v3 2/2] arm64: Implement HAVE_LIVEPATCH Song Liu
@ 2025-03-25 12:53 ` Petr Mladek
2025-03-25 13:37 ` Song Liu
2025-04-10 15:17 ` Petr Mladek
3 siblings, 1 reply; 21+ messages in thread
From: Petr Mladek @ 2025-03-25 12:53 UTC (permalink / raw)
To: Song Liu
Cc: linux-arm-kernel, linux-kernel, linux-toolchains, live-patching,
indu.bhagat, puranjay, wnliu, irogers, joe.lawrence, jpoimboe,
mark.rutland, peterz, roman.gushchin, rostedt, will, kernel-team
On Thu 2025-03-20 10:15:57, Song Liu wrote:
> There are recent efforts to enable livepatch for arm64, with sframe [1] or
> without sframe [2]. This set tries to enable livepatch without sframe. Some
> of the code, however, are from [1].
>
> Although the sframe implementation is more promising in longer term, it
> suffers from the following issues:
>
> 1. sframe is not yet supported in llvm;
> 2. There is still bug in binutil [3], so that we cannot yet use sframe
> with gcc;
> 3. sframe unwinder hasn't been fully verified in the kernel.
>
> On the other hand, arm64 processors have become more and more important in
> the data center world. Therefore, it is getting critical to support
> livepatching of arm64 kernels.
>
> With recent change in arm64 unwinder [4], it is possible to reliably
> livepatch arm64 kernels without sframe. This is because we do not need
> arch_stack_walk_reliable() to get reliable stack trace in all scenarios.
> Instead, we only need arch_stack_walk_reliable() to detect when the
> stack trace is not reliable, then the livepatch logic can retry the patch
> transition at a later time.
>
> Given the increasing need of livepatching, and relatively long time before
> sframe is fully ready (for both gcc and clang), we would like to enable
> livepatch without sframe.
>
> Thanks!
>
> [1] https://lore.kernel.org/live-patching/20250127213310.2496133-1-wnliu@google.com/
> [2] https://lore.kernel.org/live-patching/20250129232936.1795412-1-song@kernel.org/
> [3] https://sourceware.org/bugzilla/show_bug.cgi?id=32589
> [4] https://lore.kernel.org/linux-arm-kernel/20241017092538.1859841-1-mark.rutland@arm.com/
Hi, I am sorry but I haven't found time to look at this in time before
the merge window. Is it acceptable to postpone this change to 6.16,
please?
Best Regards,
Petr
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe
2025-03-25 12:53 ` [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe Petr Mladek
@ 2025-03-25 13:37 ` Song Liu
0 siblings, 0 replies; 21+ messages in thread
From: Song Liu @ 2025-03-25 13:37 UTC (permalink / raw)
To: Petr Mladek
Cc: Song Liu, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-toolchains@vger.kernel.org,
live-patching@vger.kernel.org, indu.bhagat@oracle.com,
puranjay@kernel.org, wnliu@google.com, irogers@google.com,
joe.lawrence@redhat.com, jpoimboe@kernel.org,
mark.rutland@arm.com, peterz@infradead.org,
roman.gushchin@linux.dev, rostedt@goodmis.org, will@kernel.org,
Kernel Team
Hi Petr,
> On Mar 25, 2025, at 8:53 AM, Petr Mladek <pmladek@suse.com> wrote:
[...]
>>
>> Given the increasing need of livepatching, and relatively long time before
>> sframe is fully ready (for both gcc and clang), we would like to enable
>> livepatch without sframe.
>>
>> Thanks!
>>
>> [1] https://lore.kernel.org/live-patching/20250127213310.2496133-1-wnliu@google.com/
>> [2] https://lore.kernel.org/live-patching/20250129232936.1795412-1-song@kernel.org/
>> [3] https://urldefense.com/v3/__https://sourceware.org/bugzilla/show_bug.cgi?id=32589__;!!Bt8RZUm9aw!8CB9ByorcyTxDW3rQ6_GEeMTJN9rHWCydNdIW1FRb_2-LQ6RTrSerZq9E-s8kXEB12JJ_v07xkyc2w$
>> [4] https://lore.kernel.org/linux-arm-kernel/20241017092538.1859841-1-mark.rutland@arm.com/
>
> Hi, I am sorry but I haven't found time to look at this in time before
> the merge window. Is it acceptable to postpone this change to 6.16,
> please?
Yes, we can postpone this to 6.16. I will resend the patchset after
the merge window.
Thanks,
Song
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable
2025-03-20 17:15 ` [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable Song Liu
2025-03-20 17:46 ` Weinan Liu
2025-03-21 7:11 ` Josh Poimboeuf
@ 2025-03-26 13:48 ` Miroslav Benes
2025-03-31 9:06 ` Andrea della Porta
2025-05-19 13:41 ` Mark Rutland
4 siblings, 0 replies; 21+ messages in thread
From: Miroslav Benes @ 2025-03-26 13:48 UTC (permalink / raw)
To: Song Liu
Cc: linux-arm-kernel, linux-kernel, linux-toolchains, live-patching,
indu.bhagat, puranjay, wnliu, irogers, joe.lawrence, jpoimboe,
mark.rutland, peterz, roman.gushchin, rostedt, will, kernel-team
On Thu, 20 Mar 2025, Song Liu wrote:
> With proper exception boundary detection, it is possible to implment
> arch_stack_walk_reliable without sframe.
>
> Note that, arch_stack_walk_reliable does not guarantee getting reliable
> stack in all scenarios. Instead, it can reliably detect when the stack
> trace is not reliable, which is enough to provide reliable livepatching.
>
> Signed-off-by: Song Liu <song@kernel.org>
Looks good to me.
Reviewed-by: Miroslav Benes <mbenes@suse.cz>
M
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable
2025-03-20 17:15 ` [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable Song Liu
` (2 preceding siblings ...)
2025-03-26 13:48 ` Miroslav Benes
@ 2025-03-31 9:06 ` Andrea della Porta
2025-05-19 13:41 ` Mark Rutland
4 siblings, 0 replies; 21+ messages in thread
From: Andrea della Porta @ 2025-03-31 9:06 UTC (permalink / raw)
To: Song Liu
Cc: linux-arm-kernel, linux-kernel, linux-toolchains, live-patching,
indu.bhagat, puranjay, wnliu, irogers, joe.lawrence, jpoimboe,
mark.rutland, peterz, roman.gushchin, rostedt, will, kernel-team
Hi song,
On 10:15 Thu 20 Mar , Song Liu wrote:
> With proper exception boundary detection, it is possible to implment
> arch_stack_walk_reliable without sframe.
>
> Note that, arch_stack_walk_reliable does not guarantee getting reliable
> stack in all scenarios. Instead, it can reliably detect when the stack
> trace is not reliable, which is enough to provide reliable livepatching.
>
> Signed-off-by: Song Liu <song@kernel.org>
> ---
> arch/arm64/Kconfig | 2 +-
> arch/arm64/kernel/stacktrace.c | 66 +++++++++++++++++++++++++---------
> 2 files changed, 51 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 701d980ea921..31d5e1ee6089 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -276,6 +276,7 @@ config ARM64
> select HAVE_SOFTIRQ_ON_OWN_STACK
> select USER_STACKTRACE_SUPPORT
> select VDSO_GETRANDOM
> + select HAVE_RELIABLE_STACKTRACE
> help
> ARM 64-bit (AArch64) Linux support.
>
> @@ -2500,4 +2501,3 @@ endmenu # "CPU Power Management"
> source "drivers/acpi/Kconfig"
>
> source "arch/arm64/kvm/Kconfig"
> -
> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> index 1d9d51d7627f..7e07911d8694 100644
> --- a/arch/arm64/kernel/stacktrace.c
> +++ b/arch/arm64/kernel/stacktrace.c
> @@ -56,6 +56,7 @@ struct kunwind_state {
> enum kunwind_source source;
> union unwind_flags flags;
> struct pt_regs *regs;
> + bool end_on_unreliable;
> };
>
> static __always_inline void
> @@ -230,8 +231,26 @@ kunwind_next_frame_record(struct kunwind_state *state)
> new_fp = READ_ONCE(record->fp);
> new_pc = READ_ONCE(record->lr);
>
> - if (!new_fp && !new_pc)
> - return kunwind_next_frame_record_meta(state);
> + if (!new_fp && !new_pc) {
> + int ret;
> +
> + ret = kunwind_next_frame_record_meta(state);
> + if (ret < 0) {
> + /*
> + * This covers two different conditions:
> + * 1. ret == -ENOENT, unwinding is done.
> + * 2. ret == -EINVAL, unwinding hit error.
> + */
> + return ret;
> + }
> + /*
> + * Searching across exception boundaries. The stack is now
> + * unreliable.
> + */
> + if (state->end_on_unreliable)
> + return -EINVAL;
> + return 0;
> + }
>
> unwind_consume_stack(&state->common, info, fp, sizeof(*record));
>
> @@ -277,21 +296,24 @@ kunwind_next(struct kunwind_state *state)
>
> typedef bool (*kunwind_consume_fn)(const struct kunwind_state *state, void *cookie);
>
> -static __always_inline void
> +static __always_inline int
> do_kunwind(struct kunwind_state *state, kunwind_consume_fn consume_state,
> void *cookie)
> {
> - if (kunwind_recover_return_address(state))
> - return;
> + int ret;
>
> - while (1) {
> - int ret;
> + ret = kunwind_recover_return_address(state);
> + if (ret)
> + return ret;
>
> + while (1) {
> if (!consume_state(state, cookie))
> - break;
> + return -EINVAL;
> ret = kunwind_next(state);
> + if (ret == -ENOENT)
> + return 0;
> if (ret < 0)
> - break;
> + return ret;
> }
> }
>
> @@ -324,10 +346,10 @@ do_kunwind(struct kunwind_state *state, kunwind_consume_fn consume_state,
> : stackinfo_get_unknown(); \
> })
>
> -static __always_inline void
> +static __always_inline int
> kunwind_stack_walk(kunwind_consume_fn consume_state,
> void *cookie, struct task_struct *task,
> - struct pt_regs *regs)
> + struct pt_regs *regs, bool end_on_unreliable)
> {
> struct stack_info stacks[] = {
> stackinfo_get_task(task),
> @@ -348,11 +370,12 @@ kunwind_stack_walk(kunwind_consume_fn consume_state,
> .stacks = stacks,
> .nr_stacks = ARRAY_SIZE(stacks),
> },
> + .end_on_unreliable = end_on_unreliable,
> };
>
> if (regs) {
> if (task != current)
> - return;
> + return -EINVAL;
> kunwind_init_from_regs(&state, regs);
> } else if (task == current) {
> kunwind_init_from_caller(&state);
> @@ -360,7 +383,7 @@ kunwind_stack_walk(kunwind_consume_fn consume_state,
> kunwind_init_from_task(&state, task);
> }
>
> - do_kunwind(&state, consume_state, cookie);
> + return do_kunwind(&state, consume_state, cookie);
> }
>
> struct kunwind_consume_entry_data {
> @@ -384,7 +407,18 @@ noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry,
> .cookie = cookie,
> };
>
> - kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs);
> + kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs, false);
> +}
> +
> +noinline noinstr int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry,
> + void *cookie, struct task_struct *task)
> +{
> + struct kunwind_consume_entry_data data = {
> + .consume_entry = consume_entry,
> + .cookie = cookie,
> + };
> +
> + return kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, NULL, true);
> }
>
> struct bpf_unwind_consume_entry_data {
> @@ -409,7 +443,7 @@ noinline noinstr void arch_bpf_stack_walk(bool (*consume_entry)(void *cookie, u6
> .cookie = cookie,
> };
>
> - kunwind_stack_walk(arch_bpf_unwind_consume_entry, &data, current, NULL);
> + kunwind_stack_walk(arch_bpf_unwind_consume_entry, &data, current, NULL, false);
> }
>
> static const char *state_source_string(const struct kunwind_state *state)
> @@ -456,7 +490,7 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk,
> return;
>
> printk("%sCall trace:\n", loglvl);
> - kunwind_stack_walk(dump_backtrace_entry, (void *)loglvl, tsk, regs);
> + kunwind_stack_walk(dump_backtrace_entry, (void *)loglvl, tsk, regs, false);
>
> put_task_stack(tsk);
> }
> --
> 2.47.1
>
Tested-by: Andrea della Porta <andrea.porta@suse.com>
Thanks,
Andrea
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/2] arm64: Implement HAVE_LIVEPATCH
2025-03-20 17:15 ` [PATCH v3 2/2] arm64: Implement HAVE_LIVEPATCH Song Liu
@ 2025-03-31 9:07 ` Andrea della Porta
0 siblings, 0 replies; 21+ messages in thread
From: Andrea della Porta @ 2025-03-31 9:07 UTC (permalink / raw)
To: Song Liu
Cc: linux-arm-kernel, linux-kernel, linux-toolchains, live-patching,
indu.bhagat, puranjay, wnliu, irogers, joe.lawrence, jpoimboe,
mark.rutland, peterz, roman.gushchin, rostedt, will, kernel-team,
Suraj Jitindar Singh, Torsten Duwe, Miroslav Benes, Breno Leitao
On 10:15 Thu 20 Mar , Song Liu wrote:
> This is largely based on [1] by Suraj Jitindar Singh.
>
> Test coverage:
>
> - Passed manual tests with samples/livepatch.
> - Passed all but test-kprobe.sh in selftests/livepatch.
> test-kprobe.sh is expected to fail, because arm64 doesn't have
> KPROBES_ON_FTRACE.
> - Passed tests with kpatch-build [2]. (This version includes commits that
> are not merged to upstream kpatch yet).
>
> [1] https://lore.kernel.org/all/20210604235930.603-1-surajjs@amazon.com/
> [2] https://github.com/liu-song-6/kpatch/tree/fb-6.13
> Cc: Suraj Jitindar Singh <surajjs@amazon.com>
> Cc: Torsten Duwe <duwe@suse.de>
> Signed-off-by: Song Liu <song@kernel.org>
> Acked-by: Miroslav Benes <mbenes@suse.cz>
> Tested-by: Breno Leitao <leitao@debian.org>
> ---
> arch/arm64/Kconfig | 3 +++
> arch/arm64/include/asm/thread_info.h | 4 +++-
> arch/arm64/kernel/entry-common.c | 4 ++++
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 31d5e1ee6089..dbd237b13b21 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -277,6 +277,7 @@ config ARM64
> select USER_STACKTRACE_SUPPORT
> select VDSO_GETRANDOM
> select HAVE_RELIABLE_STACKTRACE
> + select HAVE_LIVEPATCH
> help
> ARM 64-bit (AArch64) Linux support.
>
> @@ -2501,3 +2502,5 @@ endmenu # "CPU Power Management"
> source "drivers/acpi/Kconfig"
>
> source "arch/arm64/kvm/Kconfig"
> +
> +source "kernel/livepatch/Kconfig"
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 1114c1c3300a..4ac42e13032b 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -64,6 +64,7 @@ void arch_setup_new_exec(void);
> #define TIF_UPROBE 4 /* uprobe breakpoint or singlestep */
> #define TIF_MTE_ASYNC_FAULT 5 /* MTE Asynchronous Tag Check Fault */
> #define TIF_NOTIFY_SIGNAL 6 /* signal notifications exist */
> +#define TIF_PATCH_PENDING 7 /* pending live patching update */
> #define TIF_SYSCALL_TRACE 8 /* syscall trace active */
> #define TIF_SYSCALL_AUDIT 9 /* syscall auditing */
> #define TIF_SYSCALL_TRACEPOINT 10 /* syscall tracepoint for ftrace */
> @@ -92,6 +93,7 @@ void arch_setup_new_exec(void);
> #define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT)
> #define _TIF_SECCOMP (1 << TIF_SECCOMP)
> #define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU)
> +#define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING)
> #define _TIF_UPROBE (1 << TIF_UPROBE)
> #define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP)
> #define _TIF_32BIT (1 << TIF_32BIT)
> @@ -103,7 +105,7 @@ void arch_setup_new_exec(void);
> #define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
> _TIF_NOTIFY_RESUME | _TIF_FOREIGN_FPSTATE | \
> _TIF_UPROBE | _TIF_MTE_ASYNC_FAULT | \
> - _TIF_NOTIFY_SIGNAL)
> + _TIF_NOTIFY_SIGNAL | _TIF_PATCH_PENDING)
>
> #define _TIF_SYSCALL_WORK (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
> _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP | \
> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> index b260ddc4d3e9..b537af333b42 100644
> --- a/arch/arm64/kernel/entry-common.c
> +++ b/arch/arm64/kernel/entry-common.c
> @@ -8,6 +8,7 @@
> #include <linux/context_tracking.h>
> #include <linux/kasan.h>
> #include <linux/linkage.h>
> +#include <linux/livepatch.h>
> #include <linux/lockdep.h>
> #include <linux/ptrace.h>
> #include <linux/resume_user_mode.h>
> @@ -144,6 +145,9 @@ static void do_notify_resume(struct pt_regs *regs, unsigned long thread_flags)
> (void __user *)NULL, current);
> }
>
> + if (thread_flags & _TIF_PATCH_PENDING)
> + klp_update_patch_state(current);
> +
> if (thread_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> do_signal(regs);
>
> --
> 2.47.1
>
Tested-by: Andrea della Porta <andrea.porta@suse.com>
Thanks,
Andrea
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe
2025-03-20 17:15 [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe Song Liu
` (2 preceding siblings ...)
2025-03-25 12:53 ` [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe Petr Mladek
@ 2025-04-10 15:17 ` Petr Mladek
2025-05-16 16:53 ` Song Liu
3 siblings, 1 reply; 21+ messages in thread
From: Petr Mladek @ 2025-04-10 15:17 UTC (permalink / raw)
To: Song Liu, mark.rutland
Cc: linux-arm-kernel, linux-kernel, linux-toolchains, live-patching,
indu.bhagat, puranjay, wnliu, irogers, joe.lawrence, jpoimboe,
peterz, roman.gushchin, rostedt, will, kernel-team
On Thu 2025-03-20 10:15:57, Song Liu wrote:
> There are recent efforts to enable livepatch for arm64, with sframe [1] or
> without sframe [2]. This set tries to enable livepatch without sframe. Some
> of the code, however, are from [1].
>
> Although the sframe implementation is more promising in longer term, it
> suffers from the following issues:
>
> 1. sframe is not yet supported in llvm;
> 2. There is still bug in binutil [3], so that we cannot yet use sframe
> with gcc;
> 3. sframe unwinder hasn't been fully verified in the kernel.
>
> On the other hand, arm64 processors have become more and more important in
> the data center world. Therefore, it is getting critical to support
> livepatching of arm64 kernels.
>
> With recent change in arm64 unwinder [4], it is possible to reliably
> livepatch arm64 kernels without sframe. This is because we do not need
> arch_stack_walk_reliable() to get reliable stack trace in all scenarios.
> Instead, we only need arch_stack_walk_reliable() to detect when the
> stack trace is not reliable, then the livepatch logic can retry the patch
> transition at a later time.
>
> Given the increasing need of livepatching, and relatively long time before
> sframe is fully ready (for both gcc and clang), we would like to enable
> livepatch without sframe.
>
> Thanks!
>
> [1] https://lore.kernel.org/live-patching/20250127213310.2496133-1-wnliu@google.com/
> [2] https://lore.kernel.org/live-patching/20250129232936.1795412-1-song@kernel.org/
> [3] https://sourceware.org/bugzilla/show_bug.cgi?id=32589
> [4] https://lore.kernel.org/linux-arm-kernel/20241017092538.1859841-1-mark.rutland@arm.com/
>
> Changes v2 => v3:
> 1. Remove a redundant check for -ENOENT. (Josh Poimboeuf)
> 2. Add Tested-by and Acked-by on v1. (I forgot to add them in v2.)
The approach and both patches look reasonable:
Reviewed-by: Petr Mladek <pmladek@suse.com>
Is anyone, Arm people, Mark, against pushing this into linux-next,
please?
Best Regards,
Petr
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe
2025-04-10 15:17 ` Petr Mladek
@ 2025-05-16 16:53 ` Song Liu
2025-05-19 12:57 ` Mark Rutland
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Song Liu @ 2025-05-16 16:53 UTC (permalink / raw)
To: Petr Mladek
Cc: mark.rutland, linux-arm-kernel, linux-kernel, linux-toolchains,
live-patching, indu.bhagat, puranjay, wnliu, irogers,
joe.lawrence, jpoimboe, peterz, roman.gushchin, rostedt, will,
kernel-team
On Thu, Apr 10, 2025 at 8:17 AM Petr Mladek <pmladek@suse.com> wrote:
>
[...]
> >
> > [1] https://lore.kernel.org/live-patching/20250127213310.2496133-1-wnliu@google.com/
> > [2] https://lore.kernel.org/live-patching/20250129232936.1795412-1-song@kernel.org/
> > [3] https://sourceware.org/bugzilla/show_bug.cgi?id=32589
> > [4] https://lore.kernel.org/linux-arm-kernel/20241017092538.1859841-1-mark.rutland@arm.com/
> >
> > Changes v2 => v3:
> > 1. Remove a redundant check for -ENOENT. (Josh Poimboeuf)
> > 2. Add Tested-by and Acked-by on v1. (I forgot to add them in v2.)
>
> The approach and both patches look reasonable:
>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
>
> Is anyone, Arm people, Mark, against pushing this into linux-next,
> please?
Ping.
ARM folks, please share your thoughts on this work. To fully support
livepatching of kernel modules, we also need [1]. We hope we can
land this in the 6.16 kernel.
Thanks,
Song
[1] https://lore.kernel.org/linux-arm-kernel/20250412010940.1686376-1-dylanbhatch@google.com/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe
2025-05-16 16:53 ` Song Liu
@ 2025-05-19 12:57 ` Mark Rutland
2025-05-19 14:22 ` Will Deacon
2025-05-19 16:40 ` Mark Rutland
2 siblings, 0 replies; 21+ messages in thread
From: Mark Rutland @ 2025-05-19 12:57 UTC (permalink / raw)
To: Song Liu
Cc: Petr Mladek, linux-arm-kernel, linux-kernel, linux-toolchains,
live-patching, indu.bhagat, puranjay, wnliu, irogers,
joe.lawrence, jpoimboe, peterz, roman.gushchin, rostedt, will,
kernel-team
On Fri, May 16, 2025 at 09:53:36AM -0700, Song Liu wrote:
> On Thu, Apr 10, 2025 at 8:17 AM Petr Mladek <pmladek@suse.com> wrote:
> >
> [...]
> > >
> > > [1] https://lore.kernel.org/live-patching/20250127213310.2496133-1-wnliu@google.com/
> > > [2] https://lore.kernel.org/live-patching/20250129232936.1795412-1-song@kernel.org/
> > > [3] https://sourceware.org/bugzilla/show_bug.cgi?id=32589
> > > [4] https://lore.kernel.org/linux-arm-kernel/20241017092538.1859841-1-mark.rutland@arm.com/
> > >
> > > Changes v2 => v3:
> > > 1. Remove a redundant check for -ENOENT. (Josh Poimboeuf)
> > > 2. Add Tested-by and Acked-by on v1. (I forgot to add them in v2.)
> >
> > The approach and both patches look reasonable:
> >
> > Reviewed-by: Petr Mladek <pmladek@suse.com>
> >
> > Is anyone, Arm people, Mark, against pushing this into linux-next,
> > please?
>
> Ping.
>
> ARM folks, please share your thoughts on this work. To fully support
> livepatching of kernel modules, we also need [1]. We hope we can
> land this in the 6.16 kernel.
Hi; apologies for the delay -- my time had been consumed by
FPSIMD/SVE/SME fixes and related non-upstream stuff for the last few
weeks, and now I'm catching up.
For the stacktrace side, I'm happy with enabling this without sframe,
and I hase some minor nits which we can either fix up now or as a
follow-up. I'll cover those in another reply, and chase up the module /
code-patching bit shortly.
Mark.
> Thanks,
> Song
>
> [1] https://lore.kernel.org/linux-arm-kernel/20250412010940.1686376-1-dylanbhatch@google.com/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable
2025-03-20 17:15 ` [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable Song Liu
` (3 preceding siblings ...)
2025-03-31 9:06 ` Andrea della Porta
@ 2025-05-19 13:41 ` Mark Rutland
2025-05-19 16:57 ` Song Liu
2025-05-20 14:28 ` Will Deacon
4 siblings, 2 replies; 21+ messages in thread
From: Mark Rutland @ 2025-05-19 13:41 UTC (permalink / raw)
To: Song Liu
Cc: linux-arm-kernel, linux-kernel, linux-toolchains, live-patching,
indu.bhagat, puranjay, wnliu, irogers, joe.lawrence, jpoimboe,
peterz, roman.gushchin, rostedt, will, kernel-team
On Thu, Mar 20, 2025 at 10:15:58AM -0700, Song Liu wrote:
> With proper exception boundary detection, it is possible to implment
> arch_stack_walk_reliable without sframe.
>
> Note that, arch_stack_walk_reliable does not guarantee getting reliable
> stack in all scenarios. Instead, it can reliably detect when the stack
> trace is not reliable, which is enough to provide reliable livepatching.
>
> Signed-off-by: Song Liu <song@kernel.org>
> ---
> arch/arm64/Kconfig | 2 +-
> arch/arm64/kernel/stacktrace.c | 66 +++++++++++++++++++++++++---------
> 2 files changed, 51 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 701d980ea921..31d5e1ee6089 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -276,6 +276,7 @@ config ARM64
> select HAVE_SOFTIRQ_ON_OWN_STACK
> select USER_STACKTRACE_SUPPORT
> select VDSO_GETRANDOM
> + select HAVE_RELIABLE_STACKTRACE
> help
> ARM 64-bit (AArch64) Linux support.
>
> @@ -2500,4 +2501,3 @@ endmenu # "CPU Power Management"
> source "drivers/acpi/Kconfig"
>
> source "arch/arm64/kvm/Kconfig"
> -
> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> index 1d9d51d7627f..7e07911d8694 100644
> --- a/arch/arm64/kernel/stacktrace.c
> +++ b/arch/arm64/kernel/stacktrace.c
> @@ -56,6 +56,7 @@ struct kunwind_state {
> enum kunwind_source source;
> union unwind_flags flags;
> struct pt_regs *regs;
> + bool end_on_unreliable;
> };
>
> static __always_inline void
> @@ -230,8 +231,26 @@ kunwind_next_frame_record(struct kunwind_state *state)
> new_fp = READ_ONCE(record->fp);
> new_pc = READ_ONCE(record->lr);
>
> - if (!new_fp && !new_pc)
> - return kunwind_next_frame_record_meta(state);
> + if (!new_fp && !new_pc) {
> + int ret;
> +
> + ret = kunwind_next_frame_record_meta(state);
> + if (ret < 0) {
> + /*
> + * This covers two different conditions:
> + * 1. ret == -ENOENT, unwinding is done.
> + * 2. ret == -EINVAL, unwinding hit error.
> + */
> + return ret;
> + }
> + /*
> + * Searching across exception boundaries. The stack is now
> + * unreliable.
> + */
> + if (state->end_on_unreliable)
> + return -EINVAL;
> + return 0;
> + }
My original expectation for this this was that we'd propogate the
errors, and then all the reliability logic would live un a consume_entry
wrapper like we have for BPF, e.g.
| static __always_inline bool
| arch_reliable_kunwind_consume_entry(const struct kunwind_state *state, void *cookie)
| {
| struct kunwind_consume_entry_data *data = cookie;
|
| /*
| * When unwinding across an exception boundary, the PC will be
| * reliable, but we do not know whether the FP is live, and so we
| * cannot perform the *next* unwind reliably.
| *
| * Give up as soon as we hit an exception boundary.
| */
| if (state->source == KUNWIND_SOURCE_REGS_PC)
| return false;
|
| return data->consume_entry(data->cookie, state->common.pc);
| }
|
| noinline noinstr int arch_stack_walk_reliable(stack_trace_consume_fn consume_entry,
| void *cookie,
| struct task_struct *task)
| {
| int ret;
| struct kunwind_consume_entry_data data = {
| .consume_entry = consume_entry,
| .cookie = cookie,
| };
|
| ret = kunwind_stack_walk(arch_reliable_kunwind_consume_entry, &data,
| task, NULL);
| return ret == -ENOENT ? 0 : ret;
| }
... and then in future we can add anything spdecific to reliable
stacktrace there.
That aside, this generally looks good to me. The only thing that I note
is that we're lacking a check on the return value of
kretprobe_find_ret_addr(), and we should return -EINVAL when that is
NULL, but that should never happen in normal operation.
I've pushed a arm64/stacktrace-updates branch [1] with fixups for those
as two separate commits atop this one. If that looks good to you I
suggest we post that as a series and ask Will and Catalin to take that
as-is.
I'll look at the actual patching bits now.
Mark.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/ arm64/stacktrace-updates
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe
2025-05-16 16:53 ` Song Liu
2025-05-19 12:57 ` Mark Rutland
@ 2025-05-19 14:22 ` Will Deacon
2025-05-19 16:40 ` Mark Rutland
2 siblings, 0 replies; 21+ messages in thread
From: Will Deacon @ 2025-05-19 14:22 UTC (permalink / raw)
To: Song Liu
Cc: Petr Mladek, mark.rutland, linux-arm-kernel, linux-kernel,
linux-toolchains, live-patching, indu.bhagat, puranjay, wnliu,
irogers, joe.lawrence, jpoimboe, peterz, roman.gushchin, rostedt,
kernel-team
On Fri, May 16, 2025 at 09:53:36AM -0700, Song Liu wrote:
> On Thu, Apr 10, 2025 at 8:17 AM Petr Mladek <pmladek@suse.com> wrote:
> >
> [...]
> > >
> > > [1] https://lore.kernel.org/live-patching/20250127213310.2496133-1-wnliu@google.com/
> > > [2] https://lore.kernel.org/live-patching/20250129232936.1795412-1-song@kernel.org/
> > > [3] https://sourceware.org/bugzilla/show_bug.cgi?id=32589
> > > [4] https://lore.kernel.org/linux-arm-kernel/20241017092538.1859841-1-mark.rutland@arm.com/
> > >
> > > Changes v2 => v3:
> > > 1. Remove a redundant check for -ENOENT. (Josh Poimboeuf)
> > > 2. Add Tested-by and Acked-by on v1. (I forgot to add them in v2.)
> >
> > The approach and both patches look reasonable:
> >
> > Reviewed-by: Petr Mladek <pmladek@suse.com>
> >
> > Is anyone, Arm people, Mark, against pushing this into linux-next,
> > please?
>
> Ping.
>
> ARM folks, please share your thoughts on this work. To fully support
> livepatching of kernel modules, we also need [1]. We hope we can
> land this in the 6.16 kernel.
>
> Thanks,
> Song
>
> [1] https://lore.kernel.org/linux-arm-kernel/20250412010940.1686376-1-dylanbhatch@google.com/
FWIW: I reviewed the patch above ([1]) already but didn't hear anything
back.
Will
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe
2025-05-16 16:53 ` Song Liu
2025-05-19 12:57 ` Mark Rutland
2025-05-19 14:22 ` Will Deacon
@ 2025-05-19 16:40 ` Mark Rutland
2025-05-19 17:11 ` Dylan Hatch
2 siblings, 1 reply; 21+ messages in thread
From: Mark Rutland @ 2025-05-19 16:40 UTC (permalink / raw)
To: Song Liu
Cc: Petr Mladek, linux-arm-kernel, linux-kernel, linux-toolchains,
live-patching, indu.bhagat, puranjay, wnliu, irogers,
joe.lawrence, jpoimboe, peterz, roman.gushchin, rostedt, will,
kernel-team
On Fri, May 16, 2025 at 09:53:36AM -0700, Song Liu wrote:
> ARM folks, please share your thoughts on this work. To fully support
> livepatching of kernel modules, we also need [1]. We hope we can
> land this in the 6.16 kernel.
>
> Thanks,
> Song
>
> [1] https://lore.kernel.org/linux-arm-kernel/20250412010940.1686376-1-dylanbhatch@google.com/
I've had a quick look at [1], and IIUC that's a hard prerequisite for
livepatching, as without it the kernel *will* crash if it attempts a
late module relocation.
Given that, I don't think that we can take patch 2 until Will's comments
on [1] have been addressed, but I think that we could take patch 1 (with
fixups) as per my other reply.
Mark.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable
2025-05-19 13:41 ` Mark Rutland
@ 2025-05-19 16:57 ` Song Liu
2025-05-20 14:28 ` Will Deacon
1 sibling, 0 replies; 21+ messages in thread
From: Song Liu @ 2025-05-19 16:57 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arm-kernel, linux-kernel, linux-toolchains, live-patching,
indu.bhagat, puranjay, wnliu, irogers, joe.lawrence, jpoimboe,
peterz, roman.gushchin, rostedt, will, kernel-team
Hi Mark,
Thanks for your review and the fixups!
On Mon, May 19, 2025 at 6:41 AM Mark Rutland <mark.rutland@arm.com> wrote:
>
[...]
>
> ... and then in future we can add anything spdecific to reliable
> stacktrace there.
>
> That aside, this generally looks good to me. The only thing that I note
> is that we're lacking a check on the return value of
> kretprobe_find_ret_addr(), and we should return -EINVAL when that is
> NULL, but that should never happen in normal operation.
>
> I've pushed a arm64/stacktrace-updates branch [1] with fixups for those
> as two separate commits atop this one. If that looks good to you I
> suggest we post that as a series and ask Will and Catalin to take that
> as-is.
>
> I'll look at the actual patching bits now.
>
> Mark.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/ arm64/stacktrace-updates
For the fixups:
Reviewed-and-tested-by: Song Liu <song@kernel.org>
Tested with 2/2 of this set and samples/livepatch.
Song
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe
2025-05-19 16:40 ` Mark Rutland
@ 2025-05-19 17:11 ` Dylan Hatch
0 siblings, 0 replies; 21+ messages in thread
From: Dylan Hatch @ 2025-05-19 17:11 UTC (permalink / raw)
To: Mark Rutland
Cc: Song Liu, Petr Mladek, linux-arm-kernel, linux-kernel,
linux-toolchains, live-patching, indu.bhagat, puranjay, wnliu,
irogers, joe.lawrence, jpoimboe, peterz, roman.gushchin, rostedt,
will, kernel-team
> FWIW: I reviewed the patch above ([1]) already but didn't hear anything
> back.
Sorry for the delay on this, last week was busier than expected on my
end. I'm aiming to send the revised patch within the next couple of
days.
On Mon, May 19, 2025 at 9:40 AM Mark Rutland <mark.rutland@arm.com> wrote:
> I've had a quick look at [1], and IIUC that's a hard prerequisite for
> livepatching, as without it the kernel *will* crash if it attempts a
> late module relocation.
>
This is correct. In both module-patch scenarios (module is loaded
first, or patch is loaded first) the relocations on the livepatch
module occur after it is already RX-only, so a crash is inevitable
with the current relocation code.
Thanks,
Dylan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable
2025-05-19 13:41 ` Mark Rutland
2025-05-19 16:57 ` Song Liu
@ 2025-05-20 14:28 ` Will Deacon
2025-05-20 16:59 ` Mark Rutland
1 sibling, 1 reply; 21+ messages in thread
From: Will Deacon @ 2025-05-20 14:28 UTC (permalink / raw)
To: Mark Rutland
Cc: Song Liu, linux-arm-kernel, linux-kernel, linux-toolchains,
live-patching, indu.bhagat, puranjay, wnliu, irogers,
joe.lawrence, jpoimboe, peterz, roman.gushchin, rostedt,
kernel-team
On Mon, May 19, 2025 at 02:41:06PM +0100, Mark Rutland wrote:
> I've pushed a arm64/stacktrace-updates branch [1] with fixups for those
> as two separate commits atop this one. If that looks good to you I
> suggest we post that as a series and ask Will and Catalin to take that
> as-is.
Yes, please post those to the list for review.
Will
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable
2025-05-20 14:28 ` Will Deacon
@ 2025-05-20 16:59 ` Mark Rutland
0 siblings, 0 replies; 21+ messages in thread
From: Mark Rutland @ 2025-05-20 16:59 UTC (permalink / raw)
To: Will Deacon
Cc: Song Liu, linux-arm-kernel, linux-kernel, linux-toolchains,
live-patching, indu.bhagat, puranjay, wnliu, irogers,
joe.lawrence, jpoimboe, peterz, roman.gushchin, rostedt,
kernel-team
On Tue, May 20, 2025 at 03:28:45PM +0100, Will Deacon wrote:
> On Mon, May 19, 2025 at 02:41:06PM +0100, Mark Rutland wrote:
> > I've pushed a arm64/stacktrace-updates branch [1] with fixups for those
> > as two separate commits atop this one. If that looks good to you I
> > suggest we post that as a series and ask Will and Catalin to take that
> > as-is.
>
> Yes, please post those to the list for review.
Sure; I'm just prepping that now...
Mark.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-05-20 17:02 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-20 17:15 [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe Song Liu
2025-03-20 17:15 ` [PATCH v3 1/2] arm64: Implement arch_stack_walk_reliable Song Liu
2025-03-20 17:46 ` Weinan Liu
2025-03-20 17:54 ` Song Liu
2025-03-21 7:11 ` Josh Poimboeuf
2025-03-26 13:48 ` Miroslav Benes
2025-03-31 9:06 ` Andrea della Porta
2025-05-19 13:41 ` Mark Rutland
2025-05-19 16:57 ` Song Liu
2025-05-20 14:28 ` Will Deacon
2025-05-20 16:59 ` Mark Rutland
2025-03-20 17:15 ` [PATCH v3 2/2] arm64: Implement HAVE_LIVEPATCH Song Liu
2025-03-31 9:07 ` Andrea della Porta
2025-03-25 12:53 ` [PATCH v3 0/2] arm64: livepatch: Enable livepatch without sframe Petr Mladek
2025-03-25 13:37 ` Song Liu
2025-04-10 15:17 ` Petr Mladek
2025-05-16 16:53 ` Song Liu
2025-05-19 12:57 ` Mark Rutland
2025-05-19 14:22 ` Will Deacon
2025-05-19 16:40 ` Mark Rutland
2025-05-19 17:11 ` Dylan Hatch
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).