Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1370813 > unrolled thread

Re: [RFC PATCH v1.9 12/14] livepatch: create per-task consistency model

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH v1.9 12/14] livepatch: create per-task consistency  model Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-04 20:30 +0200
    Re: [RFC PATCH v1.9 12/14] livepatch: create per-task consistency  model Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-04 20:40 +0200
      Re: [RFC PATCH v1.9 12/14] livepatch: create per-task consistency  model Vojtech Pavlik <vojtech@suse.com> - 2016-04-05 13:40 +0200
        Re: [RFC PATCH v1.9 12/14] livepatch: create per-task consistency  model Josh Poimboeuf <jpoimboe@redhat.com> - 2016-04-05 16:00 +0200
    Re: [RFC PATCH v1.9 12/14] livepatch: create per-task consistency  model Vojtech Pavlik <vojtech@suse.com> - 2016-04-04 21:10 +0200

#1370813 — Re: [RFC PATCH v1.9 12/14] livepatch: create per-task consistency model

On Thu, Mar 31, 2016 at 03:12:39PM +0200, Miroslav Benes wrote:
> On Fri, 25 Mar 2016, Josh Poimboeuf wrote:
> 
> [...]
> 
> > diff --git a/kernel/fork.c b/kernel/fork.c
> > index d277e83..27b181e 100644
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -76,6 +76,7 @@
> >  #include <linux/compiler.h>
> >  #include <linux/sysctl.h>
> >  #include <linux/kcov.h>
> > +#include <linux/livepatch.h>
> >  
> >  #include <asm/pgtable.h>
> >  #include <asm/pgalloc.h>
> > @@ -1615,6 +1616,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
> >  	total_forks++;
> >  	spin_unlock(&current->sighand->siglock);
> >  	syscall_tracepoint_update(p);
> > +	klp_update_task_universe(p);
> 
> Shouldn't we copy transition and TIF from the parent? I deal with a race 
> in kgraft and the solution seems to be this code exactly at this place in 
> copy_process(). I need to think about it.

Hm, can you explain why it should be copied from the parent?

I'm thinking the above code is correct for today, but it should still be
changed to be more future-proof.

Here's my thinking:

A forked task starts out with no stack, so if I understand correctly, it
can safely start out in the goal universe, regardless of which universe
its parent belongs to.

However, the current ret_from_fork code is a mess, and Andy Lutomirski
has mentioned that he would like to give newly forked tasks a proper
stack such that instead of jumping to ret_from_fork, they would just
return from schedule().  In that case, it would no longer be safe to
start the new task in the goal universe because it could be "sleeping"
on a to-be-patched function.

