2008-11-21 11:48:56

by Nikanth Karthikesan

[permalink] [raw]
Subject: [RFC] kreplace: Rebootless kernel updates

This RFC patch adds support for limited form of rebootless kernel patching
even without building the entire kernel.

When looking for a shortcut to avoid the rebuild/reboot cycle when hacking the
kernel - the ksplice[1] was posted. This patch extends kprobes to do something
similar, which would require even lesser time to _experiment_ with the running
kernel.

This small patch extends jprobes so that the jprobe's handler is executed but
skips executing the actual function. But this has its own limitations such as
Cannot access symbols not exported for modules (ofcourse hacks like
pointers[2] can be used.), problems related to return values[3], etc... This
is currently a x86_64 only _hack_.

Thanks
Nikanth Karthikesan

[1] http://lkml.org/lkml/2008/9/13/6
[2] kallsyms_lookup_name may not be exported to the modules, but still one can
create and destroy a kprobe for a function to get its pointer!
[3] I wrote few helpers to handle return values of different sizes but omiting
them here to make it more readable. Patch for just no return value or
returning through accumulator is attached.

Sample code which uses this to replace the attempt_back_merge function in
block layer to always return zero, so that back merges would never be done.
The kernel patch follows this.

#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/kprobes.h>
#include <linux/blkdev.h>

/* New routine having the same arguments as actual routine */
int kreplace_attempt_back_merge(struct request_queue *q, struct request *rq)
{
static int count=0;
count++;
if (count%50 == 0)
printk("kreplace_attempt_back_merge called another 50 times\n");
set_ax(0); /* return value through ax - depends on arch, etc... */
jprobe_return();
return 0;// silence gcc
}

static struct jprobe my_kreplace = {
.entry = kreplace_attempt_back_merge,
.kp = {
.symbol_name = "attempt_back_merge",
},
};

static int __init jprobe_init(void)
{
int ret;

ret = register_kreplace(&my_kreplace);
if (ret < 0) {
printk(KERN_INFO "register_kreplace failed, returned %d\n", ret);
return -1;
}
printk(KERN_INFO "Planted kreplace at %p, handler addr %p\n",
my_kreplace.kp.addr, my_kreplace.entry);
return 0;
}

static void __exit jprobe_exit(void)
{
unregister_kreplace(&my_kreplace);
printk(KERN_INFO "kreplace at %p unregistered\n", my_kreplace.kp.addr);
}

module_init(jprobe_init)
module_exit(jprobe_exit)
MODULE_LICENSE("GPL");

---

The kernel patch for kreplace, an extension to kprobes to do hot patching.
Only on x86_64. Do not try this on any other platforms without modifying.

Signed-off-by: Nikanth Karthikesan <[email protected]>

---
arch/x86/kernel/kprobes.c | 18 ++++++++++++++----
include/linux/kprobes.h | 5 ++++-
kernel/kprobes.c | 37 ++++++++++++++++++++++++++++++++-----
3 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index 6c27679..9e2ea2b 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -340,9 +340,13 @@ static void __kprobes fix_riprel(struct kprobe *p)
#endif
}

-static void __kprobes arch_copy_kprobe(struct kprobe *p)
+static void __kprobes arch_copy_kprobe(struct kprobe *p, int replace)
{
- memcpy(p->ainsn.insn, p->addr, MAX_INSN_SIZE * sizeof(kprobe_opcode_t));
+ if (replace)
+ memcpy(p->ainsn.insn, ((unsigned char []){0xc3}), 1);
+ else
+ memcpy(p->ainsn.insn, p->addr,
+ MAX_INSN_SIZE * sizeof(kprobe_opcode_t));

fix_riprel(p);

@@ -354,13 +358,13 @@ static void __kprobes arch_copy_kprobe(struct kprobe *p)
p->opcode = *p->addr;
}

