LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: [PATCH] livepatch: Enhance livepatch to support remove patch module dynamically
       [not found] <1427893812-15936-1-git-send-email-minfei.huang@yahoo.com>
@ 2015-04-01 13:35 ` Jiri Kosina
  0 siblings, 0 replies; 5+ messages in thread
From: Jiri Kosina @ 2015-04-01 13:35 UTC (permalink / raw)
  To: Minfei Huang
  Cc: jpoimboe, sjenning, Vojtech Pavlik, live-patching, linux-kernel

On Wed, 1 Apr 2015, Minfei Huang wrote:

> As mentioned in the annotation, the patch module would not permit to
> be removed once it is loaded.
> 
> Kernel may crash in the case that the function in the patch module is
> called during removing.
> 
> We will increase the module reference when the patch module is enable,
> so that the module cannt be removed. Once the module is disable, it will
> be removed.
> 
> Signed-off-by: Minfei Huang <minfei.huang@yahoo.com>
> ---
>  kernel/livepatch/core.c | 47 +++++++++++++++++++++++++++++++----------------
>  1 file changed, 31 insertions(+), 16 deletions(-)
> 
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 3f9f1d6..0266950 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -502,6 +502,17 @@ static int __klp_disable_patch(struct klp_patch *patch)
>  	return 0;
>  }
>  
> +static int __klp_disable_patch_nolock(struct klp_patch *patch)
> +{
> +	int ret = 0;
> +
> +	ret = __klp_disable_patch(patch);
> +	if (ret)
> +		return ret;
> +	module_put(patch->mod);
> +	return ret;
> +}
> +

Your patch doesn't solve the problem at all.

There is no guarantee that once __klp_disable_patch() returns noone is 
using the old code any more.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] livepatch: Enhance livepatch to support remove patch module dynamically
  2015-04-01 14:40     ` Minfei Huang
@ 2015-04-01 17:22       ` Josh Poimboeuf
  0 siblings, 0 replies; 5+ messages in thread
From: Josh Poimboeuf @ 2015-04-01 17:22 UTC (permalink / raw)
  To: Minfei Huang; +Cc: Jiri Kosina, sjenning, vojtech, live-patching, linux-kernel

On Wed, Apr 01, 2015 at 10:40:19PM +0800, Minfei Huang wrote:
> 2015-04-01 22:13 GMT+08:00 Jiri Kosina <jkosina@suse.cz>:
> > On Wed, 1 Apr 2015, Minfei Huang wrote:
> >
> >> > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> >> > > index 3f9f1d6..0266950 100644
> >> > > --- a/kernel/livepatch/core.c
> >> > > +++ b/kernel/livepatch/core.c
> >> > > @@ -502,6 +502,17 @@ static int __klp_disable_patch(struct klp_patch *patch)
> >> > >       return 0;
> >> > >  }
> >> > >
> >> > > +static int __klp_disable_patch_nolock(struct klp_patch *patch)
> >> > > +{
> >> > > +     int ret = 0;
> >> > > +
> >> > > +     ret = __klp_disable_patch(patch);
> >> > > +     if (ret)
> >> > > +             return ret;
> >> > > +     module_put(patch->mod);
> >> > > +     return ret;
> >> > > +}
> >> > > +
> >> >
> >> > Your patch doesn't solve the problem at all.
> >> >
> >> > There is no guarantee that once __klp_disable_patch() returns noone is
> >> > using the old code any more.
> >> >
> >>
> >> Yes, thanks.
> >>
> >> The __klp_disable_patch only guarantees that we will never call the
> >> function in patch module. For now, patch module can never be removed
> >> from the kernel once it was loaded. It may be inconvenience if we want
> >> re-load a new patch module replace the old patch module.
> >
> > Patch stacking is possible though.
> >
> 
> How about check the stack when start to remove the patch module? We
> can use the interface stop_machine to hang other CPU, then check and
> remove if no function in patch module is used.

Yeah, we'll eventually have something like that as part of the
consistency model.  The exact implementation is still undecided.  Until
then I think we want to disallow removing the patch module.

-- 
Josh

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] livepatch: Enhance livepatch to support remove patch module dynamically
  2015-04-01 14:13   ` Jiri Kosina
@ 2015-04-01 14:40     ` Minfei Huang
  2015-04-01 17:22       ` Josh Poimboeuf
  0 siblings, 1 reply; 5+ messages in thread
From: Minfei Huang @ 2015-04-01 14:40 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: jpoimboe, sjenning, vojtech, live-patching, linux-kernel

2015-04-01 22:13 GMT+08:00 Jiri Kosina <jkosina@suse.cz>:
> On Wed, 1 Apr 2015, Minfei Huang wrote:
>
>> > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
>> > > index 3f9f1d6..0266950 100644
>> > > --- a/kernel/livepatch/core.c
>> > > +++ b/kernel/livepatch/core.c
>> > > @@ -502,6 +502,17 @@ static int __klp_disable_patch(struct klp_patch *patch)
>> > >       return 0;
>> > >  }
>> > >
>> > > +static int __klp_disable_patch_nolock(struct klp_patch *patch)
>> > > +{
>> > > +     int ret = 0;
>> > > +
>> > > +     ret = __klp_disable_patch(patch);
>> > > +     if (ret)
>> > > +             return ret;
>> > > +     module_put(patch->mod);
>> > > +     return ret;
>> > > +}
>> > > +
>> >
>> > Your patch doesn't solve the problem at all.
>> >
>> > There is no guarantee that once __klp_disable_patch() returns noone is
>> > using the old code any more.
>> >
>>
>> Yes, thanks.
>>
>> The __klp_disable_patch only guarantees that we will never call the
>> function in patch module. For now, patch module can never be removed
>> from the kernel once it was loaded. It may be inconvenience if we want
>> re-load a new patch module replace the old patch module.
>
> Patch stacking is possible though.
>

