Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1395058
| From | Petr Mladek <pmladek@suse.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: klp_task_patch: was: [RFC PATCH v2 17/18] livepatch: change to a per-task consistency model |
| Date | 2016-05-05 14:00 +0200 |
| Message-ID | <rvq6J-7IA-5@gated-at.bofh.it> (permalink) |
| References | <rt12N-1W5-3@gated-at.bofh.it> <rt12O-1W5-9@gated-at.bofh.it> <rv6hJ-5S0-59@gated-at.bofh.it> <rv9fz-85-3@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Wed 2016-05-04 12:57:00, Josh Poimboeuf wrote:
> On Wed, May 04, 2016 at 04:48:54PM +0200, Petr Mladek wrote:
> > On Thu 2016-04-28 15:44:48, Josh Poimboeuf wrote:
> > > Change livepatch to use a basic per-task consistency model. This is the
> > > foundation which will eventually enable us to patch those ~10% of
> > > security patches which change function or data semantics. This is the
> > > biggest remaining piece needed to make livepatch more generally useful.
> > >
> > > diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
> > > new file mode 100644
> > > index 0000000..92819bb
> > > --- /dev/null
> > > +++ b/kernel/livepatch/transition.c
> > > +/*
> > > + * klp_patch_task() - change the patched state of a task
> > > + * @task: The task to change
> > > + *
> > > + * Switches the patched state of the task to the set of functions in the target
> > > + * patch state.
> > > + */
> > > +void klp_patch_task(struct task_struct *task)
> > > +{
> > > + clear_tsk_thread_flag(task, TIF_PATCH_PENDING);
> > > +
> > > + /*
> > > + * The corresponding write barriers are in klp_init_transition() and
> > > + * klp_reverse_transition(). See the comments there for an explanation.
> > > + */
> > > + smp_rmb();
> > > +
> > > + task->patch_state = klp_target_state;
> > > +}
> > > +
> > > diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> > > index bd12c6c..60d633f 100644
> > > --- a/kernel/sched/idle.c
> > > +++ b/kernel/sched/idle.c
> > > @@ -9,6 +9,7 @@
> > > #include <linux/mm.h>
> > > #include <linux/stackprotector.h>
> > > #include <linux/suspend.h>
> > > +#include <linux/livepatch.h>
> > >
> > > #include <asm/tlb.h>
> > >
> > > @@ -266,6 +267,9 @@ static void cpu_idle_loop(void)
> > >
> > > sched_ttwu_pending();
> > > schedule_preempt_disabled();
> > > +
> > > + if (unlikely(klp_patch_pending(current)))
> > > + klp_patch_task(current);
> > > }
> >
> > Some more ideas from the world of crazy races. I was shaking my head
> > if this was safe or not.
> >
> > The problem might be if the task get rescheduled between the check
> > for the pending stuff or inside the klp_patch_task() function.
> > This will get even more important when we use this construct
> > on some more locations, e.g. in some kthreads.
> >
> > If the task is sleeping on this strange locations, it might assign
> > strange values on strange times.
> >
> > I think that it is safe only because it is called with the
> > 'current' parameter and on a safe locations. It means that
> > the result is always safe and consistent. Also we could assign
> > an outdated value only when sleeping between reading klp_target_state
> > and storing task->patch_state. But if anyone modified
> > klp_target_state at this point, he also set TIF_PENDING_PATCH,
> > so the change will not get lost.
> >
> > I think that we should document that klp_patch_func() must be
> > called only from a safe location from within the affected task.
> >
> > I even suggest to avoid misuse by removing the struct *task_struct
> > parameter. It should always be called with current.
>
> Would the race involve two tasks trying to call klp_patch_task() for the
> same task at the same time? If so I don't think that would be a problem
> since they would both write the same value for task->patch_state.
I have missed that the two commands are called with preemption
disabled. So, I had the following crazy scenario in mind:
CPU0 CPU1
klp_enable_patch()
klp_target_state = KLP_PATCHED;
for_each_task()
set TIF_PENDING_PATCH
# task 123
if (klp_patch_pending(current)
klp_patch_task(current)
clear TIF_PENDING_PATCH
smp_rmb();
# switch to assembly of
# klp_patch_task()
mov klp_target_state, %r12
# interrupt and schedule
# another task
klp_reverse_transition();
klp_target_state = KLP_UNPATCHED;
klt_try_to_complete_transition()
task = 123;
if (task->patch_state == klp_target_state;
return 0;
=> task 123 is in target state and does
not block conversion
klp_complete_transition()
# disable previous patch on the stack
klp_disable_patch();
klp_target_state = KLP_UNPATCHED;
# task 123 gets scheduled again
lea %r12, task->patch_state
=> it happily stores an outdated
state
This is why the two functions should get called with preemption
disabled. We should document it at least. I imagine that we will
use them later also in another context and nobody will remember
this crazy scenario.
Well, even disabled preemption does not help. The process on
CPU1 might be also interrupted by an NMI and do some long
printk in it.
IMHO, the only safe approach is to call klp_patch_task()
only for "current" on a safe place. Then this race is harmless.
The switch happen on a safe place, so that it does not matter
into which state the process is switched.
By other words, the task state might be updated only
+ by the task itself on a safe place
+ by other task when the updated on is sleeping on a safe place
This should be well documented and the API should help to avoid
a misuse.
Best Regards,
Petr
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[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
csiph-web