-int __kprobes arch_prepare_kprobe(struct kprobe *p)
+int __kprobes arch_prepare_kprobe(struct kprobe *p, int replace)
{
/* insn: must be on special executable page on x86. */
p->ainsn.insn = get_insn_slot();
if (!p->ainsn.insn)
return -ENOMEM;
- arch_copy_kprobe(p);
+ arch_copy_kprobe(p, replace);
return 0;
}

@@ -1023,6 +1027,12 @@ void __kprobes jprobe_return(void)
(kcb->jprobe_saved_sp):"memory");
}

+void __kprobes set_ax(unsigned long ax)
+{
+ struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+ kcb->jprobe_saved_regs.ax = ax;
+}
+
int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)
{
struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 497b1d1..91e83fb 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -202,7 +202,7 @@ static inline int init_test_probes(void)
#endif /* CONFIG_KPROBES_SANITY_TEST */

extern struct mutex kprobe_mutex;
-extern int arch_prepare_kprobe(struct kprobe *p);
+extern int arch_prepare_kprobe(struct kprobe *p, int replace);
extern void arch_arm_kprobe(struct kprobe *p);
extern void arch_disarm_kprobe(struct kprobe *p);
extern int arch_init_kprobes(void);
@@ -240,11 +240,14 @@ int register_kprobes(struct kprobe **kps, int num);
void unregister_kprobes(struct kprobe **kps, int num);
int setjmp_pre_handler(struct kprobe *, struct pt_regs *);
int longjmp_break_handler(struct kprobe *, struct pt_regs *);
+int register_kreplace(struct jprobe *p);
+void unregister_kreplace(struct jprobe *p);
int register_jprobe(struct jprobe *p);
void unregister_jprobe(struct jprobe *p);
int register_jprobes(struct jprobe **jps, int num);
void unregister_jprobes(struct jprobe **jps, int num);
void jprobe_return(void);
+void set_ax(unsigned long);
unsigned long arch_deref_entry_point(void *);

int register_kretprobe(struct kretprobe *rp);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 8b57a25..8da3be7 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -269,6 +269,7 @@ void __kprobes free_insn_slot(kprobe_opcode_t * slot, int
dirty)
collect_garbage_slots();
}
#endif
+EXPORT_SYMBOL_GPL(set_ax);

/* We have preemption disabled.. so it is safe to use __ versions */
static inline void set_kprobe_instance(struct kprobe *kp)
@@ -601,7 +602,7 @@ static kprobe_opcode_t __kprobes *kprobe_addr(struct
kprobe *p)
}

