Documentation/livepatch: remove the limitation for schedule() patching
Subsystems (2)
Miroslav Benes
maintainerPATCH 1/1 Documentation/livepatch: remove the limitation for schedule() patching
The Limitations section of the documentation describes the impossibility to livepatch anything that is inlined to __schedule() function. This had been true till 4.9 kernel came. Thanks to commit 0100301bfdf5 ("sched/x86: Rewrite the switch_to() code") from Brian Gerst there is __switch_to_asm function now (implemented in assembly) called properly from context_switch(). RIP is thus saved on the stack and a task would return to proper version of __schedule() et al. functions.
Of course __switch_to_asm() is not patchable for the reason described in the section. But there is no __fentry__ call and I cannot imagine a reason to do it anyway.
Therefore, remove the paragraphs from the section.
---
FWIW, I also tested this to be sure on top of the consistency model patch set. I patched schedule() function which calls __schedule() (it is impossible to patch it directly due to notrace attribute). It works well except...
1. the patching process does not finish, because many tasks sleep in schedule. STOP/CONT signal does not help. I'll investigate.
2. reversion of the process does not work as expected. The kernel crashes after the removal of the module. A task very likely slept in schedule and was not migrated properly. It might be because of the races in klp_reverse_transition() described by Petr, or might be somewhere else. I'll look into it.
Documentation/livepatch/livepatch.txt | 19 ------------------- 1 file changed, 19 deletions(-)
Documentation/livepatch/livepatch.txt +0 -19
@@ -329,25 +329,6 @@ See Documentation/ABI/testing/sysfs-kernel-livepatch for more details. by "notrace". - + Anything inlined into __schedule() can not be patched. - - The switch_to macro is inlined into __schedule(). It switches the - context between two processes in the middle of the macro. It does - not save RIP in x86_64 version (contrary to 32-bit version). Instead, - the currently used __schedule()/switch_to() handles both processes. - - Now, let's have two different tasks. One calls the original - __schedule(), its registers are stored in a defined order and it - goes to sleep in the switch_to macro and some other task is restored - using the original __schedule(). Then there is the second task which - calls patched__schedule(), it goes to sleep there and the first task - is picked by the patched__schedule(). Its RSP is restored and now - the registers should be restored as well. But the order is different - in the new patched__schedule(), so... - - There is work in progress to remove this limitation. - - + Livepatch modules can not be removed. The current implementation just redirects the functions at the very
Signature · 1 line
2.11.0
Quoted 8 lines · Miroslav Benes
The Limitations section of the documentation describes the impossibility to livepatch anything that is inlined to
__schedule()function. This had been true till 4.9 kernel came. Thanks to commit0100301bfdf5("sched/x86: Rewrite theswitch_to()code") from Brian Gerst there is__switch_to_asmfunction now (implemented in assembly) called properly fromcontext_switch(). RIP is thus saved on the stack and a task would return to proper version of__schedule()et al. functions.
Quoted 7 lines
Of course
__switch_to_asm()is not patchable for the reason described in the section. But there is no__fentry__call and I cannot imagine a reason to do it anyway.Therefore, remove the paragraphs from the section.
Signed-off-by Miroslav Benes <[email protected]>
It is great to get a feature for free ;-)
Best Regards,
Petr
---
Quoted 7 lines
FWIW, I also tested this to be sure on top of the consistency model patch set. I patched
schedule()function which calls__schedule()(it is impossible to patch it directly due to notrace attribute). It works well except...1. the patching process does not finish, because many tasks sleep in schedule. STOP/CONT signal does not help. I'll investigate.
Are these userspace processes or kthreads? Kthreads would cause problems because they do not handle signals.
Quoted 5 lines
2. reversion of the process does not work as expected. The kernel crashes after the removal of the module. A task very likely slept in schedule and was not migrated properly. It might be because of the races in
klp_reverse_transition()described by Petr, or might be somewhere else. I'll look into it.
I hope that I will be able to do another dive into the consistency model patchset the following week.
Best Regards,
Petr
Miroslav Benes
maintainerQuoted 30 lines · Petr Mladek
Quoted 8 lines · Miroslav Benes
The Limitations section of the documentation describes the impossibility to livepatch anything that is inlined to
__schedule()function. This had been true till 4.9 kernel came. Thanks to commit0100301bfdf5("sched/x86: Rewrite theswitch_to()code") from Brian Gerst there is__switch_to_asmfunction now (implemented in assembly) called properly fromcontext_switch(). RIP is thus saved on the stack and a task would return to proper version of__schedule()et al. functions.Quoted 7 lines
Of course
__switch_to_asm()is not patchable for the reason described in the section. But there is no__fentry__call and I cannot imagine a reason to do it anyway.Therefore, remove the paragraphs from the section.
Signed-off-by Miroslav Benes <[email protected]>It is great to get a feature for free ;-)
Reviewed-by Petr Mladek <[email protected]>Best Regards,
Petr---
Quoted 5 lines
FWIW, I also tested this to be sure on top of the consistency model patch set. I patched
schedule()function which calls__schedule()(it is impossible to patch it directly due to notrace attribute). It works well except...
Quoted 5 lines
Quoted 2 lines
1. the patching process does not finish, because many tasks sleep in schedule. STOP/CONT signal does not help. I'll investigate.
Are these userspace processes or kthreads? Kthreads would cause problems because they do not handle signals.
Userspace processes, but I take it back. Stupid typo in my script. It works as expected. Kthreads sleeping in schedule() are of course there and a signal does not help.
Miroslav
Quoted 8 lines · Miroslav Benes
The Limitations section of the documentation describes the impossibility to livepatch anything that is inlined to
__schedule()function. This had been true till 4.9 kernel came. Thanks to commit0100301bfdf5("sched/x86: Rewrite theswitch_to()code") from Brian Gerst there is__switch_to_asmfunction now (implemented in assembly) called properly fromcontext_switch(). RIP is thus saved on the stack and a task would return to proper version of__schedule()et al. functions.
Quoted 7 lines
Of course
__switch_to_asm()is not patchable for the reason described in the section. But there is no__fentry__call and I cannot imagine a reason to do it anyway.Therefore, remove the paragraphs from the section.
Signed-off-by Miroslav Benes <[email protected]>
Quoted 6 lines
---
FWIW, I also tested this to be sure on top of the consistency model patch set. I patchedschedule()function which calls__schedule()(it is impossible to patch it directly due to notrace attribute). It works well except...
Quoted 8 lines
1. the patching process does not finish, because many tasks sleep in schedule. STOP/CONT signal does not help. I'll investigate.
2. reversion of the process does not work as expected. The kernel crashes after the removal of the module. A task very likely slept in schedule and was not migrated properly. It might be because of the races in
klp_reverse_transition()described by Petr, or might be somewhere else. I'll look into it.
Hm, will be interesting to see the cause of this...
Signature · 1 line
Josh
Miroslav Benes
maintainerQuoted 9 lines · Josh Poimboeuf
Quoted 6 lines · Miroslav Benes
2. reversion of the process does not work as expected. The kernel crashes after the removal of the module. A task very likely slept in schedule and was not migrated properly. It might be because of the races in
klp_reverse_transition()described by Petr, or might be somewhere else. I'll look into it.Hm, will be interesting to see the cause of this...
The absence of the patched schedule() on the stack was the cause. klp_try_switch_task() thus did not see it and happily migrated the task.
The reason is funny. One cannot patch __schedule() (which is of interested) because of the notrace attribute. So all the callers need to be processed. I tried to make my life easier and patched only schedule(). GCC then inlined new __schedule() to the new schedule(). When I added noinline attribute to the new __schedule() everything was fine (because suddenly new schedule() was on the stack as expected).
There is still one thing which I don't understand. Why __schedule() (patched or the original) is not on the stack. The actual "sleep" should happen in __switch_to_asm() which is C function now. And there is a call to __switch_to_asm() in __schedule(). __schedule() thus should be on the stack, shouldn't it? What am I missing? __switch_to_asm() pushes %rbp on the stack...
Miroslav
Quoted 6 lines · Miroslav Benes
There is still one thing which I don't understand. Why
__schedule()(patched or the original) is not on the stack. The actual "sleep" should happen in__switch_to_asm()which is C function now. And there is a call to__switch_to_asm()in__schedule().__schedule()thus should be on the stack, shouldn't it? What am I missing?__switch_to_asm()pushes %rbp on the stack...
Ah, this is an unwinder bug. get_frame_pointer() needs to be fixed so that for an inactive task it returns a pointer to inactive_task_frame.bp rather than the value of inactive_task_frame.bp itself. Will fix it.
Signature · 1 line
Josh
Miroslav Benes
maintainerQuoted 11 lines · Josh Poimboeuf
Quoted 6 lines · Miroslav Benes
There is still one thing which I don't understand. Why
__schedule()(patched or the original) is not on the stack. The actual "sleep" should happen in__switch_to_asm()which is C function now. And there is a call to__switch_to_asm()in__schedule().__schedule()thus should be on the stack, shouldn't it? What am I missing?__switch_to_asm()pushes %rbp on the stack...Ah, this is an unwinder bug.
get_frame_pointer()needs to be fixed so that for an inactive task it returns a pointer to inactive_task_frame.bp rather than the value of inactive_task_frame.bp itself. Will fix it.
And it works with the fix. Thanks.
Miroslav
Jiri Kosina
AppliedQuoted 21 lines · Miroslav Benes
The Limitations section of the documentation describes the impossibility to livepatch anything that is inlined to
__schedule()function. This had been true till 4.9 kernel came. Thanks to commit0100301bfdf5("sched/x86: Rewrite theswitch_to()code") from Brian Gerst there is__switch_to_asmfunction now (implemented in assembly) called properly fromcontext_switch(). RIP is thus saved on the stack and a task would return to proper version of__schedule()et al. functions.Of course
__switch_to_asm()is not patchable for the reason described in the section. But there is no__fentry__call and I cannot imagine a reason to do it anyway.Therefore, remove the paragraphs from the section.
Signed-off-by Miroslav Benes <[email protected]>---
FWIW, I also tested this to be sure on top of the consistency model patch set. I patchedschedule()function which calls__schedule()(it is impossible to patch it directly due to notrace attribute). It works well except...
Quoted 8 lines
1. the patching process does not finish, because many tasks sleep in schedule. STOP/CONT signal does not help. I'll investigate.
2. reversion of the process does not work as expected. The kernel crashes after the removal of the module. A task very likely slept in schedule and was not migrated properly. It might be because of the races in
klp_reverse_transition()described by Petr, or might be somewhere else. I'll look into it.
Applied, thanks.
Signature · 2 lines
Jiri Kosina SUSE Labs