Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1390507 > unrolled thread
| Started by | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| First post | 2016-04-28 22:50 +0200 |
| Last post | 2016-05-10 13:50 +0200 |
| Articles | 20 on this page of 72 — 11 participants |
Back to article view | Back to linux.kernel
[RFC PATCH v2 00/18] livepatch: hybrid consistency model Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-28 22:50 +0200
[RFC PATCH v2 14/18] livepatch: remove unnecessary object loaded check Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-28 22:50 +0200
[RFC PATCH v2 10/18] livepatch/powerpc: add TIF_PATCH_PENDING thread flag Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-28 22:50 +0200
Re: [RFC PATCH v2 10/18] livepatch/powerpc: add TIF_PATCH_PENDING thread flag Petr Mladek <pmladek@suse.com> - 2016-05-03 11:10 +0200
Re: [RFC PATCH v2 10/18] livepatch/powerpc: add TIF_PATCH_PENDING thread flag Miroslav Benes <mbenes@suse.cz> - 2016-05-03 14:10 +0200
[RFC PATCH v2 16/18] livepatch: store function sizes Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-28 22:50 +0200
[RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-28 22:50 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Andy Lutomirski <luto@amacapital.net> - 2016-04-29 20:10 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Andy Lutomirski <luto@amacapital.net> - 2016-04-29 22:20 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-29 22:30 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Andy Lutomirski <luto@amacapital.net> - 2016-04-29 22:40 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-29 23:30 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Andy Lutomirski <luto@amacapital.net> - 2016-04-29 23:40 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Jiri Kosina <jikos@kernel.org> - 2016-04-30 00:20 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-30 01:00 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Andy Lutomirski <luto@amacapital.net> - 2016-04-30 02:20 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-30 00:50 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Andy Lutomirski <luto@amacapital.net> - 2016-04-30 02:10 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-02 16:00 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Andy Lutomirski <luto@amacapital.net> - 2016-05-02 18:00 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-02 19:40 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Andy Lutomirski <luto@amacapital.net> - 2016-05-02 20:20 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Ingo Molnar <mingo@kernel.org> - 2016-05-02 20:40 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-02 21:50 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Jiri Kosina <jikos@kernel.org> - 2016-05-02 22:00 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Jiri Kosina <jikos@kernel.org> - 2016-05-02 22:10 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Andy Lutomirski <luto@amacapital.net> - 2016-05-03 02:50 +0200
RE: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking David Laight <David.Laight@ACULAB.COM> - 2016-05-04 17:20 +0200
Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-29 22:20 +0200
[RFC PATCH v2 09/18] livepatch/x86: add TIF_PATCH_PENDING thread flag Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-28 22:50 +0200
Re: [RFC PATCH v2 09/18] livepatch/x86: add TIF_PATCH_PENDING thread flag Andy Lutomirski <luto@amacapital.net> - 2016-04-29 20:10 +0200
Re: [RFC PATCH v2 09/18] livepatch/x86: add TIF_PATCH_PENDING thread flag Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-29 22:20 +0200
[RFC PATCH v2 02/18] x86/asm/head: use a common function for starting CPUs Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-28 22:50 +0200
[RFC PATCH v2 13/18] livepatch: separate enabled and patched states Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-28 22:50 +0200
Re: [RFC PATCH v2 13/18] livepatch: separate enabled and patched states Petr Mladek <pmladek@suse.com> - 2016-05-03 11:40 +0200
Re: [RFC PATCH v2 13/18] livepatch: separate enabled and patched states Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-03 15:50 +0200
[RFC PATCH v2 06/18] x86: dump_trace() error handling Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-28 22:50 +0200
Re: [RFC PATCH v2 06/18] x86: dump_trace() error handling Minfei Huang <mnghuan@gmail.com> - 2016-04-29 15:50 +0200
Re: [RFC PATCH v2 06/18] x86: dump_trace() error handling Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-29 16:10 +0200
[RFC PATCH v2 03/18] x86/asm/head: standardize the bottom of the stack for idle tasks Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-28 23:00 +0200
Re: [RFC PATCH v2 03/18] x86/asm/head: standardize the bottom of the stack for idle tasks Brian Gerst <brgerst@gmail.com> - 2016-04-29 20:50 +0200
Re: [RFC PATCH v2 03/18] x86/asm/head: standardize the bottom of the stack for idle tasks Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-29 22:30 +0200
Re: [RFC PATCH v2 03/18] x86/asm/head: standardize the bottom of the stack for idle tasks Andy Lutomirski <luto@kernel.org> - 2016-04-29 21:40 +0200
Re: [RFC PATCH v2 03/18] x86/asm/head: standardize the bottom of the stack for idle tasks Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-29 23:00 +0200
Re: [RFC PATCH v2 03/18] x86/asm/head: standardize the bottom of the stack for idle tasks Andy Lutomirski <luto@amacapital.net> - 2016-04-29 23:40 +0200
Re: [RFC PATCH v2 03/18] x86/asm/head: standardize the bottom of the stack for idle tasks Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-30 01:30 +0200
Re: [RFC PATCH v2 03/18] x86/asm/head: standardize the bottom of the stack for idle tasks Andy Lutomirski <luto@amacapital.net> - 2016-04-30 02:20 +0200
[RFC PATCH v2 04/18] x86: move _stext marker before head code Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-28 23:00 +0200
[RFC PATCH v2 01/18] x86/asm/head: clean up initial stack variable Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-28 23:00 +0200
Re: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Petr Mladek <pmladek@suse.com> - 2016-05-04 10:50 +0200
Re: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-04 18:00 +0200
Re: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Miroslav Benes <mbenes@suse.cz> - 2016-05-05 11:50 +0200
Re: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Petr Mladek <pmladek@suse.com> - 2016-05-05 15:10 +0200
barriers: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Petr Mladek <pmladek@suse.com> - 2016-05-04 14:40 +0200
Re: barriers: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Peter Zijlstra <peterz@infradead.org> - 2016-05-04 16:00 +0200
Re: barriers: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-04 19:00 +0200
Re: barriers: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Petr Mladek <pmladek@suse.com> - 2016-05-04 16:20 +0200
Re: barriers: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-04 19:30 +0200
Re: barriers: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Petr Mladek <pmladek@suse.com> - 2016-05-05 13:30 +0200
Re: barriers: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Miroslav Benes <mbenes@suse.cz> - 2016-05-09 17:50 +0200
Re: barriers: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-04 19:10 +0200
Re: barriers: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Petr Mladek <pmladek@suse.com> - 2016-05-05 12:30 +0200
klp_task_patch: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Petr Mladek <pmladek@suse.com> - 2016-05-04 16:50 +0200
Re: klp_task_patch: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Jiri Kosina <jikos@kernel.org> - 2016-05-04 17:00 +0200
Re: klp_task_patch: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-04 20:00 +0200
Re: klp_task_patch: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Petr Mladek <pmladek@suse.com> - 2016-05-05 14:00 +0200
Re: klp_task_patch: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-06 14:40 +0200
Re: klp_task_patch: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Petr Mladek <pmladek@suse.com> - 2016-05-09 14:30 +0200
Re: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Petr Mladek <pmladek@suse.com> - 2016-05-06 13:40 +0200
Re: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Josh Poimboeuf <jpoimboe@redhat.com> - 2016-05-06 14:50 +0200
Re: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Miroslav Benes <mbenes@suse.cz> - 2016-05-09 11:50 +0200
Re: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model Miroslav Benes <mbenes@suse.cz> - 2016-05-10 13:50 +0200
Page 1 of 4 [1] 2 3 4 Next page →
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-04-28 22:50 +0200 |
| Subject | [RFC PATCH v2 00/18] livepatch: hybrid consistency model |
| Message-ID | <rt12N-1W5-3@gated-at.bofh.it> |
This is v2 of the livepatch hybrid consistency model, based on
linux-next/master.
v1 of this patch set was posted over a year ago:
https://lkml.kernel.org/r/cover.1423499826.git.jpoimboe@redhat.com
The biggest complaint at that time was that stack traces are unreliable.
Since CONFIG_STACK_VALIDATION was merged, that issue has been addressed.
I've also tried to address all other outstanding complaints and issues.
Ingo and Peter, note that I'm using task_rq_lock() in patch 17/18 to
make sure a task stays asleep while its stack gets checked. I'm not
sure if there's a better way to achieve that goal -- any suggestions
there would be greatly appreciated.
Patches 1-7 create a mechanism for detecting whether a given stack trace
can be deemed reliable.
Patches 8-18 add the consistency model. See patch 17/18 for more
details about the consistency model itself.
Remaining TODOs:
- how to patch kthreads without RELIABLE_STACKTRACE?
- safe patch module removal
- fake signal facility
- allow user to force a task to the patched state
- enable the patching of kthreads which are sleeping on affected
functions, via the livepatch ftrace handler
- WARN on certain stack error conditions
v2:
- "universe" -> "patch state"
- rename klp_update_task_universe() -> klp_patch_task()
- add preempt IRQ tracking (TF_PREEMPT_IRQ)
- fix print_context_stack_reliable() bug
- improve print_context_stack_reliable() comments
- klp_ftrace_handler comment fixes
- add "patch_state" proc file to tid_base_stuff
- schedule work even for !RELIABLE_STACKTRACE
- forked child inherits patch state from parent
- add detailed comment to livepatch.h klp_func definition about the
klp_func patched/transition state transitions
- update exit_to_usermode_loop() comment
- clear all TIF_KLP_NEED_UPDATE flags in klp_complete_transition()
- remove unnecessary function externs
- add livepatch documentation, sysfs documentation, /proc documentation
- /proc/pid/patch_state: -1 means no patch is currently being applied/reverted
- "TIF_KLP_NEED_UPDATE" -> "TIF_PATCH_PENDING"
- support for s390 and powerpc-le
- don't assume stacks with dynamic ftrace trampolines are reliable
- add _TIF_ALLWORK_MASK info to commit log
v1.9:
- revive from the dead and rebased
- reliable stacks!
- add support for immediate consistency model
- add a ton of comments
- fix up memory barriers
- remove "allow patch modules to be removed" patch for now, it still
needs more discussion and thought - it can be done with something
- "proc/pid/universe" -> "proc/pid/patch_status"
- remove WARN_ON_ONCE from !func condition in ftrace handler -- can
happen because of RCU
- keep klp_mutex private by putting the work_fn in core.c
- convert states from int to boolean
- remove obsolete '@state' comments
- several header file and include improvements suggested by Jiri S
- change kallsyms_lookup_size_offset() errors from EINVAL -> ENOENT
- change proc file permissions S_IRUGO -> USR
- use klp_for_each_object/func helpers
Jiri Slaby (1):
livepatch/s390: reorganize TIF thread flag bits
Josh Poimboeuf (16):
x86/asm/head: clean up initial stack variable
x86/asm/head: use a common function for starting CPUs
x86/asm/head: standardize the bottom of the stack for idle tasks
x86: move _stext marker before head code
sched: add task flag for preempt IRQ tracking
x86: dump_trace() error handling
stacktrace/x86: function for detecting reliable stack traces
livepatch: temporary stubs for klp_patch_pending() and
klp_patch_task()
livepatch/x86: add TIF_PATCH_PENDING thread flag
livepatch/powerpc: add TIF_PATCH_PENDING thread flag
livepatch: separate enabled and patched states
livepatch: remove unnecessary object loaded check
livepatch: move patching functions into patch.c
livepatch: store function sizes
livepatch: change to a per-task consistency model
livepatch: add /proc/<pid>/patch_state
Miroslav Benes (1):
livepatch/s390: add TIF_PATCH_PENDING thread flag
Documentation/ABI/testing/sysfs-kernel-livepatch | 8 +
Documentation/filesystems/proc.txt | 18 +
Documentation/livepatch/livepatch.txt | 132 ++++++-
arch/Kconfig | 6 +
arch/powerpc/include/asm/thread_info.h | 4 +-
arch/powerpc/kernel/signal.c | 4 +
arch/s390/include/asm/thread_info.h | 24 +-
arch/s390/kernel/entry.S | 31 +-
arch/x86/Kconfig | 1 +
arch/x86/entry/common.c | 9 +-
arch/x86/include/asm/realmode.h | 2 +-
arch/x86/include/asm/smp.h | 3 -
arch/x86/include/asm/stacktrace.h | 36 +-
arch/x86/include/asm/thread_info.h | 2 +
arch/x86/kernel/acpi/sleep.c | 2 +-
arch/x86/kernel/dumpstack.c | 108 +++++-
arch/x86/kernel/dumpstack_32.c | 22 +-
arch/x86/kernel/dumpstack_64.c | 53 ++-
arch/x86/kernel/head_32.S | 8 +-
arch/x86/kernel/head_64.S | 34 +-
arch/x86/kernel/smpboot.c | 2 +-
arch/x86/kernel/stacktrace.c | 24 ++
arch/x86/kernel/vmlinux.lds.S | 2 +-
fs/proc/base.c | 15 +
include/linux/init_task.h | 9 +
include/linux/kernel.h | 1 +
include/linux/livepatch.h | 57 ++-
include/linux/sched.h | 4 +
include/linux/stacktrace.h | 20 +-
kernel/extable.c | 2 +-
kernel/fork.c | 5 +-
kernel/livepatch/Makefile | 2 +-
kernel/livepatch/core.c | 342 +++++-----------
kernel/livepatch/patch.c | 254 ++++++++++++
kernel/livepatch/patch.h | 33 ++
kernel/livepatch/transition.c | 474 +++++++++++++++++++++++
kernel/livepatch/transition.h | 14 +
kernel/sched/core.c | 4 +
kernel/sched/idle.c | 4 +
kernel/stacktrace.c | 4 +-
lib/Kconfig.debug | 6 +
41 files changed, 1413 insertions(+), 372 deletions(-)
create mode 100644 kernel/livepatch/patch.c
create mode 100644 kernel/livepatch/patch.h
create mode 100644 kernel/livepatch/transition.c
create mode 100644 kernel/livepatch/transition.h
--
2.4.11
[toc] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-04-28 22:50 +0200 |
| Subject | [RFC PATCH v2 14/18] livepatch: remove unnecessary object loaded check |
| Message-ID | <rt12P-1W5-25@gated-at.bofh.it> |
| In reply to | #1390507 |
klp_patch_object()'s callers already ensure that the object is loaded,
so its call to klp_is_object_loaded() is unnecessary.
This will also make it possible to move the patching code into a
separate file.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
kernel/livepatch/core.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 2b59230..2ad7892 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -469,9 +469,6 @@ static int klp_patch_object(struct klp_object *obj)
if (WARN_ON(obj->patched))
return -EINVAL;
- if (WARN_ON(!klp_is_object_loaded(obj)))
- return -EINVAL;
-
klp_for_each_func(obj, func) {
ret = klp_patch_func(func);
if (ret) {
--
2.4.11
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-04-28 22:50 +0200 |
| Subject | [RFC PATCH v2 10/18] livepatch/powerpc: add TIF_PATCH_PENDING thread flag |
| Message-ID | <rt12P-1W5-29@gated-at.bofh.it> |
| In reply to | #1390507 |
Add the TIF_PATCH_PENDING thread flag to enable the new livepatch per-task consistency model for powerpc. The bit getting set indicates the thread has a pending patch which needs to be applied when the thread exits the kernel. The bit is included in the _TIF_USER_WORK_MASK macro so that do_notify_resume() and klp_patch_task() get called when the bit is set. Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- arch/powerpc/include/asm/thread_info.h | 4 +++- arch/powerpc/kernel/signal.c | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h index 8febc3f..df262ca 100644 --- a/arch/powerpc/include/asm/thread_info.h +++ b/arch/powerpc/include/asm/thread_info.h @@ -88,6 +88,7 @@ static inline struct thread_info *current_thread_info(void) TIF_NEED_RESCHED */ #define TIF_32BIT 4 /* 32 bit binary */ #define TIF_RESTORE_TM 5 /* need to restore TM FP/VEC/VSX */ +#define TIF_PATCH_PENDING 6 /* pending live patching update */ #define TIF_SYSCALL_AUDIT 7 /* syscall auditing active */ #define TIF_SINGLESTEP 8 /* singlestepping active */ #define TIF_NOHZ 9 /* in adaptive nohz mode */ @@ -111,6 +112,7 @@ static inline struct thread_info *current_thread_info(void) #define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG) #define _TIF_32BIT (1<<TIF_32BIT) #define _TIF_RESTORE_TM (1<<TIF_RESTORE_TM) +#define _TIF_PATCH_PENDING (1<<TIF_PATCH_PENDING) #define _TIF_SYSCALL_AUDIT (1<<TIF_SYSCALL_AUDIT) #define _TIF_SINGLESTEP (1<<TIF_SINGLESTEP) #define _TIF_SECCOMP (1<<TIF_SECCOMP) @@ -127,7 +129,7 @@ static inline struct thread_info *current_thread_info(void) #define _TIF_USER_WORK_MASK (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \ _TIF_NOTIFY_RESUME | _TIF_UPROBE | \ - _TIF_RESTORE_TM) + _TIF_RESTORE_TM | _TIF_PATCH_PENDING) #define _TIF_PERSYSCALL_MASK (_TIF_RESTOREALL|_TIF_NOERROR) /* Bits in local_flags */ diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c index cb64d6f..844497b 100644 --- a/arch/powerpc/kernel/signal.c +++ b/arch/powerpc/kernel/signal.c @@ -14,6 +14,7 @@ #include <linux/uprobes.h> #include <linux/key.h> #include <linux/context_tracking.h> +#include <linux/livepatch.h> #include <asm/hw_breakpoint.h> #include <asm/uaccess.h> #include <asm/unistd.h> @@ -159,6 +160,9 @@ void do_notify_resume(struct pt_regs *regs, unsigned long thread_info_flags) tracehook_notify_resume(regs); } + if (thread_info_flags & _TIF_PATCH_PENDING) + klp_patch_task(current); + user_enter(); } -- 2.4.11
[toc] | [prev] | [next] | [standalone]
| From | Petr Mladek <pmladek@suse.com> |
|---|---|
| Date | 2016-05-03 11:10 +0200 |
| Subject | Re: [RFC PATCH v2 10/18] livepatch/powerpc: add TIF_PATCH_PENDING thread flag |
| Message-ID | <ruEv8-5nI-25@gated-at.bofh.it> |
| In reply to | #1390510 |
On Thu 2016-04-28 15:44:41, Josh Poimboeuf wrote: > Add the TIF_PATCH_PENDING thread flag to enable the new livepatch > per-task consistency model for powerpc. The bit getting set indicates > the thread has a pending patch which needs to be applied when the thread > exits the kernel. > > The bit is included in the _TIF_USER_WORK_MASK macro so that > do_notify_resume() and klp_patch_task() get called when the bit is set. > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > --- > arch/powerpc/include/asm/thread_info.h | 4 +++- > arch/powerpc/kernel/signal.c | 4 ++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h > index 8febc3f..df262ca 100644 > --- a/arch/powerpc/include/asm/thread_info.h > +++ b/arch/powerpc/include/asm/thread_info.h > @@ -88,6 +88,7 @@ static inline struct thread_info *current_thread_info(void) > TIF_NEED_RESCHED */ > #define TIF_32BIT 4 /* 32 bit binary */ > #define TIF_RESTORE_TM 5 /* need to restore TM FP/VEC/VSX */ > +#define TIF_PATCH_PENDING 6 /* pending live patching update */ > #define TIF_SYSCALL_AUDIT 7 /* syscall auditing active */ > #define TIF_SINGLESTEP 8 /* singlestepping active */ > #define TIF_NOHZ 9 /* in adaptive nohz mode */ > @@ -111,6 +112,7 @@ static inline struct thread_info *current_thread_info(void) > #define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG) > #define _TIF_32BIT (1<<TIF_32BIT) > #define _TIF_RESTORE_TM (1<<TIF_RESTORE_TM) > +#define _TIF_PATCH_PENDING (1<<TIF_PATCH_PENDING) > #define _TIF_SYSCALL_AUDIT (1<<TIF_SYSCALL_AUDIT) > #define _TIF_SINGLESTEP (1<<TIF_SINGLESTEP) > #define _TIF_SECCOMP (1<<TIF_SECCOMP) > @@ -127,7 +129,7 @@ static inline struct thread_info *current_thread_info(void) > > #define _TIF_USER_WORK_MASK (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \ > _TIF_NOTIFY_RESUME | _TIF_UPROBE | \ > - _TIF_RESTORE_TM) > + _TIF_RESTORE_TM | _TIF_PATCH_PENDING) > #define _TIF_PERSYSCALL_MASK (_TIF_RESTOREALL|_TIF_NOERROR) > > /* Bits in local_flags */ > diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c > index cb64d6f..844497b 100644 > --- a/arch/powerpc/kernel/signal.c > +++ b/arch/powerpc/kernel/signal.c > @@ -14,6 +14,7 @@ > #include <linux/uprobes.h> > #include <linux/key.h> > #include <linux/context_tracking.h> > +#include <linux/livepatch.h> > #include <asm/hw_breakpoint.h> > #include <asm/uaccess.h> > #include <asm/unistd.h> > @@ -159,6 +160,9 @@ void do_notify_resume(struct pt_regs *regs, unsigned long thread_info_flags) > tracehook_notify_resume(regs); > } > > + if (thread_info_flags & _TIF_PATCH_PENDING) > + klp_patch_task(current); > + JFYI, if we later add the fake signal to speed up migration of sleeping task, we would need to move this up before calling do_signal(). It would help to avoid cycling here twice. Mirek surely knows more details about it. Best Regards, Petr > user_enter(); > } > > -- > 2.4.11 >
[toc] | [prev] | [next] | [standalone]
| From | Miroslav Benes <mbenes@suse.cz> |
|---|---|
| Date | 2016-05-03 14:10 +0200 |
| Subject | Re: [RFC PATCH v2 10/18] livepatch/powerpc: add TIF_PATCH_PENDING thread flag |
| Message-ID | <ruHjl-7XO-15@gated-at.bofh.it> |
| In reply to | #1393216 |
On Tue, 3 May 2016, Petr Mladek wrote: > On Thu 2016-04-28 15:44:41, Josh Poimboeuf wrote: > > Add the TIF_PATCH_PENDING thread flag to enable the new livepatch > > per-task consistency model for powerpc. The bit getting set indicates > > the thread has a pending patch which needs to be applied when the thread > > exits the kernel. > > > > The bit is included in the _TIF_USER_WORK_MASK macro so that > > do_notify_resume() and klp_patch_task() get called when the bit is set. > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > --- > > arch/powerpc/include/asm/thread_info.h | 4 +++- > > arch/powerpc/kernel/signal.c | 4 ++++ > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h > > index 8febc3f..df262ca 100644 > > --- a/arch/powerpc/include/asm/thread_info.h > > +++ b/arch/powerpc/include/asm/thread_info.h > > @@ -88,6 +88,7 @@ static inline struct thread_info *current_thread_info(void) > > TIF_NEED_RESCHED */ > > #define TIF_32BIT 4 /* 32 bit binary */ > > #define TIF_RESTORE_TM 5 /* need to restore TM FP/VEC/VSX */ > > +#define TIF_PATCH_PENDING 6 /* pending live patching update */ > > #define TIF_SYSCALL_AUDIT 7 /* syscall auditing active */ > > #define TIF_SINGLESTEP 8 /* singlestepping active */ > > #define TIF_NOHZ 9 /* in adaptive nohz mode */ > > @@ -111,6 +112,7 @@ static inline struct thread_info *current_thread_info(void) > > #define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG) > > #define _TIF_32BIT (1<<TIF_32BIT) > > #define _TIF_RESTORE_TM (1<<TIF_RESTORE_TM) > > +#define _TIF_PATCH_PENDING (1<<TIF_PATCH_PENDING) > > #define _TIF_SYSCALL_AUDIT (1<<TIF_SYSCALL_AUDIT) > > #define _TIF_SINGLESTEP (1<<TIF_SINGLESTEP) > > #define _TIF_SECCOMP (1<<TIF_SECCOMP) > > @@ -127,7 +129,7 @@ static inline struct thread_info *current_thread_info(void) > > > > #define _TIF_USER_WORK_MASK (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \ > > _TIF_NOTIFY_RESUME | _TIF_UPROBE | \ > > - _TIF_RESTORE_TM) > > + _TIF_RESTORE_TM | _TIF_PATCH_PENDING) > > #define _TIF_PERSYSCALL_MASK (_TIF_RESTOREALL|_TIF_NOERROR) > > > > /* Bits in local_flags */ > > diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c > > index cb64d6f..844497b 100644 > > --- a/arch/powerpc/kernel/signal.c > > +++ b/arch/powerpc/kernel/signal.c > > @@ -14,6 +14,7 @@ > > #include <linux/uprobes.h> > > #include <linux/key.h> > > #include <linux/context_tracking.h> > > +#include <linux/livepatch.h> > > #include <asm/hw_breakpoint.h> > > #include <asm/uaccess.h> > > #include <asm/unistd.h> > > @@ -159,6 +160,9 @@ void do_notify_resume(struct pt_regs *regs, unsigned long thread_info_flags) > > tracehook_notify_resume(regs); > > } > > > > + if (thread_info_flags & _TIF_PATCH_PENDING) > > + klp_patch_task(current); > > + > > JFYI, if we later add the fake signal to speed up migration of > sleeping task, we would need to move this up before calling > do_signal(). It would help to avoid cycling here twice. > Mirek surely knows more details about it. Yes, that is true if we go with a fake signal implementation we have in kGraft. Nevertheless it is the issue of that patch so let's discuss that there (when I send it). Miroslav
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-04-28 22:50 +0200 |
| Subject | [RFC PATCH v2 16/18] livepatch: store function sizes |
| Message-ID | <rt12P-1W5-23@gated-at.bofh.it> |
| In reply to | #1390507 |
For the consistency model we'll need to know the sizes of the old and
new functions to determine if they're on the stacks of any tasks.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
include/linux/livepatch.h | 3 +++
kernel/livepatch/core.c | 16 ++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 9ba26c5..c38c694 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -37,6 +37,8 @@
* @old_addr: the address of the function being patched
* @kobj: kobject for sysfs resources
* @stack_node: list node for klp_ops func_stack list
+ * @old_size: size of the old function
+ * @new_size: size of the new function
* @patched: the func has been added to the klp_ops list
*/
struct klp_func {
@@ -56,6 +58,7 @@ struct klp_func {
unsigned long old_addr;
struct kobject kobj;
struct list_head stack_node;
+ unsigned long old_size, new_size;
bool patched;
};
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index f28504d..aa3dbdf 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -577,6 +577,22 @@ static int klp_init_object_loaded(struct klp_patch *patch,
&func->old_addr);
if (ret)
return ret;
+
+ ret = kallsyms_lookup_size_offset(func->old_addr,
+ &func->old_size, NULL);
+ if (!ret) {
+ pr_err("kallsyms size lookup failed for '%s'\n",
+ func->old_name);
+ return -ENOENT;
+ }
+
+ ret = kallsyms_lookup_size_offset((unsigned long)func->new_func,
+ &func->new_size, NULL);
+ if (!ret) {
+ pr_err("kallsyms size lookup failed for '%s' replacement\n",
+ func->old_name);
+ return -ENOENT;
+ }
}
return 0;
--
2.4.11
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-04-28 22:50 +0200 |
| Subject | [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <rt12P-1W5-33@gated-at.bofh.it> |
| In reply to | #1390507 |
A preempted function might not have had a chance to save the frame
pointer to the stack yet, which can result in its caller getting skipped
on a stack trace.
Add a flag to indicate when the task has been preempted so that stack
dump code can determine whether the stack trace is reliable.
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
include/linux/sched.h | 1 +
kernel/fork.c | 2 +-
kernel/sched/core.c | 4 ++++
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 3d31572..fb364a0 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2137,6 +2137,7 @@ extern void thread_group_cputime_adjusted(struct task_struct *p, cputime_t *ut,
#define PF_SWAPWRITE 0x00800000 /* Allowed to write to swap */
#define PF_NO_SETAFFINITY 0x04000000 /* Userland is not allowed to meddle with cpus_allowed */
#define PF_MCE_EARLY 0x08000000 /* Early kill for mce process policy */
+#define PF_PREEMPT_IRQ 0x10000000 /* Thread is preempted by an irq */
#define PF_MUTEX_TESTER 0x20000000 /* Thread belongs to the rt mutex tester */
#define PF_FREEZER_SKIP 0x40000000 /* Freezer should not count it as freezable */
#define PF_SUSPEND_TASK 0x80000000 /* this thread called freeze_processes and should not be frozen */
diff --git a/kernel/fork.c b/kernel/fork.c
index b73a539..d2fe04a 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1373,7 +1373,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
goto bad_fork_cleanup_count;
delayacct_tsk_init(p); /* Must remain after dup_task_struct() */
- p->flags &= ~(PF_SUPERPRIV | PF_WQ_WORKER);
+ p->flags &= ~(PF_SUPERPRIV | PF_WQ_WORKER | PF_PREEMPT_IRQ);
p->flags |= PF_FORKNOEXEC;
INIT_LIST_HEAD(&p->children);
INIT_LIST_HEAD(&p->sibling);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9d84d60..7594267 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3422,6 +3422,8 @@ asmlinkage __visible void __sched preempt_schedule_irq(void)
prev_state = exception_enter();
+ current->flags |= PF_PREEMPT_IRQ;
+
do {
preempt_disable();
local_irq_enable();
@@ -3430,6 +3432,8 @@ asmlinkage __visible void __sched preempt_schedule_irq(void)
sched_preempt_enable_no_resched();
} while (need_resched());
+ current->flags &= ~PF_PREEMPT_IRQ;
+
exception_exit(prev_state);
}
--
2.4.11
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-04-29 20:10 +0200 |
| Subject | Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <rtl1w-1Wz-15@gated-at.bofh.it> |
| In reply to | #1390513 |
On Thu, Apr 28, 2016 at 1:44 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > A preempted function might not have had a chance to save the frame > pointer to the stack yet, which can result in its caller getting skipped > on a stack trace. > > Add a flag to indicate when the task has been preempted so that stack > dump code can determine whether the stack trace is reliable. I think I like this, but how do you handle the rather similar case in which a task goes to sleep because it's waiting on IO that happened in response to get_user, put_user, copy_from_user, etc? --Andy
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-04-29 22:20 +0200 |
| Subject | Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <rtn3k-3Ce-11@gated-at.bofh.it> |
| In reply to | #1391318 |
On Fri, Apr 29, 2016 at 1:11 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > On Fri, Apr 29, 2016 at 11:06:53AM -0700, Andy Lutomirski wrote: >> On Thu, Apr 28, 2016 at 1:44 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >> > A preempted function might not have had a chance to save the frame >> > pointer to the stack yet, which can result in its caller getting skipped >> > on a stack trace. >> > >> > Add a flag to indicate when the task has been preempted so that stack >> > dump code can determine whether the stack trace is reliable. >> >> I think I like this, but how do you handle the rather similar case in >> which a task goes to sleep because it's waiting on IO that happened in >> response to get_user, put_user, copy_from_user, etc? > > Hm, good question. I was thinking that page faults had a dedicated > stack, but now looking at the entry and traps code, that doesn't seem to > be the case. > > Anyway I think it shouldn't be a problem if we make sure that any kernel > function which might trigger a valid page fault (e.g., > copy_user_generic_string) do the proper frame pointer setup first. Then > the stack should still be reliable. > > In fact I might be able to teach objtool to enforce that: any function > which uses an exception table should create a stack frame. > > Or alternatively, maybe set some kind of flag for page faults, similar > to what I did with this patch. > How about doing it the other way around: teach the unwinder to detect when it hits a non-outermost entry (i.e. it lands in idtentry, etc) and use some reasonable heuristic as to whether it's okay to keep unwinding. You should be able to handle preemption like that, too -- the unwind process will end up in an IRQ frame. --Andy
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-04-29 22:30 +0200 |
| Subject | Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <rtnd0-3HL-37@gated-at.bofh.it> |
| In reply to | #1391385 |
On Fri, Apr 29, 2016 at 01:19:23PM -0700, Andy Lutomirski wrote: > On Fri, Apr 29, 2016 at 1:11 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > On Fri, Apr 29, 2016 at 11:06:53AM -0700, Andy Lutomirski wrote: > >> On Thu, Apr 28, 2016 at 1:44 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > >> > A preempted function might not have had a chance to save the frame > >> > pointer to the stack yet, which can result in its caller getting skipped > >> > on a stack trace. > >> > > >> > Add a flag to indicate when the task has been preempted so that stack > >> > dump code can determine whether the stack trace is reliable. > >> > >> I think I like this, but how do you handle the rather similar case in > >> which a task goes to sleep because it's waiting on IO that happened in > >> response to get_user, put_user, copy_from_user, etc? > > > > Hm, good question. I was thinking that page faults had a dedicated > > stack, but now looking at the entry and traps code, that doesn't seem to > > be the case. > > > > Anyway I think it shouldn't be a problem if we make sure that any kernel > > function which might trigger a valid page fault (e.g., > > copy_user_generic_string) do the proper frame pointer setup first. Then > > the stack should still be reliable. > > > > In fact I might be able to teach objtool to enforce that: any function > > which uses an exception table should create a stack frame. > > > > Or alternatively, maybe set some kind of flag for page faults, similar > > to what I did with this patch. > > > > How about doing it the other way around: teach the unwinder to detect > when it hits a non-outermost entry (i.e. it lands in idtentry, etc) > and use some reasonable heuristic as to whether it's okay to keep > unwinding. You should be able to handle preemption like that, too -- > the unwind process will end up in an IRQ frame. How exactly would the unwinder detect if a text address is in an idtentry? Maybe put all the idt entries in a special ELF section? -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-04-29 22:40 +0200 |
| Subject | Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <rtnmG-3LT-21@gated-at.bofh.it> |
| In reply to | #1391406 |
On Fri, Apr 29, 2016 at 1:27 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > On Fri, Apr 29, 2016 at 01:19:23PM -0700, Andy Lutomirski wrote: >> On Fri, Apr 29, 2016 at 1:11 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >> > On Fri, Apr 29, 2016 at 11:06:53AM -0700, Andy Lutomirski wrote: >> >> On Thu, Apr 28, 2016 at 1:44 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >> >> > A preempted function might not have had a chance to save the frame >> >> > pointer to the stack yet, which can result in its caller getting skipped >> >> > on a stack trace. >> >> > >> >> > Add a flag to indicate when the task has been preempted so that stack >> >> > dump code can determine whether the stack trace is reliable. >> >> >> >> I think I like this, but how do you handle the rather similar case in >> >> which a task goes to sleep because it's waiting on IO that happened in >> >> response to get_user, put_user, copy_from_user, etc? >> > >> > Hm, good question. I was thinking that page faults had a dedicated >> > stack, but now looking at the entry and traps code, that doesn't seem to >> > be the case. >> > >> > Anyway I think it shouldn't be a problem if we make sure that any kernel >> > function which might trigger a valid page fault (e.g., >> > copy_user_generic_string) do the proper frame pointer setup first. Then >> > the stack should still be reliable. >> > >> > In fact I might be able to teach objtool to enforce that: any function >> > which uses an exception table should create a stack frame. >> > >> > Or alternatively, maybe set some kind of flag for page faults, similar >> > to what I did with this patch. >> > >> >> How about doing it the other way around: teach the unwinder to detect >> when it hits a non-outermost entry (i.e. it lands in idtentry, etc) >> and use some reasonable heuristic as to whether it's okay to keep >> unwinding. You should be able to handle preemption like that, too -- >> the unwind process will end up in an IRQ frame. > > How exactly would the unwinder detect if a text address is in an > idtentry? Maybe put all the idt entries in a special ELF section? > Hmm. What actually happens when you unwind all the way into the entry code? Don't you end up in something that isn't in an ELF function? Can you detect that? Ideally, the unwinder could actually detect that it's hit a pt_regs struct and report that. If used for stack dumps, it could display some indication of this and then continue its unwinding by decoding the pt_regs. If used for patching, it could take some other appropriate action. I would have no objection to annotating all the pt_regs-style entry code, whether by putting it in a separate section or by making a table of addresses. There are a couple of nasty cases if NMI or MCE is involved but, as of 4.6, outside of NMI, MCE, and vmalloc faults (ugh!), there should always be a complete pt_regs on the stack before interrupts get enabled for each entry. Of course, finding the thing may be nontrivial in case other things were pushed. I suppose we could try to rejigger the code so that rbp points to pt_regs or similar. --Andy
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-04-29 23:30 +0200 |
| Subject | Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <rto93-4rM-5@gated-at.bofh.it> |
| In reply to | #1391411 |
On Fri, Apr 29, 2016 at 01:32:53PM -0700, Andy Lutomirski wrote: > On Fri, Apr 29, 2016 at 1:27 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > On Fri, Apr 29, 2016 at 01:19:23PM -0700, Andy Lutomirski wrote: > >> On Fri, Apr 29, 2016 at 1:11 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > >> > On Fri, Apr 29, 2016 at 11:06:53AM -0700, Andy Lutomirski wrote: > >> >> On Thu, Apr 28, 2016 at 1:44 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > >> >> > A preempted function might not have had a chance to save the frame > >> >> > pointer to the stack yet, which can result in its caller getting skipped > >> >> > on a stack trace. > >> >> > > >> >> > Add a flag to indicate when the task has been preempted so that stack > >> >> > dump code can determine whether the stack trace is reliable. > >> >> > >> >> I think I like this, but how do you handle the rather similar case in > >> >> which a task goes to sleep because it's waiting on IO that happened in > >> >> response to get_user, put_user, copy_from_user, etc? > >> > > >> > Hm, good question. I was thinking that page faults had a dedicated > >> > stack, but now looking at the entry and traps code, that doesn't seem to > >> > be the case. > >> > > >> > Anyway I think it shouldn't be a problem if we make sure that any kernel > >> > function which might trigger a valid page fault (e.g., > >> > copy_user_generic_string) do the proper frame pointer setup first. Then > >> > the stack should still be reliable. > >> > > >> > In fact I might be able to teach objtool to enforce that: any function > >> > which uses an exception table should create a stack frame. > >> > > >> > Or alternatively, maybe set some kind of flag for page faults, similar > >> > to what I did with this patch. > >> > > >> > >> How about doing it the other way around: teach the unwinder to detect > >> when it hits a non-outermost entry (i.e. it lands in idtentry, etc) > >> and use some reasonable heuristic as to whether it's okay to keep > >> unwinding. You should be able to handle preemption like that, too -- > >> the unwind process will end up in an IRQ frame. > > > > How exactly would the unwinder detect if a text address is in an > > idtentry? Maybe put all the idt entries in a special ELF section? > > > > Hmm. > > What actually happens when you unwind all the way into the entry code? > Don't you end up in something that isn't in an ELF function? Can you > detect that? For entry from user space (e.g., syscalls), it's easy to detect because there's always a pt_regs at the bottom of the stack. So if the unwinder reaches the stack address at (thread.sp0 - sizeof(pt_regs)), it knows it's done. But for nested entry (e.g. in-kernel irqs/exceptions like preemption and page faults which don't have dedicated stacks), where the pt_regs is stored somewhere in the middle of the stack instead of the bottom, there's no reliable way to detect that. > Ideally, the unwinder could actually detect that it's > hit a pt_regs struct and report that. If used for stack dumps, it > could display some indication of this and then continue its unwinding > by decoding the pt_regs. If used for patching, it could take some > other appropriate action. > > I would have no objection to annotating all the pt_regs-style entry > code, whether by putting it in a separate section or by making a table > of addresses. I think the easiest way to make it work would be to modify the idtentry macro to put all the idt entries in a dedicated section. Then the unwinder could easily detect any calls from that code. > There are a couple of nasty cases if NMI or MCE is involved but, as of > 4.6, outside of NMI, MCE, and vmalloc faults (ugh!), there should > always be a complete pt_regs on the stack before interrupts get > enabled for each entry. Of course, finding the thing may be > nontrivial in case other things were pushed. NMI, MCE and interrupts aren't a problem because they have dedicated stacks, which are easy to detect. If the tasks' stack is on an exception stack or an irq stack, we consider it unreliable. And also, they don't sleep. The stack of any running task (other than current) is automatically considered unreliable anyway, since they could be modifying it while we're reading it. > I suppose we could try to rejigger the code so that rbp points to > pt_regs or similar. I think we should avoid doing something like that because it would break gdb and all the other unwinders who don't know about it. -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-04-29 23:40 +0200 |
| Subject | Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <rtoiK-4yd-23@gated-at.bofh.it> |
| In reply to | #1391441 |
On Fri, Apr 29, 2016 at 2:25 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > On Fri, Apr 29, 2016 at 01:32:53PM -0700, Andy Lutomirski wrote: >> On Fri, Apr 29, 2016 at 1:27 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >> > On Fri, Apr 29, 2016 at 01:19:23PM -0700, Andy Lutomirski wrote: >> >> On Fri, Apr 29, 2016 at 1:11 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >> >> > On Fri, Apr 29, 2016 at 11:06:53AM -0700, Andy Lutomirski wrote: >> >> >> On Thu, Apr 28, 2016 at 1:44 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >> >> >> > A preempted function might not have had a chance to save the frame >> >> >> > pointer to the stack yet, which can result in its caller getting skipped >> >> >> > on a stack trace. >> >> >> > >> >> >> > Add a flag to indicate when the task has been preempted so that stack >> >> >> > dump code can determine whether the stack trace is reliable. >> >> >> >> >> >> I think I like this, but how do you handle the rather similar case in >> >> >> which a task goes to sleep because it's waiting on IO that happened in >> >> >> response to get_user, put_user, copy_from_user, etc? >> >> > >> >> > Hm, good question. I was thinking that page faults had a dedicated >> >> > stack, but now looking at the entry and traps code, that doesn't seem to >> >> > be the case. >> >> > >> >> > Anyway I think it shouldn't be a problem if we make sure that any kernel >> >> > function which might trigger a valid page fault (e.g., >> >> > copy_user_generic_string) do the proper frame pointer setup first. Then >> >> > the stack should still be reliable. >> >> > >> >> > In fact I might be able to teach objtool to enforce that: any function >> >> > which uses an exception table should create a stack frame. >> >> > >> >> > Or alternatively, maybe set some kind of flag for page faults, similar >> >> > to what I did with this patch. >> >> > >> >> >> >> How about doing it the other way around: teach the unwinder to detect >> >> when it hits a non-outermost entry (i.e. it lands in idtentry, etc) >> >> and use some reasonable heuristic as to whether it's okay to keep >> >> unwinding. You should be able to handle preemption like that, too -- >> >> the unwind process will end up in an IRQ frame. >> > >> > How exactly would the unwinder detect if a text address is in an >> > idtentry? Maybe put all the idt entries in a special ELF section? >> > >> >> Hmm. >> >> What actually happens when you unwind all the way into the entry code? >> Don't you end up in something that isn't in an ELF function? Can you >> detect that? > > For entry from user space (e.g., syscalls), it's easy to detect because > there's always a pt_regs at the bottom of the stack. So if the unwinder > reaches the stack address at (thread.sp0 - sizeof(pt_regs)), it knows > it's done. > > But for nested entry (e.g. in-kernel irqs/exceptions like preemption and > page faults which don't have dedicated stacks), where the pt_regs is > stored somewhere in the middle of the stack instead of the bottom, > there's no reliable way to detect that. > >> Ideally, the unwinder could actually detect that it's >> hit a pt_regs struct and report that. If used for stack dumps, it >> could display some indication of this and then continue its unwinding >> by decoding the pt_regs. If used for patching, it could take some >> other appropriate action. >> >> I would have no objection to annotating all the pt_regs-style entry >> code, whether by putting it in a separate section or by making a table >> of addresses. > > I think the easiest way to make it work would be to modify the idtentry > macro to put all the idt entries in a dedicated section. Then the > unwinder could easily detect any calls from that code. That would work. Would it make sense to do the same for the irq entries? I'd be glad to review a patch. It should be straightforward. > >> There are a couple of nasty cases if NMI or MCE is involved but, as of >> 4.6, outside of NMI, MCE, and vmalloc faults (ugh!), there should >> always be a complete pt_regs on the stack before interrupts get >> enabled for each entry. Of course, finding the thing may be >> nontrivial in case other things were pushed. > > NMI, MCE and interrupts aren't a problem because they have dedicated > stacks, which are easy to detect. If the tasks' stack is on an > exception stack or an irq stack, we consider it unreliable. Only on x86_64. > > And also, they don't sleep. The stack of any running task (other than > current) is automatically considered unreliable anyway, since they could > be modifying it while we're reading it. True. > >> I suppose we could try to rejigger the code so that rbp points to >> pt_regs or similar. > > I think we should avoid doing something like that because it would break > gdb and all the other unwinders who don't know about it. How so? Currently, rbp in the entry code is meaningless. I'm suggesting that, when we do, for example, 'call \do_sym' in idtentry, we point rbp to the pt_regs. Currently it points to something stale (which the dump_stack code might be relying on. Hmm.) But it's probably also safe to assume that if you unwind to the 'call \do_sym', then pt_regs is the next thing on the stack, so just doing the section thing would work. We should really re-add DWARF some day. --Andy -- Andy Lutomirski AMA Capital Management, LLC
[toc] | [prev] | [next] | [standalone]
| From | Jiri Kosina <jikos@kernel.org> |
|---|---|
| Date | 2016-04-30 00:20 +0200 |
| Subject | Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <rtoVs-5lW-5@gated-at.bofh.it> |
| In reply to | #1391450 |
On Fri, 29 Apr 2016, Andy Lutomirski wrote: > > NMI, MCE and interrupts aren't a problem because they have dedicated > > stacks, which are easy to detect. If the tasks' stack is on an > > exception stack or an irq stack, we consider it unreliable. > > Only on x86_64. Well, MCEs are more or less x86-specific as well. But otherwise good point, thanks Andy. So, how does stack layout generally look like in case when NMI is actually running on proper kernel stack? I thought it's guaranteed to contain pt_regs anyway in all cases. Is that not guaranteed to be the case? Thanks, -- Jiri Kosina SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-04-30 01:00 +0200 |
| Subject | Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <rtpy9-5GW-5@gated-at.bofh.it> |
| In reply to | #1391489 |
On Sat, Apr 30, 2016 at 12:11:45AM +0200, Jiri Kosina wrote: > On Fri, 29 Apr 2016, Andy Lutomirski wrote: > > > NMI, MCE and interrupts aren't a problem because they have dedicated > > > stacks, which are easy to detect. If the tasks' stack is on an > > > exception stack or an irq stack, we consider it unreliable. > > > > Only on x86_64. > > Well, MCEs are more or less x86-specific as well. But otherwise good > point, thanks Andy. > > So, how does stack layout generally look like in case when NMI is actually > running on proper kernel stack? I thought it's guaranteed to contain > pt_regs anyway in all cases. Is that not guaranteed to be the case? If the NMI were using the normal kernel stack and it interrupted kernel space, pt_regs would be placed in the "middle" of the stack rather than the bottom, and there's currently no way to detect that. However, NMIs don't sleep, and we only consider sleeping tasks for stack reliability, so it wouldn't be an issue anyway. -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-04-30 02:20 +0200 |
| Subject | Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <rtqNz-6V3-1@gated-at.bofh.it> |
| In reply to | #1391489 |
On Apr 29, 2016 3:11 PM, "Jiri Kosina" <jikos@kernel.org> wrote: > > On Fri, 29 Apr 2016, Andy Lutomirski wrote: > > > > NMI, MCE and interrupts aren't a problem because they have dedicated > > > stacks, which are easy to detect. If the tasks' stack is on an > > > exception stack or an irq stack, we consider it unreliable. > > > > Only on x86_64. > > Well, MCEs are more or less x86-specific as well. But otherwise good > point, thanks Andy. > > So, how does stack layout generally look like in case when NMI is actually > running on proper kernel stack? I thought it's guaranteed to contain > pt_regs anyway in all cases. Is that not guaranteed to be the case? > On x86, at least, there will still be pt_regs for the NMI. For the interrupted state, though, there might not be pt_regs, as the NMI might have happened while still populating pt_regs. In fact, the NMI stack could overlap task_pt_regs. For x86_32, there's no guarantee that pt_regs contains sp due to hardware silliness. You need to parse it more carefully, as, !user_mode(regs), then the old sp is just above pt_regs. --Andy
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-04-30 00:50 +0200 |
| Subject | Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <rtpot-5CD-1@gated-at.bofh.it> |
| In reply to | #1391450 |
On Fri, Apr 29, 2016 at 02:37:41PM -0700, Andy Lutomirski wrote: > On Fri, Apr 29, 2016 at 2:25 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > I think the easiest way to make it work would be to modify the idtentry > > macro to put all the idt entries in a dedicated section. Then the > > unwinder could easily detect any calls from that code. > > That would work. Would it make sense to do the same for the irq entries? Yes, I think so. > >> I suppose we could try to rejigger the code so that rbp points to > >> pt_regs or similar. > > > > I think we should avoid doing something like that because it would break > > gdb and all the other unwinders who don't know about it. > > How so? > > Currently, rbp in the entry code is meaningless. I'm suggesting that, > when we do, for example, 'call \do_sym' in idtentry, we point rbp to > the pt_regs. Currently it points to something stale (which the > dump_stack code might be relying on. Hmm.) But it's probably also > safe to assume that if you unwind to the 'call \do_sym', then pt_regs > is the next thing on the stack, so just doing the section thing would > work. Yes, rbp is meaningless on the entry from user space. But if an in-kernel interrupt occurs (e.g. page fault, preemption) and you have nested entry, rbp keeps its old value, right? So the unwinder can walk past the nested entry frame and keep going until it gets to the original entry. > We should really re-add DWARF some day. Working on it :-) -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-04-30 02:10 +0200 |
| Subject | Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <rtqDU-6Oh-5@gated-at.bofh.it> |
| In reply to | #1391507 |
On Apr 29, 2016 3:41 PM, "Josh Poimboeuf" <jpoimboe@redhat.com> wrote: > > On Fri, Apr 29, 2016 at 02:37:41PM -0700, Andy Lutomirski wrote: > > On Fri, Apr 29, 2016 at 2:25 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > I think the easiest way to make it work would be to modify the idtentry > > > macro to put all the idt entries in a dedicated section. Then the > > > unwinder could easily detect any calls from that code. > > > > That would work. Would it make sense to do the same for the irq entries? > > Yes, I think so. > > > >> I suppose we could try to rejigger the code so that rbp points to > > >> pt_regs or similar. > > > > > > I think we should avoid doing something like that because it would break > > > gdb and all the other unwinders who don't know about it. > > > > How so? > > > > Currently, rbp in the entry code is meaningless. I'm suggesting that, > > when we do, for example, 'call \do_sym' in idtentry, we point rbp to > > the pt_regs. Currently it points to something stale (which the > > dump_stack code might be relying on. Hmm.) But it's probably also > > safe to assume that if you unwind to the 'call \do_sym', then pt_regs > > is the next thing on the stack, so just doing the section thing would > > work. > > Yes, rbp is meaningless on the entry from user space. But if an > in-kernel interrupt occurs (e.g. page fault, preemption) and you have > nested entry, rbp keeps its old value, right? So the unwinder can walk > past the nested entry frame and keep going until it gets to the original > entry. Yes. It would be nice if we could do better, though, and actually notice the pt_regs and identify the entry. For example, I'd love to see "page fault, RIP=xyz" printed in the middle of a stack dump on a crash. Also, I think that just following rbp links will lose the actual function that took the page fault (or whatever function pt_regs->ip actually points to). > > > We should really re-add DWARF some day. > > Working on it :-) Excellent. Have you looked at my vdso unwinding test at all? If we could do something similar for the kernel, IMO it would make testing much more pleasant. --Andy
[toc] | [prev] | [next] | [standalone]
| From | Josh Poimboeuf <jpoimboe@redhat.com> |
|---|---|
| Date | 2016-05-02 16:00 +0200 |
| Subject | Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <rumye-4g2-15@gated-at.bofh.it> |
| In reply to | #1391535 |
On Fri, Apr 29, 2016 at 05:08:50PM -0700, Andy Lutomirski wrote: > On Apr 29, 2016 3:41 PM, "Josh Poimboeuf" <jpoimboe@redhat.com> wrote: > > > > On Fri, Apr 29, 2016 at 02:37:41PM -0700, Andy Lutomirski wrote: > > > On Fri, Apr 29, 2016 at 2:25 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > >> I suppose we could try to rejigger the code so that rbp points to > > > >> pt_regs or similar. > > > > > > > > I think we should avoid doing something like that because it would break > > > > gdb and all the other unwinders who don't know about it. > > > > > > How so? > > > > > > Currently, rbp in the entry code is meaningless. I'm suggesting that, > > > when we do, for example, 'call \do_sym' in idtentry, we point rbp to > > > the pt_regs. Currently it points to something stale (which the > > > dump_stack code might be relying on. Hmm.) But it's probably also > > > safe to assume that if you unwind to the 'call \do_sym', then pt_regs > > > is the next thing on the stack, so just doing the section thing would > > > work. > > > > Yes, rbp is meaningless on the entry from user space. But if an > > in-kernel interrupt occurs (e.g. page fault, preemption) and you have > > nested entry, rbp keeps its old value, right? So the unwinder can walk > > past the nested entry frame and keep going until it gets to the original > > entry. > > Yes. > > It would be nice if we could do better, though, and actually notice > the pt_regs and identify the entry. For example, I'd love to see > "page fault, RIP=xyz" printed in the middle of a stack dump on a > crash. > > Also, I think that just following rbp links will lose the > actual function that took the page fault (or whatever function > pt_regs->ip actually points to). Hm. I think we could fix all that in a more standard way. Whenever a new pt_regs frame gets saved on entry, we could also create a new stack frame which points to a fake kernel_entry() function. That would tell the unwinder there's a pt_regs frame without otherwise breaking frame pointers across the frame. Then I guess we wouldn't need my other solution of putting the idt entries in a special section. How does that sound? > Have you looked at my vdso unwinding test at all? If we could do > something similar for the kernel, IMO it would make testing much more > pleasant. I found it, but I'm not sure what it would mean to do something similar for the kernel. Do you mean doing something like an NMI sampling-based approach where we periodically do a random stack sanity check? (If so, I do have something like that planned.) -- Josh
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-05-02 18:00 +0200 |
| Subject | Re: [RFC PATCH v2 05/18] sched: add task flag for preempt IRQ tracking |
| Message-ID | <ruoqm-6cc-3@gated-at.bofh.it> |
| In reply to | #1392223 |
On Mon, May 2, 2016 at 6:52 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > On Fri, Apr 29, 2016 at 05:08:50PM -0700, Andy Lutomirski wrote: >> On Apr 29, 2016 3:41 PM, "Josh Poimboeuf" <jpoimboe@redhat.com> wrote: >> > >> > On Fri, Apr 29, 2016 at 02:37:41PM -0700, Andy Lutomirski wrote: >> > > On Fri, Apr 29, 2016 at 2:25 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >> > > >> I suppose we could try to rejigger the code so that rbp points to >> > > >> pt_regs or similar. >> > > > >> > > > I think we should avoid doing something like that because it would break >> > > > gdb and all the other unwinders who don't know about it. >> > > >> > > How so? >> > > >> > > Currently, rbp in the entry code is meaningless. I'm suggesting that, >> > > when we do, for example, 'call \do_sym' in idtentry, we point rbp to >> > > the pt_regs. Currently it points to something stale (which the >> > > dump_stack code might be relying on. Hmm.) But it's probably also >> > > safe to assume that if you unwind to the 'call \do_sym', then pt_regs >> > > is the next thing on the stack, so just doing the section thing would >> > > work. >> > >> > Yes, rbp is meaningless on the entry from user space. But if an >> > in-kernel interrupt occurs (e.g. page fault, preemption) and you have >> > nested entry, rbp keeps its old value, right? So the unwinder can walk >> > past the nested entry frame and keep going until it gets to the original >> > entry. >> >> Yes. >> >> It would be nice if we could do better, though, and actually notice >> the pt_regs and identify the entry. For example, I'd love to see >> "page fault, RIP=xyz" printed in the middle of a stack dump on a >> crash. >> >> Also, I think that just following rbp links will lose the >> actual function that took the page fault (or whatever function >> pt_regs->ip actually points to). > > Hm. I think we could fix all that in a more standard way. Whenever a > new pt_regs frame gets saved on entry, we could also create a new stack > frame which points to a fake kernel_entry() function. That would tell > the unwinder there's a pt_regs frame without otherwise breaking frame > pointers across the frame. > > Then I guess we wouldn't need my other solution of putting the idt > entries in a special section. > > How does that sound? Let me try to understand. The normal call sequence is call; push %rbp; mov %rsp, %rbp. So rbp points to (prev rbp, prev rip) on the stack, and you can follow the chain back. Right now, on a user access page fault or similar, we have rbp (probably) pointing to the interrupted frame, and the interrupted rip isn't saved anywhere that a naive unwinder can find it. (It's in pt_regs, but the rbp chain skips right over that.) We could change the entry code so that an interrupt / idtentry does: push pt_regs push kernel_entry push %rbp mov %rsp, %rbp call handler pop %rbp addq $8, %rsp or similar. That would make it appear that the actual C handler was caused by a dummy function "kernel_entry". Now the unwinder would get to kernel_entry, but it *still* wouldn't find its way to the calling frame, which only solves part of the problem. We could at least teach the unwinder how kernel_entry works and let it decode pt_regs to continue unwinding. This would be nice, and I think it could work. I think I like this, except that, if it used a separate section, it could potentially be faster, as, for each actual entry type, the offset from the C handler frame to pt_regs is a foregone conclusion. But this is pretty simple and performance is already abysmal in most handlers. There's an added benefit to using a separate section, though: we could also annotate the calls with what type of entry they were so the unwinder could print it out nicely. I could be convinced either way. > >> Have you looked at my vdso unwinding test at all? If we could do >> something similar for the kernel, IMO it would make testing much more >> pleasant. > > I found it, but I'm not sure what it would mean to do something similar > for the kernel. Do you mean doing something like an NMI sampling-based > approach where we periodically do a random stack sanity check? I was imagining something a little more strict: single-step interesting parts of the kernel and make sure that each step unwinds correctly. That could detect missing frames and similar.
[toc] | [prev] | [next] | [standalone]
Page 1 of 4 [1] 2 3 4 Next page →
Back to top | Article view | linux.kernel
csiph-web