How about check the stack when start to remove the patch module? We
can use the interface stop_machine to hang other CPU, then check and
remove if no function in patch module is used.

Thanks
Minfei

> --
> Jiri Kosina
> SUSE Labs

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] livepatch: Enhance livepatch to support remove patch module dynamically
  2015-04-01 14:00 ` Minfei Huang
@ 2015-04-01 14:13   ` Jiri Kosina
  2015-04-01 14:40     ` Minfei Huang
  0 siblings, 1 reply; 5+ messages in thread
From: Jiri Kosina @ 2015-04-01 14:13 UTC (permalink / raw)
  To: Minfei Huang; +Cc: jpoimboe, sjenning, vojtech, live-patching, linux-kernel

On Wed, 1 Apr 2015, Minfei Huang wrote:

> > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > > index 3f9f1d6..0266950 100644
> > > --- a/kernel/livepatch/core.c
> > > +++ b/kernel/livepatch/core.c
> > > @@ -502,6 +502,17 @@ static int __klp_disable_patch(struct klp_patch *patch)
> > >       return 0;
> > >  }
> > >
> > > +static int __klp_disable_patch_nolock(struct klp_patch *patch)
> > > +{
> > > +     int ret = 0;
> > > +
> > > +     ret = __klp_disable_patch(patch);
> > > +     if (ret)
> > > +             return ret;
> > > +     module_put(patch->mod);
> > > +     return ret;
> > > +}
> > > +
> >
> > Your patch doesn't solve the problem at all.
> >
> > There is no guarantee that once __klp_disable_patch() returns noone is
> > using the old code any more.
> >
> 
> Yes, thanks.
> 
> The __klp_disable_patch only guarantees that we will never call the
> function in patch module. For now, patch module can never be removed
> from the kernel once it was loaded. It may be inconvenience if we want
> re-load a new patch module replace the old patch module.

Patch stacking is possible though.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] livepatch: Enhance livepatch to support remove patch module dynamically
       [not found] <CAKoi8FLeHsdyVnnWLcBxCibFw_B3yXKvV8DBncEOGqBy1_zzuw@mail.gmail.com>
@ 2015-04-01 14:00 ` Minfei Huang
  2015-04-01 14:13   ` Jiri Kosina
  0 siblings, 1 reply; 5+ messages in thread
From: Minfei Huang @ 2015-04-01 14:00 UTC (permalink / raw)
  To: jpoimboe, sjenning, jkosina, vojtech
  Cc: live-patching, linux-kernel, Minfei Huang

2015-04-01 21:35 GMT+08:00 Jiri Kosina <jkosina@suse.cz>:
>
> On Wed, 1 Apr 2015, Minfei Huang wrote:
> >
> > Sorry, Use the gmail account to resend this patch, because the yahoo account cannt receive the maillist.
> >
> > ----
> >
> > As mentioned in the annotation, the patch module would not permit to
> > be removed once it is loaded.
> >
> > Kernel may crash in the case that the function in the patch module is
> > called during removing.
> >
> > We will increase the module reference when the patch module is enable,
> > so that the module cannt be removed. Once the module is disable, it will
> > be removed.
> >
> > Signed-off-by: Minfei Huang <mnfhuang@gmail.com>
> > ---
> >  kernel/livepatch/core.c | 47 +++++++++++++++++++++++++++++++----------------
> >  1 file changed, 31 insertions(+), 16 deletions(-)
> >
> > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> > index 3f9f1d6..0266950 100644
> > --- a/kernel/livepatch/core.c
> > +++ b/kernel/livepatch/core.c
> > @@ -502,6 +502,17 @@ static int __klp_disable_patch(struct klp_patch *patch)
> >       return 0;
> >  }
> >
> > +static int __klp_disable_patch_nolock(struct klp_patch *patch)
> > +{
> > +     int ret = 0;
> > +
> > +     ret = __klp_disable_patch(patch);
> > +     if (ret)
> > +             return ret;
> > +     module_put(patch->mod);
> > +     return ret;
> > +}
> > +
>
> Your patch doesn't solve the problem at all.
>
> There is no guarantee that once __klp_disable_patch() returns noone is
> using the old code any more.
>

Yes, thanks.

The __klp_disable_patch only guarantees that we will never call the
function in patch module. For now, patch module can never be removed
from the kernel once it was loaded. It may be inconvenience if we want
re-load a new patch module replace the old patch module.

Thanks
Minfei

> --
> Jiri Kosina
> SUSE Labs

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-04-01 17:23 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1427893812-15936-1-git-send-email-minfei.huang@yahoo.com>
2015-04-01 13:35 ` [PATCH] livepatch: Enhance livepatch to support remove patch module dynamically Jiri Kosina
     [not found] <CAKoi8FLeHsdyVnnWLcBxCibFw_B3yXKvV8DBncEOGqBy1_zzuw@mail.gmail.com>
2015-04-01 14:00 ` Minfei Huang
2015-04-01 14:13   ` Jiri Kosina
2015-04-01 14:40     ` Minfei Huang
2015-04-01 17:22       ` Josh Poimboeuf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).