So for proper future proofing, newly forked tasks should be started in
the initial universe (rather than starting in the goal universe or
inheriting the parent's universe).  They can then be transitioned over
to the goal universe like any other task.  How does that sound?

-- 
Josh

[toc] | [next] | [standalone]

#1370816

On Mon, Apr 04, 2016 at 08:27:59PM +0200, Vojtech Pavlik wrote:
> On Mon, Apr 04, 2016 at 01:21:38PM -0500, Josh Poimboeuf wrote:
> 
> > Hm, can you explain why it should be copied from the parent?
> > 
> > I'm thinking the above code is correct for today, but it should still be
> > changed to be more future-proof.
> > 
> > Here's my thinking:
> > 
> > A forked task starts out with no stack, so if I understand correctly, it
> > can safely start out in the goal universe, regardless of which universe
> > its parent belongs to.
> > 
> > However, the current ret_from_fork code is a mess, and Andy Lutomirski
> > has mentioned that he would like to give newly forked tasks a proper
> > stack such that instead of jumping to ret_from_fork, they would just
> > return from schedule().  In that case, it would no longer be safe to
> > start the new task in the goal universe because it could be "sleeping"
> > on a to-be-patched function.
> > 
> > So for proper future proofing, newly forked tasks should be started in
> > the initial universe (rather than starting in the goal universe or
> > inheriting the parent's universe).  They can then be transitioned over
> > to the goal universe like any other task.  How does that sound?
> 
> How could a newly forked task start in the old universe if its parent
> has already been migrated? Any context it inherits will already be from
> the new universe.

Can you be more specific about "context" and why inheritance of it would
be a problem?

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1371426

On Mon, Apr 04, 2016 at 01:33:07PM -0500, Josh Poimboeuf wrote:
> On Mon, Apr 04, 2016 at 08:27:59PM +0200, Vojtech Pavlik wrote:
> > On Mon, Apr 04, 2016 at 01:21:38PM -0500, Josh Poimboeuf wrote:
> > 
> > > Hm, can you explain why it should be copied from the parent?
> > > 
> > > I'm thinking the above code is correct for today, but it should still be
> > > changed to be more future-proof.
> > > 
> > > Here's my thinking:
> > > 
> > > A forked task starts out with no stack, so if I understand correctly, it
> > > can safely start out in the goal universe, regardless of which universe
> > > its parent belongs to.
> > > 
> > > However, the current ret_from_fork code is a mess, and Andy Lutomirski
> > > has mentioned that he would like to give newly forked tasks a proper
> > > stack such that instead of jumping to ret_from_fork, they would just
> > > return from schedule().  In that case, it would no longer be safe to
> > > start the new task in the goal universe because it could be "sleeping"
> > > on a to-be-patched function.
> > > 
> > > So for proper future proofing, newly forked tasks should be started in
> > > the initial universe (rather than starting in the goal universe or
> > > inheriting the parent's universe).  They can then be transitioned over
> > > to the goal universe like any other task.  How does that sound?
> > 
> > How could a newly forked task start in the old universe if its parent
> > has already been migrated? Any context it inherits will already be from
> > the new universe.
> 
> Can you be more specific about "context" and why inheritance of it would
> be a problem?

Currently a forked task starts out with no stack, and as such it can
start in the goal universe.

If we create a synthetic stack, then we may need to start in the initial
universe, as the synthetic stack would likely be created using initial
universe return addresses. 

If we simply copy the stack of the parent process, which is in my
opionion the safest way, as it places little assumptions on the
compiler, then it may contain either old or new addresses
and we need to copy the universe flag along.

-- 
Vojtech Pavlik
Director SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1371615

On Tue, Apr 05, 2016 at 01:36:34PM +0200, Vojtech Pavlik wrote:
> On Mon, Apr 04, 2016 at 01:33:07PM -0500, Josh Poimboeuf wrote:
> > On Mon, Apr 04, 2016 at 08:27:59PM +0200, Vojtech Pavlik wrote:
> > > On Mon, Apr 04, 2016 at 01:21:38PM -0500, Josh Poimboeuf wrote:
> > > 
> > > > Hm, can you explain why it should be copied from the parent?
> > > > 
> > > > I'm thinking the above code is correct for today, but it should still be
> > > > changed to be more future-proof.
> > > > 
> > > > Here's my thinking:
> > > > 
> > > > A forked task starts out with no stack, so if I understand correctly, it
> > > > can safely start out in the goal universe, regardless of which universe
> > > > its parent belongs to.
> > > > 
> > > > However, the current ret_from_fork code is a mess, and Andy Lutomirski
> > > > has mentioned that he would like to give newly forked tasks a proper
> > > > stack such that instead of jumping to ret_from_fork, they would just
> > > > return from schedule().  In that case, it would no longer be safe to
> > > > start the new task in the goal universe because it could be "sleeping"
> > > > on a to-be-patched function.
> > > > 
> > > > So for proper future proofing, newly forked tasks should be started in
> > > > the initial universe (rather than starting in the goal universe or
> > > > inheriting the parent's universe).  They can then be transitioned over
> > > > to the goal universe like any other task.  How does that sound?
> > > 
> > > How could a newly forked task start in the old universe if its parent
> > > has already been migrated? Any context it inherits will already be from
> > > the new universe.
> > 
> > Can you be more specific about "context" and why inheritance of it would
> > be a problem?
> 
> Currently a forked task starts out with no stack, and as such it can
> start in the goal universe.
> 
> If we create a synthetic stack, then we may need to start in the initial
> universe, as the synthetic stack would likely be created using initial
> universe return addresses. 
> 
> If we simply copy the stack of the parent process, which is in my
> opionion the safest way, as it places little assumptions on the
> compiler, then it may contain either old or new addresses
> and we need to copy the universe flag along.

That all makes sense.  I'll change it to inherit the parent's universe
for now.

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1370828

On Mon, Apr 04, 2016 at 01:21:38PM -0500, Josh Poimboeuf wrote:

> Hm, can you explain why it should be copied from the parent?
> 
> I'm thinking the above code is correct for today, but it should still be
> changed to be more future-proof.
> 
> Here's my thinking:
> 
> A forked task starts out with no stack, so if I understand correctly, it
> can safely start out in the goal universe, regardless of which universe
> its parent belongs to.
> 
> However, the current ret_from_fork code is a mess, and Andy Lutomirski
> has mentioned that he would like to give newly forked tasks a proper
> stack such that instead of jumping to ret_from_fork, they would just
> return from schedule().  In that case, it would no longer be safe to
> start the new task in the goal universe because it could be "sleeping"
> on a to-be-patched function.
> 
> So for proper future proofing, newly forked tasks should be started in
> the initial universe (rather than starting in the goal universe or
> inheriting the parent's universe).  They can then be transitioned over
> to the goal universe like any other task.  How does that sound?

How could a newly forked task start in the old universe if its parent
has already been migrated? Any context it inherits will already be from
the new universe.

-- 
Vojtech Pavlik
Director SuSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web