static int __kprobes __register_kprobe(struct kprobe *p,
- unsigned long called_from)
+ unsigned long called_from, int replace)
{
int ret = 0;
struct kprobe *old_p;
@@ -647,7 +648,7 @@ static int __kprobes __register_kprobe(struct kprobe *p,
goto out;
}

- ret = arch_prepare_kprobe(p);
+ ret = arch_prepare_kprobe(p, replace);
if (ret)
goto out;

@@ -742,7 +743,7 @@ static int __register_kprobes(struct kprobe **kps, int
num,
if (num <= 0)
return -EINVAL;
for (i = 0; i < num; i++) {
- ret = __register_kprobe(kps[i], called_from);
+ ret = __register_kprobe(kps[i], called_from, 0);
if (ret < 0) {
if (i > 0)
unregister_kprobes(kps, i);
@@ -819,7 +820,7 @@ static int __register_jprobes(struct jprobe **jps, int
num,
/* Todo: Verify probepoint is a function entry point */
jp->kp.pre_handler = setjmp_pre_handler;
jp->kp.break_handler = longjmp_break_handler;
- ret = __register_kprobe(&jp->kp, called_from);
+ ret = __register_kprobe(&jp->kp, called_from, 0);
}
if (ret < 0) {
if (i > 0)
@@ -836,11 +837,35 @@ int __kprobes register_jprobe(struct jprobe *jp)
(unsigned long)__builtin_return_address(0));
}

+int __kprobes register_kreplace(struct jprobe *jp)
+{
+ int ret = 0;
+ unsigned long addr;
+
+ addr = arch_deref_entry_point(jp->entry);
+
+ if (!kernel_text_address(addr))
+ ret = -EINVAL;
+ else {
+ /* Todo: Verify probepoint is a function entry point */
+ jp->kp.pre_handler = setjmp_pre_handler;
+ jp->kp.break_handler = longjmp_break_handler;
+ ret = __register_kprobe(&jp->kp,
+ (unsigned long)__builtin_return_address(0), 1);
+ }
+ return ret;
+}
+
void __kprobes unregister_jprobe(struct jprobe *jp)
{
unregister_jprobes(&jp, 1);
}

+void __kprobes unregister_kreplace(struct jprobe *jp)
+{
+ unregister_jprobes(&jp, 1);
+}
+
int __kprobes register_jprobes(struct jprobe **jps, int num)
{
return __register_jprobes(jps, num,
@@ -956,7 +981,7 @@ static int __kprobes __register_kretprobe(struct kretprobe
*rp,

rp->nmissed = 0;
/* Establish function entry probe point */
- ret = __register_kprobe(&rp->kp, called_from);
+ ret = __register_kprobe(&rp->kp, called_from, 0);
if (ret != 0)
free_rp_inst(rp);
return ret;
@@ -1333,6 +1358,8 @@ EXPORT_SYMBOL_GPL(register_kprobe);
EXPORT_SYMBOL_GPL(unregister_kprobe);
EXPORT_SYMBOL_GPL(register_kprobes);
EXPORT_SYMBOL_GPL(unregister_kprobes);
+EXPORT_SYMBOL_GPL(register_kreplace);
+EXPORT_SYMBOL_GPL(unregister_kreplace);
EXPORT_SYMBOL_GPL(register_jprobe);
EXPORT_SYMBOL_GPL(unregister_jprobe);
EXPORT_SYMBOL_GPL(register_jprobes);


Subject: Re: [RFC] kreplace: Rebootless kernel updates

On Fri, Nov 21, 2008 at 05:20:25PM +0530, Nikanth Karthikesan wrote:
> This RFC patch adds support for limited form of rebootless kernel patching
> even without building the entire kernel.
>
> When looking for a shortcut to avoid the rebuild/reboot cycle when hacking the
> kernel - the ksplice[1] was posted. This patch extends kprobes to do something
> similar, which would require even lesser time to _experiment_ with the running
> kernel.

There have been other implementations of this feature, I am sure quite a
few people would have objections to having this as part of the kernel :-)

> This small patch extends jprobes so that the jprobe's handler is executed but
> skips executing the actual function. But this has its own limitations such as
> Cannot access symbols not exported for modules (ofcourse hacks like
> pointers[2] can be used.), problems related to return values[3], etc... This
> is currently a x86_64 only _hack_.

There are many other issues too... How do you enforce correct usage of this
infrastrucutre? What prevents people from overriding core-kernel
functions with their own?

Kprobes themselves provide enough ammunition to users to shoot themselves
in the foot, but this is way more dangerous than that.
...

> The kernel patch for kreplace, an extension to kprobes to do hot patching.
> Only on x86_64. Do not try this on any other platforms without modifying.
>
> Signed-off-by: Nikanth Karthikesan <[email protected]>
>
> ---
> arch/x86/kernel/kprobes.c | 18 ++++++++++++++----
> include/linux/kprobes.h | 5 ++++-
> kernel/kprobes.c | 37 ++++++++++++++++++++++++++++++++-----
> 3 files changed, 50 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
> index 6c27679..9e2ea2b 100644
> --- a/arch/x86/kernel/kprobes.c
> +++ b/arch/x86/kernel/kprobes.c
> @@ -340,9 +340,13 @@ static void __kprobes fix_riprel(struct kprobe *p)
> #endif
> }
>
> -static void __kprobes arch_copy_kprobe(struct kprobe *p)
> +static void __kprobes arch_copy_kprobe(struct kprobe *p, int replace)
> {
> - memcpy(p->ainsn.insn, p->addr, MAX_INSN_SIZE * sizeof(kprobe_opcode_t));
> + if (replace)
> + memcpy(p->ainsn.insn, ((unsigned char []){0xc3}), 1);
> + else
> + memcpy(p->ainsn.insn, p->addr,
> + MAX_INSN_SIZE * sizeof(kprobe_opcode_t));

This is limiting - especially since we allow multiple probes at the same
address. You modify the instruction underneath to always be a ret.

It also breaks existing functionality -- especially aggregate probes and
return probes.

...

> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 497b1d1..91e83fb 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -202,7 +202,7 @@ static inline int init_test_probes(void)
> #endif /* CONFIG_KPROBES_SANITY_TEST */
>
> extern struct mutex kprobe_mutex;
> -extern int arch_prepare_kprobe(struct kprobe *p);
> +extern int arch_prepare_kprobe(struct kprobe *p, int replace);
> extern void arch_arm_kprobe(struct kprobe *p);
> extern void arch_disarm_kprobe(struct kprobe *p);
> extern int arch_init_kprobes(void);
> @@ -240,11 +240,14 @@ int register_kprobes(struct kprobe **kps, int num);
> void unregister_kprobes(struct kprobe **kps, int num);
> int setjmp_pre_handler(struct kprobe *, struct pt_regs *);
> int longjmp_break_handler(struct kprobe *, struct pt_regs *);
> +int register_kreplace(struct jprobe *p);
> +void unregister_kreplace(struct jprobe *p);
> int register_jprobe(struct jprobe *p);
> void unregister_jprobe(struct jprobe *p);
> int register_jprobes(struct jprobe **jps, int num);
> void unregister_jprobes(struct jprobe **jps, int num);
> void jprobe_return(void);
> +void set_ax(unsigned long);

Please choose a better arch agnostic naming scheme -- set_ret()?

Ananth

2008-11-21 14:42:00

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [RFC] kreplace: Rebootless kernel updates

Hi Nikanth,

Nikanth Karthikesan wrote:
> This RFC patch adds support for limited form of rebootless kernel patching
> even without building the entire kernel.
>
> When looking for a shortcut to avoid the rebuild/reboot cycle when hacking the
> kernel - the ksplice[1] was posted. This patch extends kprobes to do something
> similar, which would require even lesser time to _experiment_ with the running
> kernel.
>
> This small patch extends jprobes so that the jprobe's handler is executed but
> skips executing the actual function. But this has its own limitations such as
> Cannot access symbols not exported for modules (ofcourse hacks like
> pointers[2] can be used.), problems related to return values[3], etc... This
> is currently a x86_64 only _hack_.

Hmm,
Would you like to replace a function to another function?
If so, AFAIK, you can do that with kprobe and below pre_handler.
(see booster enabled path in setup_singlestep())

pre_handler(...)
{
reset_current_kprobe(); /* this kprobe doesn't need any more */
regs->ip = new_function; /* change IP to new function */
preempt_enable_no_resched(); /* recover preempt count */
return 1; /* No need to setup singlestep */
}

Thank you,


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: [email protected]

2008-11-21 16:08:56

by Balbir Singh

[permalink] [raw]
Subject: Re: [RFC] kreplace: Rebootless kernel updates

Ananth N Mavinakayanahalli wrote:
> On Fri, Nov 21, 2008 at 05:20:25PM +0530, Nikanth Karthikesan wrote:
>> This RFC patch adds support for limited form of rebootless kernel patching
>> even without building the entire kernel.
>>
>> When looking for a shortcut to avoid the rebuild/reboot cycle when hacking the
>> kernel - the ksplice[1] was posted. This patch extends kprobes to do something
>> similar, which would require even lesser time to _experiment_ with the running
>> kernel.
>
> There have been other implementations of this feature, I am sure quite a
> few people would have objections to having this as part of the kernel :-)
>

I had a patch for x86 a long time ago in 2005, but I never posted it :(

>> This small patch extends jprobes so that the jprobe's handler is executed but
>> skips executing the actual function. But this has its own limitations such as
>> Cannot access symbols not exported for modules (ofcourse hacks like
>> pointers[2] can be used.), problems related to return values[3], etc... This
>> is currently a x86_64 only _hack_.
>
> There are many other issues too... How do you enforce correct usage of this
> infrastrucutre? What prevents people from overriding core-kernel
> functions with their own?
>

Yes and we need to be careful about licensing, tainting the kernel with such an
implementation.

> Kprobes themselves provide enough ammunition to users to shoot themselves
> in the foot, but this is way more dangerous than that.
> ...
>

Undoubtedly, but a good warning is the best way to keep people warned about
running such code :) It is a useful thing to have and to run, but running it
would take more guts than anything else.



--
Balbir

2008-11-21 17:29:40

by Chris Friesen

[permalink] [raw]
Subject: Re: [RFC] kreplace: Rebootless kernel updates

Balbir Singh wrote:

> Undoubtedly, but a good warning is the best way to keep people warned about
> running such code :) It is a useful thing to have and to run, but running it
> would take more guts than anything else.

There is significant demand for this sort of thing in the
telecom/enterprise space, where they would like to apply kernel patches
(especially security patches and bugfixes) with minimal downtime.

Chris

2008-11-21 20:20:25

by Anders Kaseorg

[permalink] [raw]
Subject: Re: [RFC] kreplace: Rebootless kernel updates

On Fri, 21 Nov 2008, Nikanth Karthikesan wrote:
> When looking for a shortcut to avoid the rebuild/reboot cycle when
> hacking the kernel - the ksplice[1] was posted. This patch extends
> kprobes to do something similar, which would require even lesser time to
> _experiment_ with the running kernel.

Maybe we haven’t done a good job of explaining just how quickly you can
use Ksplice for rapid experimentation. Using the Ksplice utilities
<http://www.ksplice.com/software>, you can write a 5-line patch today that
does the same experiment as your 42-line kreplace example, without using
unsafe hacks:

$ cat example.patch
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -488,6 +488,11 @@

int attempt_back_merge(struct request_queue *q, struct request *rq)
{
+ static int count = 0;
+ count++;
+ if (count % 50 == 0)
+ printk("attempt_back_merge called another 50 times\n");
+ return 0;
struct request *next = elv_latter_request(q, rq);

if (next)
$ ksplice-create -j4 --patch=example.patch linux-2.6/
Ksplice update tarball written to ksplice-sxk9pvow.tar.gz
$ sudo ksplice-apply ksplice-sxk9pvow.tar.gz
Done!
$ sudo ksplice-undo sxk9pvow

Assuming the kernel source tree has been prepared for Ksplice (e.g. if you
have used Ksplice before), it takes as little as 35 seconds to compile the
Ksplice update and a fraction of a second to apply it. The running kernel
does not need to have been specially prepared or patched.

> This small patch extends jprobes so that the jprobe's handler is
> executed but skips executing the actual function. But this has its own
> limitations such as Cannot access symbols not exported for modules
> (ofcourse hacks like pointers[2] can be used.), problems related to
> return values[3], etc... This is currently a x86_64 only _hack_.

An even more fundamental limitation of kprobes/jprobes is that it cannot
hook functions that have been inlined (or partially inlined) by the
compiler, because the compiler only writes mcount() calls at the beginning
of function bodies after inlining has been performed. (Note that at least
20% of functions in the kernel that don’t have an explicit inline keyword
get inlined anyway.)

This problem, and others such as the ambiguous symbol name problem (see
<http://www.ksplice.com/paper>), mean that jprobes doesn’t work well for
much more than gathering statistics and debugging, which is what it was
designed for. Building hot updates with jprobes makes for a cute jprobes
example, but let’s be clear: it isn’t nearly robust enough for security
patches or telecom/enterprise use.

Ksplice solves all of these problems, and additionally includes extensive
safety checks (run-pre matching, kernel stack checking, update dependency
tracking) to avoid a wide range of dangerous situations, which would
commonly cause a simplistic hot update system to corrupt the running
kernel.

Anders

2008-11-22 03:54:19

by Jeff Arnold

[permalink] [raw]
Subject: Re: [RFC] kreplace: Rebootless kernel updates

Hello,

> [kreplace] is currently a x86_64 only _hack_

The point of Ksplice is to be a hot update system that isn't a hack.
Ksplice is quite far along at this point; it implements many safety and
automation features that kreplace doesn't address. Is there something in
particular that discouraged you from using Ksplice?

Jeff Arnold
[email protected]

2008-11-23 19:39:19

by Andi Kleen

[permalink] [raw]
Subject: Re: [RFC] kreplace: Rebootless kernel updates

Jeff Arnold <[email protected]> writes:

> Hello,
>
>> [kreplace] is currently a x86_64 only _hack_
>
> The point of Ksplice is to be a hot update system that isn't a hack.
> Ksplice is quite far along at this point; it implements many safety and
> automation features that kreplace doesn't address. Is there something in
> particular that discouraged you from using Ksplice?

I suspect that it wasn't mainline?

The usual problem is that it's much easier to get simple patches
in than complicated ones.

-Andi

--
[email protected]

2008-11-23 20:53:46

by Jeff Arnold

[permalink] [raw]
Subject: Re: [RFC] kreplace: Rebootless kernel updates

> I suspect that it wasn't mainline?

OK. We're aware of that problem and are working on it ;)

> The usual problem is that it's much easier to get simple patches
> in than complicated ones.

Right. Unfortunately, if you make a hot update system's design too
simple, it can be dangerous and will potentially do more harm than good.

If there's anything that I can do to help people understand our code and
why we think that it's as simple as you'd want a hot update system to be,
please let me know.

Jeff Arnold
[email protected]

2008-11-24 11:06:41

by Nikanth Karthikesan

[permalink] [raw]
Subject: Re: [RFC] kreplace: Rebootless kernel updates

On Friday 21 November 2008 19:08:00 Ananth N Mavinakayanahalli wrote:
> On Fri, Nov 21, 2008 at 05:20:25PM +0530, Nikanth Karthikesan wrote:
> > This RFC patch adds support for limited form of rebootless kernel
> > patching even without building the entire kernel.
> >
> > When looking for a shortcut to avoid the rebuild/reboot cycle when
> > hacking the kernel - the ksplice[1] was posted. This patch extends
> > kprobes to do something similar, which would require even lesser time to
> > _experiment_ with the running kernel.
>
> There have been other implementations of this feature, I am sure quite a
> few people would have objections to having this as part of the kernel :-)
>

I think the few would be quiet large ;)

> > This small patch extends jprobes so that the jprobe's handler is executed
> > but skips executing the actual function. But this has its own limitations
> > such as Cannot access symbols not exported for modules (ofcourse hacks
> > like pointers[2] can be used.), problems related to return values[3],
> > etc... This is currently a x86_64 only _hack_.
>
> There are many other issues too... How do you enforce correct usage of this
> infrastrucutre? What prevents people from overriding core-kernel
> functions with their own?
>

I agree, this is incomplete.

> Kprobes themselves provide enough ammunition to users to shoot themselves
> in the foot, but this is way more dangerous than that.
> ...
>

Yes.

> > The kernel patch for kreplace, an extension to kprobes to do hot
> > patching. Only on x86_64. Do not try this on any other platforms without
> > modifying.
> >
> > Signed-off-by: Nikanth Karthikesan <[email protected]>
> >
> > ---
> > arch/x86/kernel/kprobes.c | 18 ++++++++++++++----
> > include/linux/kprobes.h | 5 ++++-
> > kernel/kprobes.c | 37 ++++++++++++++++++++++++++++++++-----
> > 3 files changed, 50 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
> > index 6c27679..9e2ea2b 100644
> > --- a/arch/x86/kernel/kprobes.c
> > +++ b/arch/x86/kernel/kprobes.c
> > @@ -340,9 +340,13 @@ static void __kprobes fix_riprel(struct kprobe *p)
> > #endif
> > }
> >
> > -static void __kprobes arch_copy_kprobe(struct kprobe *p)
> > +static void __kprobes arch_copy_kprobe(struct kprobe *p, int replace)
> > {
> > - memcpy(p->ainsn.insn, p->addr, MAX_INSN_SIZE *
> > sizeof(kprobe_opcode_t)); + if (replace)
> > + memcpy(p->ainsn.insn, ((unsigned char []){0xc3}), 1);
> > + else
> > + memcpy(p->ainsn.insn, p->addr,
> > + MAX_INSN_SIZE * sizeof(kprobe_opcode_t));
>
> This is limiting - especially since we allow multiple probes at the same
> address. You modify the instruction underneath to always be a ret.
>
> It also breaks existing functionality -- especially aggregate probes and
> return probes.
>

Oh, yeah. And it is possible to implement this correctly in other ways!

> ...
>
> > diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> > index 497b1d1..91e83fb 100644
> > --- a/include/linux/kprobes.h
> > +++ b/include/linux/kprobes.h
> > @@ -202,7 +202,7 @@ static inline int init_test_probes(void)
> > #endif /* CONFIG_KPROBES_SANITY_TEST */
> >
> > extern struct mutex kprobe_mutex;
> > -extern int arch_prepare_kprobe(struct kprobe *p);
> > +extern int arch_prepare_kprobe(struct kprobe *p, int replace);
> > extern void arch_arm_kprobe(struct kprobe *p);
> > extern void arch_disarm_kprobe(struct kprobe *p);
> > extern int arch_init_kprobes(void);
> > @@ -240,11 +240,14 @@ int register_kprobes(struct kprobe **kps, int num);
> > void unregister_kprobes(struct kprobe **kps, int num);
> > int setjmp_pre_handler(struct kprobe *, struct pt_regs *);
> > int longjmp_break_handler(struct kprobe *, struct pt_regs *);
> > +int register_kreplace(struct jprobe *p);
> > +void unregister_kreplace(struct jprobe *p);
> > int register_jprobe(struct jprobe *p);
> > void unregister_jprobe(struct jprobe *p);
> > int register_jprobes(struct jprobe **jps, int num);
> > void unregister_jprobes(struct jprobe **jps, int num);
> > void jprobe_return(void);
> > +void set_ax(unsigned long);
>
> Please choose a better arch agnostic naming scheme -- set_ret()?
>

I did write more helpers to return values of different sizes, but only their
function names look good. So, I didnt post them as I wanted to know the
comments for the idea first.

Thanks a lot for your comments.

Thanks
Nikanth




2008-11-24 11:06:56

by Nikanth Karthikesan

[permalink] [raw]
Subject: Re: [RFC] kreplace: Rebootless kernel updates

On Friday 21 November 2008 20:09:52 Masami Hiramatsu wrote:
> Hi Nikanth,
>
> Nikanth Karthikesan wrote:

> Hmm,
> Would you like to replace a function to another function?
> If so, AFAIK, you can do that with kprobe and below pre_handler.
> (see booster enabled path in setup_singlestep())
>
> pre_handler(...)
> {
> reset_current_kprobe(); /* this kprobe doesn't need any more */
> regs->ip = new_function; /* change IP to new function */
> preempt_enable_no_resched(); /* recover preempt count */
> return 1; /* No need to setup singlestep */
> }
>
>

So, am I seeing worries to enable something, which is already possible, but
just not documented?

Thanks
Nikanth Karthikesan


2008-11-24 11:07:22

by Nikanth Karthikesan

[permalink] [raw]
Subject: Re: [RFC] kreplace: Rebootless kernel updates

On Saturday 22 November 2008 09:16:59 Jeff Arnold wrote:
> Hello,
>
> > [kreplace] is currently a x86_64 only _hack_
>
> The point of Ksplice is to be a hot update system that isn't a hack.
> Ksplice is quite far along at this point; it implements many safety and
> automation features that kreplace doesn't address.

I agree.

> Is there something in particular that discouraged you from using Ksplice?
>

It is not mainline yet. And just the curiosity of doing it some other way with
minimal code.

Thanks
Nikanth Karthikesan

2008-11-24 11:07:51

by Nikanth Karthikesan

[permalink] [raw]
Subject: Re: [RFC] kreplace: Rebootless kernel updates

On Friday 21 November 2008 21:34:43 Balbir Singh wrote:
> Ananth N Mavinakayanahalli wrote:
> > On Fri, Nov 21, 2008 at 05:20:25PM +0530, Nikanth Karthikesan wrote:
>
> I had a patch for x86 a long time ago in 2005, but I never posted it :(
>

I just didn't wanted to say that after some time. ;)

>
> Yes and we need to be careful about licensing, tainting the kernel with
> such an implementation.
>

Agreed.

> > Kprobes themselves provide enough ammunition to users to shoot themselves
> > in the foot, but this is way more dangerous than that.
> > ...
>
> Undoubtedly, but a good warning is the best way to keep people warned about
> running such code :) It is a useful thing to have and to run, but running
> it would take more guts than anything else.

Thanks for the comments.

Thanks
Nikanth Karthikesan

2008-11-24 15:16:31

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [RFC] kreplace: Rebootless kernel updates

Nikanth Karthikesan wrote:
> On Friday 21 November 2008 20:09:52 Masami Hiramatsu wrote:
>> Hi Nikanth,
>>
>> Nikanth Karthikesan wrote:
>
>> Hmm,
>> Would you like to replace a function to another function?
>> If so, AFAIK, you can do that with kprobe and below pre_handler.
>> (see booster enabled path in setup_singlestep())
>>
>> pre_handler(...)
>> {
>> reset_current_kprobe(); /* this kprobe doesn't need any more */
>> regs->ip = new_function; /* change IP to new function */
>> preempt_enable_no_resched(); /* recover preempt count */
>> return 1; /* No need to setup singlestep */
>> }
>>
>>
>
> So, am I seeing worries to enable something, which is already possible, but
> just not documented?

Actually, you can do this with kprobes, but not documented,
because the regs->ip setup depends on arch. (above code is only for
x86/x86-64).

I think it's easier to just provide above pre_handler for each arch than
jprobe-based kreplace approach.

Thank you,

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: [email protected]

2008-11-26 02:48:17

by Nikanth K

[permalink] [raw]
Subject: Re: [RFC] kreplace: Rebootless kernel updates

Hi Masami

On Mon, Nov 24, 2008 at 8:45 PM, Masami Hiramatsu <[email protected]> wrote:
> Nikanth Karthikesan wrote:
>> On Friday 21 November 2008 20:09:52 Masami Hiramatsu wrote:
>>> Hi Nikanth,
>>>
>>> Nikanth Karthikesan wrote:
>>
>>> Hmm,
>>> Would you like to replace a function to another function?
>>> If so, AFAIK, you can do that with kprobe and below pre_handler.
>>> (see booster enabled path in setup_singlestep())
>>>
>>> pre_handler(...)
>>> {
>>> reset_current_kprobe(); /* this kprobe doesn't need any more */
>>> regs->ip = new_function; /* change IP to new function */
>>> preempt_enable_no_resched(); /* recover preempt count */
>>> return 1; /* No need to setup singlestep */
>>> }
>>>
>>>
>>
>> So, am I seeing worries to enable something, which is already possible, but
>> just not documented?
>
> Actually, you can do this with kprobes, but not documented,
> because the regs->ip setup depends on arch. (above code is only for
> x86/x86-64).
>
> I think it's easier to just provide above pre_handler for each arch than
> jprobe-based kreplace approach.
>

I thought jprobes would provide easy access to function parameters.

Thanks
Nikanth