[RFC] kreplace: Rebootless kernel updates

[RFC] kreplace: Rebootless kernel updates

[Nikanth Karthikesan]

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 <knikanth@suse.de>

---
 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);

Re: [RFC] kreplace: Rebootless kernel updates

[Ananth N Mavinakayanahalli]

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 <knikanth@suse.de>
> 
> ---
>  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

Re: [RFC] kreplace: Rebootless kernel updates

[Masami Hiramatsu]

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: mhiramat@redhat.com

Re: [RFC] kreplace: Rebootless kernel updates

[Balbir Singh]

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

Re: [RFC] kreplace: Rebootless kernel updates

[Chris Friesen]

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

Re: [RFC] kreplace: Rebootless kernel updates

[Anders Kaseorg]

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

Re: [RFC] kreplace: Rebootless kernel updates

[Jeff Arnold]

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
jbarnold@mit.edu

Re: [RFC] kreplace: Rebootless kernel updates

[Andi Kleen]

Jeff Arnold <jbarnold@MIT.EDU> 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

-- 
ak@linux.intel.com

Re: [RFC] kreplace: Rebootless kernel updates

[Jeff Arnold]

> 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
jbarnold@mit.edu

Re: [RFC] kreplace: Rebootless kernel updates

[Nikanth Karthikesan]

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 <knikanth@suse.de>
> >
> > ---
> >  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

Re: [RFC] kreplace: Rebootless kernel updates

[Nikanth Karthikesan]

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

Re: [RFC] kreplace: Rebootless kernel updates

[Nikanth Karthikesan]

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

Re: [RFC] kreplace: Rebootless kernel updates

[Nikanth Karthikesan]

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

Re: [RFC] kreplace: Rebootless kernel updates

[Masami Hiramatsu]

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: mhiramat@redhat.com

Re: [RFC] kreplace: Rebootless kernel updates

[Nikanth K]

Hi Masami

On Mon, Nov 24, 2008 at 8:45 PM, Masami Hiramatsu <mhiramat@redhat.com> 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