LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH][kprobes] support kretprobe-blacklist
@ 2007-08-17 19:43 Masami Hiramatsu
  2007-08-17 22:03 ` Andrew Morton
  2007-08-27 13:31 ` [PATCH][kprobes] support kretprobe-blacklist Christoph Hellwig
  0 siblings, 2 replies; 7+ messages in thread
From: Masami Hiramatsu @ 2007-08-17 19:43 UTC (permalink / raw)
  To: Andrew Morton, ananth, prasanna, anil.s.keshavamurthy,
	Jim Keniston, davem
  Cc: linux-kernel

This patch introduces architecture dependent kretprobe
blacklists to prohibit users from inserting return
probes on the function in which kprobes can be inserted
but kretprobes can not.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>

---
 <Problem Description>
When a kretprobe is inserted in the entry of the "__switch_to()",
it causes kernel panic on i386 with recent kernel.

 <Problem Analysis>
In include/asm-i386/current.h, "current" is defined as an entry of
percpu variables.:

 DECLARE_PER_CPU(struct task_struct *, current_task);
 static __always_inline struct task_struct *get_current(void)
 {
         return x86_read_percpu(current_task);
 }

 #define current get_current()

This mean the "current" macro is separated from its stack register.
Kretprobe expects that "current" value when a function is called is
not changed, or both of "current" value and stack register are changed in
the target function.
But __switch_to() in arch/i386/kernel/process.c changes only the value of
"current", but doesn't changes the stack register(this was already switched
to new stack before calling __switch_to()):

 struct task_struct fastcall * __switch_to(struct task_struct *prev_p, struct
 task_struct *next_p)
 {
 	...
        x86_write_percpu(current_task, next_p);

        return prev_p;
 }

Kretprobe modifies the return address stored in the stack, and
it saves the original return address in the kretprobe_instance with
the value of "current".
When kretprobe is hit at the return of __switch_to(), it searches
kretprobe_instance related to the probe point from a hash list by
using the hash-value of the "current" instead of stack address.
However, since the value of "current" is already changed,
it can't find proper instance, and invokes BUG() macro.

 <Solution>
As a result of discussion with other kprobe developers, I'd
like to introduce arch-dependent blacklist.
To introduce "__kretprobes" mark (like "__kprobes") is another way.
But I thought it is not efficient way, because the kretprobe can
be inserted only in the entry of functions and there is no need to
check against a whole function.

This patch also removes "__kprobes" mark from "__switch_to" on x86_64
and registers "__switch_to" to the blacklist on x86-64, because that mark
is to prohibit user from inserting only kretprobe.

Thank you,

Masami Hiramatsu
Hitachi Computer Products (America), Inc.

 arch/i386/kernel/kprobes.c   |    5 +++++
 arch/x86_64/kernel/kprobes.c |    6 ++++++
 arch/x86_64/kernel/process.c |    2 +-
 include/asm-i386/kprobes.h   |    1 +
 include/asm-x86_64/kprobes.h |    1 +
 include/linux/kprobes.h      |    8 ++++++++
 kernel/kprobes.c             |   23 +++++++++++++++++++++++
 7 files changed, 45 insertions(+), 1 deletion(-)

Index: 2.6-mm/arch/i386/kernel/kprobes.c
===================================================================
--- 2.6-mm.orig/arch/i386/kernel/kprobes.c
+++ 2.6-mm/arch/i386/kernel/kprobes.c
@@ -41,6 +41,11 @@ void jprobe_return_end(void);

 DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
 DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
+struct kretprobe_blacklist arch_kretprobe_blacklist[] = {
+	{"__switch_to", }, /* This function switches only current task, but
+			     doesn't switch kernel stack.*/
+	{NULL, NULL}	/* Terminator */
+};

 /* insert a jmp code */
 static __always_inline void set_jmp_op(void *from, void *to)
Index: 2.6-mm/include/asm-i386/kprobes.h
===================================================================
--- 2.6-mm.orig/include/asm-i386/kprobes.h
+++ 2.6-mm/include/asm-i386/kprobes.h
@@ -44,6 +44,7 @@ typedef u8 kprobe_opcode_t;

 #define ARCH_SUPPORTS_KRETPROBES
 #define flush_insn_slot(p)	do { } while (0)
+#define ARCH_SUPPORTS_KRETPROBE_BLACKLIST

 void arch_remove_kprobe(struct kprobe *p);
 void kretprobe_trampoline(void);
Index: 2.6-mm/kernel/kprobes.c
===================================================================
--- 2.6-mm.orig/kernel/kprobes.c
+++ 2.6-mm/kernel/kprobes.c
@@ -716,6 +716,18 @@ int __kprobes register_kretprobe(struct
 	int ret = 0;
 	struct kretprobe_instance *inst;
 	int i;
+#ifdef ARCH_SUPPORTS_KRETPROBE_BLACKLIST
+	void *addr = rp->kp.addr;
+
+	if (addr == NULL)
+		kprobe_lookup_name(rp->kp.symbol_name, addr);
+	addr += rp->kp.offset;
+
+	for (i = 0; arch_kretprobe_blacklist[i].name != NULL; i++) {
+		if (arch_kretprobe_blacklist[i].addr == addr)
+			return -EINVAL;
+	}
+#endif

 	rp->kp.pre_handler = pre_handler_kretprobe;
 	rp->kp.post_handler = NULL;
@@ -794,6 +806,17 @@ static int __init init_kprobes(void)
 		INIT_HLIST_HEAD(&kretprobe_inst_table[i]);
 	}

+#ifdef ARCH_SUPPORTS_KRETPROBE_BLACKLIST
+	/* lookup the function address from its name */
+	for (i = 0; arch_kretprobe_blacklist[i].name != NULL; i++) {
+		kprobe_lookup_name(arch_kretprobe_blacklist[i].name,
+				   arch_kretprobe_blacklist[i].addr);
+		if (!arch_kretprobe_blacklist[i].addr)
+			printk("kretprobe: Unknown blacklisted function: %s\n",
+			       arch_kretprobe_blacklist[i].name);
+	}
+#endif
+
 	/* By default, kprobes are enabled */
 	kprobe_enabled = true;

Index: 2.6-mm/arch/x86_64/kernel/kprobes.c
===================================================================
--- 2.6-mm.orig/arch/x86_64/kernel/kprobes.c
+++ 2.6-mm/arch/x86_64/kernel/kprobes.c
@@ -49,6 +49,12 @@ static void __kprobes arch_copy_kprobe(s
 DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
 DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);

+struct kretprobe_blacklist arch_kretprobe_blacklist[] = {
+	{"__switch_to", }, /* This function switches only current task, but
+			      doesn't switch kernel stack.*/
+	{NULL, NULL}	/* Terminator */
+};
+
 /*
  * returns non-zero if opcode modifies the interrupt flag.
  */
Index: 2.6-mm/include/asm-x86_64/kprobes.h
===================================================================
--- 2.6-mm.orig/include/asm-x86_64/kprobes.h
+++ 2.6-mm/include/asm-x86_64/kprobes.h
@@ -42,6 +42,7 @@ typedef u8 kprobe_opcode_t;
 	: (((unsigned long)current_thread_info()) + THREAD_SIZE - (ADDR)))

 #define ARCH_SUPPORTS_KRETPROBES
+#define ARCH_SUPPORTS_KRETPROBE_BLACKLIST

 void kretprobe_trampoline(void);
 extern void arch_remove_kprobe(struct kprobe *p);
Index: 2.6-mm/include/linux/kprobes.h
===================================================================
--- 2.6-mm.orig/include/linux/kprobes.h
+++ 2.6-mm/include/linux/kprobes.h
@@ -166,6 +166,14 @@ struct kretprobe_instance {
 	struct task_struct *task;
 };

+#ifdef ARCH_SUPPORTS_KRETPROBE_BLACKLIST
+struct kretprobe_blacklist {
+	const char *name;
+	void *addr;
+};
+extern struct kretprobe_blacklist arch_kretprobe_blacklist[];
+#endif
+
 static inline void kretprobe_assert(struct kretprobe_instance *ri,
 	unsigned long orig_ret_address, unsigned long trampoline_address)
 {
Index: 2.6-mm/arch/x86_64/kernel/process.c
===================================================================
--- 2.6-mm.orig/arch/x86_64/kernel/process.c
+++ 2.6-mm/arch/x86_64/kernel/process.c
@@ -584,7 +584,7 @@ static inline void __switch_to_xtra(stru
  *
  * Kprobes not supported here. Set the probe on schedule instead.
  */
-__kprobes struct task_struct *
+struct task_struct *
 __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 {
 	struct thread_struct *prev = &prev_p->thread,






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

* Re: [PATCH][kprobes] support kretprobe-blacklist
  2007-08-17 19:43 [PATCH][kprobes] support kretprobe-blacklist Masami Hiramatsu
@ 2007-08-17 22:03 ` Andrew Morton
  2007-08-17 23:08   ` Masami Hiramatsu
  2007-08-21 21:01   ` [PATCH][kprobes] support kretprobe-blacklist take2 Masami Hiramatsu
  2007-08-27 13:31 ` [PATCH][kprobes] support kretprobe-blacklist Christoph Hellwig
  1 sibling, 2 replies; 7+ messages in thread
From: Andrew Morton @ 2007-08-17 22:03 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: ananth, prasanna, anil.s.keshavamurthy, Jim Keniston, davem,
	linux-kernel

On Fri, 17 Aug 2007 15:43:04 -0400
Masami Hiramatsu <mhiramat@redhat.com> wrote:

> This patch introduces architecture dependent kretprobe
> blacklists to prohibit users from inserting return
> probes on the function in which kprobes can be inserted
> but kretprobes can not.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
> 
> ---
>  <Problem Description>
> When a kretprobe is inserted in the entry of the "__switch_to()",
> it causes kernel panic on i386 with recent kernel.
> 
>  <Problem Analysis>
> In include/asm-i386/current.h, "current" is defined as an entry of
> percpu variables.:
> 
>  DECLARE_PER_CPU(struct task_struct *, current_task);
>  static __always_inline struct task_struct *get_current(void)
>  {
>          return x86_read_percpu(current_task);
>  }
> 
>  #define current get_current()
> 
> This mean the "current" macro is separated from its stack register.
> Kretprobe expects that "current" value when a function is called is
> not changed, or both of "current" value and stack register are changed in
> the target function.
> But __switch_to() in arch/i386/kernel/process.c changes only the value of
> "current", but doesn't changes the stack register(this was already switched
> to new stack before calling __switch_to()):
> 
>  struct task_struct fastcall * __switch_to(struct task_struct *prev_p, struct
>  task_struct *next_p)
>  {
>  	...
>         x86_write_percpu(current_task, next_p);
> 
>         return prev_p;
>  }
> 
> Kretprobe modifies the return address stored in the stack, and
> it saves the original return address in the kretprobe_instance with
> the value of "current".
> When kretprobe is hit at the return of __switch_to(), it searches
> kretprobe_instance related to the probe point from a hash list by
> using the hash-value of the "current" instead of stack address.
> However, since the value of "current" is already changed,
> it can't find proper instance, and invokes BUG() macro.
> 
>  <Solution>
> As a result of discussion with other kprobe developers, I'd
> like to introduce arch-dependent blacklist.
> To introduce "__kretprobes" mark (like "__kprobes") is another way.
> But I thought it is not efficient way, because the kretprobe can
> be inserted only in the entry of functions and there is no need to
> check against a whole function.
> 
> This patch also removes "__kprobes" mark from "__switch_to" on x86_64
> and registers "__switch_to" to the blacklist on x86-64, because that mark
> is to prohibit user from inserting only kretprobe.
> 
> Index: 2.6-mm/include/asm-i386/kprobes.h
> ===================================================================
> --- 2.6-mm.orig/include/asm-i386/kprobes.h
> +++ 2.6-mm/include/asm-i386/kprobes.h
> @@ -44,6 +44,7 @@ typedef u8 kprobe_opcode_t;
> 
>  #define ARCH_SUPPORTS_KRETPROBES
>  #define flush_insn_slot(p)	do { } while (0)
> +#define ARCH_SUPPORTS_KRETPROBE_BLACKLIST

Can we avoid adding this please?

It should at least have been a CONFIG_foo thing, defined in arch/*/Kconfig.

But that still requires nasty ifdefs in the C code.  It would be very small
overhead just to require that all architectures implement
arch_kretprobe_blacklist[] (which can be renamed to kretprobe_blacklist[]).
 Architectures which don't need a blacklist can just have { { 0, 0 } }.

If the few bytes of overhead on non-x86 really offends then one could do
something like this:

in powerpc header file:

#define kretprobe_blacklist_size 0

in x86 header file:

extern const int kretprobe_blacklist_size;

in x86 C file:

const int kretprobe_blacklist_size = ARRAY_SIZE(kretprobe_blacklist);

and then this code:

> --- 2.6-mm.orig/kernel/kprobes.c
> +++ 2.6-mm/kernel/kprobes.c
> @@ -716,6 +716,18 @@ int __kprobes register_kretprobe(struct
>  	int ret = 0;
>  	struct kretprobe_instance *inst;
>  	int i;
> +#ifdef ARCH_SUPPORTS_KRETPROBE_BLACKLIST
> +	void *addr = rp->kp.addr;
> +
> +	if (addr == NULL)
> +		kprobe_lookup_name(rp->kp.symbol_name, addr);
> +	addr += rp->kp.offset;
> +
> +	for (i = 0; arch_kretprobe_blacklist[i].name != NULL; i++) {
> +		if (arch_kretprobe_blacklist[i].addr == addr)
> +			return -EINVAL;
> +	}
> +#endif

can be put inside

	if (kretprobe_blacklist_size) {
		...
	}

so the compiler will remove it all for (say) powerpc.

There are lots of ways of doing it but code like this:

> 
> +#ifdef ARCH_SUPPORTS_KRETPROBE_BLACKLIST
> +	/* lookup the function address from its name */
> +	for (i = 0; arch_kretprobe_blacklist[i].name != NULL; i++) {
> +		kprobe_lookup_name(arch_kretprobe_blacklist[i].name,
> +				   arch_kretprobe_blacklist[i].addr);
> +		if (!arch_kretprobe_blacklist[i].addr)
> +			printk("kretprobe: Unknown blacklisted function: %s\n",
> +			       arch_kretprobe_blacklist[i].name);
> +	}
> +#endif

really isn't the sort of thing we like to see spreading through core kernel
code.

Have a think about it please, see what we can come up with?

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

* Re: [PATCH][kprobes] support kretprobe-blacklist
  2007-08-17 22:03 ` Andrew Morton
@ 2007-08-17 23:08   ` Masami Hiramatsu
  2007-08-21 21:01   ` [PATCH][kprobes] support kretprobe-blacklist take2 Masami Hiramatsu
  1 sibling, 0 replies; 7+ messages in thread
From: Masami Hiramatsu @ 2007-08-17 23:08 UTC (permalink / raw)
  To: Andrew Morton
  Cc: ananth, prasanna, anil.s.keshavamurthy, Jim Keniston, davem,
	linux-kernel

Hi Andrew,

Thank you for comments.

Andrew Morton wrote:
>> Index: 2.6-mm/include/asm-i386/kprobes.h
>> ===================================================================
>> --- 2.6-mm.orig/include/asm-i386/kprobes.h
>> +++ 2.6-mm/include/asm-i386/kprobes.h
>> @@ -44,6 +44,7 @@ typedef u8 kprobe_opcode_t;
>>
>>  #define ARCH_SUPPORTS_KRETPROBES
>>  #define flush_insn_slot(p)	do { } while (0)
>> +#define ARCH_SUPPORTS_KRETPROBE_BLACKLIST
> 
> Can we avoid adding this please?

Yes, sure. I'll update my patch and eliminate those ifdefs.

> It should at least have been a CONFIG_foo thing, defined in arch/*/Kconfig.
> 
> But that still requires nasty ifdefs in the C code.  It would be very small
> overhead just to require that all architectures implement
> arch_kretprobe_blacklist[] (which can be renamed to kretprobe_blacklist[]).
>  Architectures which don't need a blacklist can just have { { 0, 0 } }.
> 
> If the few bytes of overhead on non-x86 really offends then one could do
> something like this:
> 
> in powerpc header file:
> 
> #define kretprobe_blacklist_size 0
> 
> in x86 header file:
> 
> extern const int kretprobe_blacklist_size;
> 
> in x86 C file:
> 
> const int kretprobe_blacklist_size = ARRAY_SIZE(kretprobe_blacklist);

It's looks very nice, thank you for the advice.
I think we can directly define "kretprobe_blacklist" as 0 in (for
example)ppc header instead of introducing "kretprobe_blacklist_size",
and check if "kretprobe_blacklist" is 0 or not in common code, is it OK?

> and then this code:
> 
>> --- 2.6-mm.orig/kernel/kprobes.c
>> +++ 2.6-mm/kernel/kprobes.c
>> @@ -716,6 +716,18 @@ int __kprobes register_kretprobe(struct
>>  	int ret = 0;
>>  	struct kretprobe_instance *inst;
>>  	int i;
>> +#ifdef ARCH_SUPPORTS_KRETPROBE_BLACKLIST
>> +	void *addr = rp->kp.addr;
>> +
>> +	if (addr == NULL)
>> +		kprobe_lookup_name(rp->kp.symbol_name, addr);
>> +	addr += rp->kp.offset;
>> +
>> +	for (i = 0; arch_kretprobe_blacklist[i].name != NULL; i++) {
>> +		if (arch_kretprobe_blacklist[i].addr == addr)
>> +			return -EINVAL;
>> +	}
>> +#endif
> 
> can be put inside
> 
> 	if (kretprobe_blacklist_size) {
> 		...
> 	}
> 
> so the compiler will remove it all for (say) powerpc.
>
> There are lots of ways of doing it but code like this:
> 
>> +#ifdef ARCH_SUPPORTS_KRETPROBE_BLACKLIST
>> +	/* lookup the function address from its name */
>> +	for (i = 0; arch_kretprobe_blacklist[i].name != NULL; i++) {
>> +		kprobe_lookup_name(arch_kretprobe_blacklist[i].name,
>> +				   arch_kretprobe_blacklist[i].addr);
>> +		if (!arch_kretprobe_blacklist[i].addr)
>> +			printk("kretprobe: Unknown blacklisted function: %s\n",
>> +			       arch_kretprobe_blacklist[i].name);
>> +	}
>> +#endif
> 
> really isn't the sort of thing we like to see spreading through core kernel
> code.
> 
> Have a think about it please, see what we can come up with?

OK, I see. I'll do that next time.

Best regards,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com, masami.hiramatsu.pt@hitachi.com
Tel: +1-978-392-2419
Tel: +1-508-982-2642 (cell phone)
Fax: +1-978-392-1001

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

* [PATCH][kprobes] support kretprobe-blacklist take2
  2007-08-17 22:03 ` Andrew Morton
  2007-08-17 23:08   ` Masami Hiramatsu
@ 2007-08-21 21:01   ` Masami Hiramatsu
  2007-08-23  4:12     ` Ananth N Mavinakayanahalli
  1 sibling, 1 reply; 7+ messages in thread
From: Masami Hiramatsu @ 2007-08-21 21:01 UTC (permalink / raw)
  To: Andrew Morton
  Cc: ananth, prasanna, anil.s.keshavamurthy, Jim Keniston, davem,
	linux-kernel

Hi Andrew,

I updated my patch and removed #ifdefs from it.
Thank you,

Masami Hiramatsu
Hitachi Computer Products (America), Inc.


This patch introduces architecture dependent kretprobe
blacklists to prohibit users from inserting return
probes on the function in which kprobes can be inserted
but kretprobes can not.
This patch also removes "__kprobes" mark from "__switch_to" on x86_64
and registers "__switch_to" to the blacklist on x86-64, because that mark
is to prohibit user from inserting only kretprobe.


Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>

---
 arch/avr32/kernel/kprobes.c   |    2 ++
 arch/i386/kernel/kprobes.c    |    7 +++++++
 arch/ia64/kernel/kprobes.c    |    2 ++
 arch/powerpc/kernel/kprobes.c |    2 ++
 arch/s390/kernel/kprobes.c    |    2 ++
 arch/sparc64/kernel/kprobes.c |    2 ++
 arch/x86_64/kernel/kprobes.c  |    7 +++++++
 arch/x86_64/kernel/process.c  |    2 +-
 include/asm-avr32/kprobes.h   |    2 ++
 include/asm-i386/kprobes.h    |    2 ++
 include/asm-ia64/kprobes.h    |    1 +
 include/asm-powerpc/kprobes.h |    1 +
 include/asm-s390/kprobes.h    |    1 +
 include/asm-sparc64/kprobes.h |    2 ++
 include/asm-x86_64/kprobes.h  |    1 +
 include/linux/kprobes.h       |    6 ++++++
 kernel/kprobes.c              |   23 +++++++++++++++++++++++
 17 files changed, 64 insertions(+), 1 deletion(-)

Index: 2.6.23-rc2-mm2/arch/i386/kernel/kprobes.c
===================================================================
--- 2.6.23-rc2-mm2.orig/arch/i386/kernel/kprobes.c
+++ 2.6.23-rc2-mm2/arch/i386/kernel/kprobes.c
@@ -42,6 +42,13 @@ void jprobe_return_end(void);
 DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
 DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);

+struct kretprobe_blackpoint kretprobe_blacklist[] = {
+	{"__switch_to", }, /* This function switches only current task, but
+			     doesn't switch kernel stack.*/
+	{NULL, NULL}	/* Terminator */
+};
+const int kretprobe_blacklist_size = ARRAY_SIZE(kretprobe_blacklist);
+
 /* insert a jmp code */
 static __always_inline void set_jmp_op(void *from, void *to)
 {
Index: 2.6.23-rc2-mm2/kernel/kprobes.c
===================================================================
--- 2.6.23-rc2-mm2.orig/kernel/kprobes.c
+++ 2.6.23-rc2-mm2/kernel/kprobes.c
@@ -716,6 +716,18 @@ int __kprobes register_kretprobe(struct
 	int ret = 0;
 	struct kretprobe_instance *inst;
 	int i;
+	void *addr = rp->kp.addr;
+
+	if (kretprobe_blacklist_size) {
+		if (addr == NULL)
+			kprobe_lookup_name(rp->kp.symbol_name, addr);
+		addr += rp->kp.offset;
+
+		for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
+			if (kretprobe_blacklist[i].addr == addr)
+				return -EINVAL;
+		}
+	}

 	rp->kp.pre_handler = pre_handler_kretprobe;
 	rp->kp.post_handler = NULL;
@@ -794,6 +806,17 @@ static int __init init_kprobes(void)
 		INIT_HLIST_HEAD(&kretprobe_inst_table[i]);
 	}

+	if (kretprobe_blacklist_size) {
+		/* lookup the function address from its name */
+		for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
+			kprobe_lookup_name(kretprobe_blacklist[i].name,
+					   kretprobe_blacklist[i].addr);
+			if (!kretprobe_blacklist[i].addr)
+				printk("kretprobe: lookup failed: %s\n",
+				       kretprobe_blacklist[i].name);
+		}
+	}
+
 	/* By default, kprobes are enabled */
 	kprobe_enabled = true;

Index: 2.6.23-rc2-mm2/arch/x86_64/kernel/kprobes.c
===================================================================
--- 2.6.23-rc2-mm2.orig/arch/x86_64/kernel/kprobes.c
+++ 2.6.23-rc2-mm2/arch/x86_64/kernel/kprobes.c
@@ -49,6 +49,13 @@ static void __kprobes arch_copy_kprobe(s
 DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
 DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);

+struct kretprobe_blackpoint kretprobe_blacklist[] = {
+	{"__switch_to", }, /* This function switches only current task, but
+			      doesn't switch kernel stack.*/
+	{NULL, NULL}	/* Terminator */
+};
+const int kretprobe_blacklist_size = ARRAY_SIZE(kretprobe_blacklist);
+
 /*
  * returns non-zero if opcode modifies the interrupt flag.
  */
Index: 2.6.23-rc2-mm2/include/linux/kprobes.h
===================================================================
--- 2.6.23-rc2-mm2.orig/include/linux/kprobes.h
+++ 2.6.23-rc2-mm2/include/linux/kprobes.h
@@ -166,6 +166,12 @@ struct kretprobe_instance {
 	struct task_struct *task;
 };

+struct kretprobe_blackpoint {
+	const char *name;
+	void *addr;
+};
+extern struct kretprobe_blackpoint kretprobe_blacklist[];
+
 static inline void kretprobe_assert(struct kretprobe_instance *ri,
 	unsigned long orig_ret_address, unsigned long trampoline_address)
 {
Index: 2.6.23-rc2-mm2/arch/x86_64/kernel/process.c
===================================================================
--- 2.6.23-rc2-mm2.orig/arch/x86_64/kernel/process.c
+++ 2.6.23-rc2-mm2/arch/x86_64/kernel/process.c
@@ -584,7 +584,7 @@ static inline void __switch_to_xtra(stru
  *
  * Kprobes not supported here. Set the probe on schedule instead.
  */
-__kprobes struct task_struct *
+struct task_struct *
 __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 {
 	struct thread_struct *prev = &prev_p->thread,
Index: 2.6.23-rc2-mm2/include/asm-ia64/kprobes.h
===================================================================
--- 2.6.23-rc2-mm2.orig/include/asm-ia64/kprobes.h
+++ 2.6.23-rc2-mm2/include/asm-ia64/kprobes.h
@@ -83,6 +83,7 @@ struct kprobe_ctlblk {
 };

 #define ARCH_SUPPORTS_KRETPROBES
+#define kretprobe_blacklist_size 0

 #define SLOT0_OPCODE_SHIFT	(37)
 #define SLOT1_p1_OPCODE_SHIFT	(37 - (64-46))
Index: 2.6.23-rc2-mm2/include/asm-powerpc/kprobes.h
===================================================================
--- 2.6.23-rc2-mm2.orig/include/asm-powerpc/kprobes.h
+++ 2.6.23-rc2-mm2/include/asm-powerpc/kprobes.h
@@ -82,6 +82,7 @@ typedef unsigned int kprobe_opcode_t;

 #define ARCH_SUPPORTS_KRETPROBES
 #define flush_insn_slot(p)	do { } while (0)
+#define kretprobe_blacklist_size 0

 void kretprobe_trampoline(void);
 extern void arch_remove_kprobe(struct kprobe *p);
Index: 2.6.23-rc2-mm2/include/asm-s390/kprobes.h
===================================================================
--- 2.6.23-rc2-mm2.orig/include/asm-s390/kprobes.h
+++ 2.6.23-rc2-mm2/include/asm-s390/kprobes.h
@@ -47,6 +47,7 @@ typedef u16 kprobe_opcode_t;
 	: (((unsigned long)current_thread_info()) + THREAD_SIZE - (ADDR)))

 #define ARCH_SUPPORTS_KRETPROBES
+#define kretprobe_blacklist_size 0

 #define KPROBE_SWAP_INST	0x10

Index: 2.6.23-rc2-mm2/include/asm-avr32/kprobes.h
===================================================================
--- 2.6.23-rc2-mm2.orig/include/asm-avr32/kprobes.h
+++ 2.6.23-rc2-mm2/include/asm-avr32/kprobes.h
@@ -17,6 +17,8 @@ typedef u16	kprobe_opcode_t;
 #define BREAKPOINT_INSTRUCTION	0xd673	/* breakpoint */
 #define MAX_INSN_SIZE		2

+#define kretprobe_blacklist_size 0
+
 #define arch_remove_kprobe(p)	do { } while (0)

 /* Architecture specific copy of original instruction */
Index: 2.6.23-rc2-mm2/include/asm-sparc64/kprobes.h
===================================================================
--- 2.6.23-rc2-mm2.orig/include/asm-sparc64/kprobes.h
+++ 2.6.23-rc2-mm2/include/asm-sparc64/kprobes.h
@@ -10,6 +10,8 @@ typedef u32 kprobe_opcode_t;
 #define BREAKPOINT_INSTRUCTION_2 0x91d02071 /* ta 0x71 */
 #define MAX_INSN_SIZE 2

+#define kretprobe_blacklist_size 0
+
 #define arch_remove_kprobe(p)	do {} while (0)

 #define flush_insn_slot(p)		\
Index: 2.6.23-rc2-mm2/arch/avr32/kernel/kprobes.c
===================================================================
--- 2.6.23-rc2-mm2.orig/arch/avr32/kernel/kprobes.c
+++ 2.6.23-rc2-mm2/arch/avr32/kernel/kprobes.c
@@ -22,6 +22,8 @@ DEFINE_PER_CPU(struct kprobe *, current_
 static unsigned long kprobe_status;
 static struct pt_regs jprobe_saved_regs;

+struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
+
 int __kprobes arch_prepare_kprobe(struct kprobe *p)
 {
 	int ret = 0;
Index: 2.6.23-rc2-mm2/arch/ia64/kernel/kprobes.c
===================================================================
--- 2.6.23-rc2-mm2.orig/arch/ia64/kernel/kprobes.c
+++ 2.6.23-rc2-mm2/arch/ia64/kernel/kprobes.c
@@ -40,6 +40,8 @@ extern void jprobe_inst_return(void);
 DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
 DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);

+struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
+
 enum instruction_type {A, I, M, F, B, L, X, u};
 static enum instruction_type bundle_encoding[32][3] = {
   { M, I, I },				/* 00 */
Index: 2.6.23-rc2-mm2/arch/powerpc/kernel/kprobes.c
===================================================================
--- 2.6.23-rc2-mm2.orig/arch/powerpc/kernel/kprobes.c
+++ 2.6.23-rc2-mm2/arch/powerpc/kernel/kprobes.c
@@ -38,6 +38,8 @@
 DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
 DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);

+struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
+
 int __kprobes arch_prepare_kprobe(struct kprobe *p)
 {
 	int ret = 0;
Index: 2.6.23-rc2-mm2/arch/s390/kernel/kprobes.c
===================================================================
--- 2.6.23-rc2-mm2.orig/arch/s390/kernel/kprobes.c
+++ 2.6.23-rc2-mm2/arch/s390/kernel/kprobes.c
@@ -33,6 +33,8 @@
 DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
 DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);

+struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
+
 int __kprobes arch_prepare_kprobe(struct kprobe *p)
 {
 	/* Make sure the probe isn't going on a difficult instruction */
Index: 2.6.23-rc2-mm2/arch/sparc64/kernel/kprobes.c
===================================================================
--- 2.6.23-rc2-mm2.orig/arch/sparc64/kernel/kprobes.c
+++ 2.6.23-rc2-mm2/arch/sparc64/kernel/kprobes.c
@@ -42,6 +42,8 @@
 DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
 DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);

+struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
+
 int __kprobes arch_prepare_kprobe(struct kprobe *p)
 {
 	p->ainsn.insn[0] = *p->addr;
Index: 2.6.23-rc2-mm2/include/asm-i386/kprobes.h
===================================================================
--- 2.6.23-rc2-mm2.orig/include/asm-i386/kprobes.h
+++ 2.6.23-rc2-mm2/include/asm-i386/kprobes.h
@@ -45,6 +45,8 @@ typedef u8 kprobe_opcode_t;
 #define ARCH_SUPPORTS_KRETPROBES
 #define flush_insn_slot(p)	do { } while (0)

+extern const int kretprobe_blacklist_size;
+
 void arch_remove_kprobe(struct kprobe *p);
 void kretprobe_trampoline(void);

Index: 2.6.23-rc2-mm2/include/asm-x86_64/kprobes.h
===================================================================
--- 2.6.23-rc2-mm2.orig/include/asm-x86_64/kprobes.h
+++ 2.6.23-rc2-mm2/include/asm-x86_64/kprobes.h
@@ -42,6 +42,7 @@ typedef u8 kprobe_opcode_t;
 	: (((unsigned long)current_thread_info()) + THREAD_SIZE - (ADDR)))

 #define ARCH_SUPPORTS_KRETPROBES
+extern const int kretprobe_blacklist_size;

 void kretprobe_trampoline(void);
 extern void arch_remove_kprobe(struct kprobe *p);


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

* Re: [PATCH][kprobes] support kretprobe-blacklist take2
  2007-08-21 21:01   ` [PATCH][kprobes] support kretprobe-blacklist take2 Masami Hiramatsu
@ 2007-08-23  4:12     ` Ananth N Mavinakayanahalli
  0 siblings, 0 replies; 7+ messages in thread
From: Ananth N Mavinakayanahalli @ 2007-08-23  4:12 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Andrew Morton, prasanna, anil.s.keshavamurthy, Jim Keniston,
	davem, linux-kernel

On Tue, Aug 21, 2007 at 05:01:19PM -0400, Masami Hiramatsu wrote:
> Hi Andrew,
> 
> I updated my patch and removed #ifdefs from it.
> Thank you,
> 
> Masami Hiramatsu
> Hitachi Computer Products (America), Inc.
> 
> 
> This patch introduces architecture dependent kretprobe
> blacklists to prohibit users from inserting return
> probes on the function in which kprobes can be inserted
> but kretprobes can not.
> This patch also removes "__kprobes" mark from "__switch_to" on x86_64
> and registers "__switch_to" to the blacklist on x86-64, because that mark
> is to prohibit user from inserting only kretprobe.
> 
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>

Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

> 
> ---
>  arch/avr32/kernel/kprobes.c   |    2 ++
>  arch/i386/kernel/kprobes.c    |    7 +++++++
>  arch/ia64/kernel/kprobes.c    |    2 ++
>  arch/powerpc/kernel/kprobes.c |    2 ++
>  arch/s390/kernel/kprobes.c    |    2 ++
>  arch/sparc64/kernel/kprobes.c |    2 ++
>  arch/x86_64/kernel/kprobes.c  |    7 +++++++
>  arch/x86_64/kernel/process.c  |    2 +-
>  include/asm-avr32/kprobes.h   |    2 ++
>  include/asm-i386/kprobes.h    |    2 ++
>  include/asm-ia64/kprobes.h    |    1 +
>  include/asm-powerpc/kprobes.h |    1 +
>  include/asm-s390/kprobes.h    |    1 +
>  include/asm-sparc64/kprobes.h |    2 ++
>  include/asm-x86_64/kprobes.h  |    1 +
>  include/linux/kprobes.h       |    6 ++++++
>  kernel/kprobes.c              |   23 +++++++++++++++++++++++
>  17 files changed, 64 insertions(+), 1 deletion(-)
> 
> Index: 2.6.23-rc2-mm2/arch/i386/kernel/kprobes.c
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/arch/i386/kernel/kprobes.c
> +++ 2.6.23-rc2-mm2/arch/i386/kernel/kprobes.c
> @@ -42,6 +42,13 @@ void jprobe_return_end(void);
>  DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
>  DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
> 
> +struct kretprobe_blackpoint kretprobe_blacklist[] = {
> +	{"__switch_to", }, /* This function switches only current task, but
> +			     doesn't switch kernel stack.*/
> +	{NULL, NULL}	/* Terminator */
> +};
> +const int kretprobe_blacklist_size = ARRAY_SIZE(kretprobe_blacklist);
> +
>  /* insert a jmp code */
>  static __always_inline void set_jmp_op(void *from, void *to)
>  {
> Index: 2.6.23-rc2-mm2/kernel/kprobes.c
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/kernel/kprobes.c
> +++ 2.6.23-rc2-mm2/kernel/kprobes.c
> @@ -716,6 +716,18 @@ int __kprobes register_kretprobe(struct
>  	int ret = 0;
>  	struct kretprobe_instance *inst;
>  	int i;
> +	void *addr = rp->kp.addr;
> +
> +	if (kretprobe_blacklist_size) {
> +		if (addr == NULL)
> +			kprobe_lookup_name(rp->kp.symbol_name, addr);
> +		addr += rp->kp.offset;
> +
> +		for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
> +			if (kretprobe_blacklist[i].addr == addr)
> +				return -EINVAL;
> +		}
> +	}
> 
>  	rp->kp.pre_handler = pre_handler_kretprobe;
>  	rp->kp.post_handler = NULL;
> @@ -794,6 +806,17 @@ static int __init init_kprobes(void)
>  		INIT_HLIST_HEAD(&kretprobe_inst_table[i]);
>  	}
> 
> +	if (kretprobe_blacklist_size) {
> +		/* lookup the function address from its name */
> +		for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
> +			kprobe_lookup_name(kretprobe_blacklist[i].name,
> +					   kretprobe_blacklist[i].addr);
> +			if (!kretprobe_blacklist[i].addr)
> +				printk("kretprobe: lookup failed: %s\n",
> +				       kretprobe_blacklist[i].name);
> +		}
> +	}
> +
>  	/* By default, kprobes are enabled */
>  	kprobe_enabled = true;
> 
> Index: 2.6.23-rc2-mm2/arch/x86_64/kernel/kprobes.c
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/arch/x86_64/kernel/kprobes.c
> +++ 2.6.23-rc2-mm2/arch/x86_64/kernel/kprobes.c
> @@ -49,6 +49,13 @@ static void __kprobes arch_copy_kprobe(s
>  DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
>  DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
> 
> +struct kretprobe_blackpoint kretprobe_blacklist[] = {
> +	{"__switch_to", }, /* This function switches only current task, but
> +			      doesn't switch kernel stack.*/
> +	{NULL, NULL}	/* Terminator */
> +};
> +const int kretprobe_blacklist_size = ARRAY_SIZE(kretprobe_blacklist);
> +
>  /*
>   * returns non-zero if opcode modifies the interrupt flag.
>   */
> Index: 2.6.23-rc2-mm2/include/linux/kprobes.h
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/include/linux/kprobes.h
> +++ 2.6.23-rc2-mm2/include/linux/kprobes.h
> @@ -166,6 +166,12 @@ struct kretprobe_instance {
>  	struct task_struct *task;
>  };
> 
> +struct kretprobe_blackpoint {
> +	const char *name;
> +	void *addr;
> +};
> +extern struct kretprobe_blackpoint kretprobe_blacklist[];
> +
>  static inline void kretprobe_assert(struct kretprobe_instance *ri,
>  	unsigned long orig_ret_address, unsigned long trampoline_address)
>  {
> Index: 2.6.23-rc2-mm2/arch/x86_64/kernel/process.c
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/arch/x86_64/kernel/process.c
> +++ 2.6.23-rc2-mm2/arch/x86_64/kernel/process.c
> @@ -584,7 +584,7 @@ static inline void __switch_to_xtra(stru
>   *
>   * Kprobes not supported here. Set the probe on schedule instead.
>   */
> -__kprobes struct task_struct *
> +struct task_struct *
>  __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
>  {
>  	struct thread_struct *prev = &prev_p->thread,
> Index: 2.6.23-rc2-mm2/include/asm-ia64/kprobes.h
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/include/asm-ia64/kprobes.h
> +++ 2.6.23-rc2-mm2/include/asm-ia64/kprobes.h
> @@ -83,6 +83,7 @@ struct kprobe_ctlblk {
>  };
> 
>  #define ARCH_SUPPORTS_KRETPROBES
> +#define kretprobe_blacklist_size 0
> 
>  #define SLOT0_OPCODE_SHIFT	(37)
>  #define SLOT1_p1_OPCODE_SHIFT	(37 - (64-46))
> Index: 2.6.23-rc2-mm2/include/asm-powerpc/kprobes.h
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/include/asm-powerpc/kprobes.h
> +++ 2.6.23-rc2-mm2/include/asm-powerpc/kprobes.h
> @@ -82,6 +82,7 @@ typedef unsigned int kprobe_opcode_t;
> 
>  #define ARCH_SUPPORTS_KRETPROBES
>  #define flush_insn_slot(p)	do { } while (0)
> +#define kretprobe_blacklist_size 0
> 
>  void kretprobe_trampoline(void);
>  extern void arch_remove_kprobe(struct kprobe *p);
> Index: 2.6.23-rc2-mm2/include/asm-s390/kprobes.h
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/include/asm-s390/kprobes.h
> +++ 2.6.23-rc2-mm2/include/asm-s390/kprobes.h
> @@ -47,6 +47,7 @@ typedef u16 kprobe_opcode_t;
>  	: (((unsigned long)current_thread_info()) + THREAD_SIZE - (ADDR)))
> 
>  #define ARCH_SUPPORTS_KRETPROBES
> +#define kretprobe_blacklist_size 0
> 
>  #define KPROBE_SWAP_INST	0x10
> 
> Index: 2.6.23-rc2-mm2/include/asm-avr32/kprobes.h
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/include/asm-avr32/kprobes.h
> +++ 2.6.23-rc2-mm2/include/asm-avr32/kprobes.h
> @@ -17,6 +17,8 @@ typedef u16	kprobe_opcode_t;
>  #define BREAKPOINT_INSTRUCTION	0xd673	/* breakpoint */
>  #define MAX_INSN_SIZE		2
> 
> +#define kretprobe_blacklist_size 0
> +
>  #define arch_remove_kprobe(p)	do { } while (0)
> 
>  /* Architecture specific copy of original instruction */
> Index: 2.6.23-rc2-mm2/include/asm-sparc64/kprobes.h
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/include/asm-sparc64/kprobes.h
> +++ 2.6.23-rc2-mm2/include/asm-sparc64/kprobes.h
> @@ -10,6 +10,8 @@ typedef u32 kprobe_opcode_t;
>  #define BREAKPOINT_INSTRUCTION_2 0x91d02071 /* ta 0x71 */
>  #define MAX_INSN_SIZE 2
> 
> +#define kretprobe_blacklist_size 0
> +
>  #define arch_remove_kprobe(p)	do {} while (0)
> 
>  #define flush_insn_slot(p)		\
> Index: 2.6.23-rc2-mm2/arch/avr32/kernel/kprobes.c
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/arch/avr32/kernel/kprobes.c
> +++ 2.6.23-rc2-mm2/arch/avr32/kernel/kprobes.c
> @@ -22,6 +22,8 @@ DEFINE_PER_CPU(struct kprobe *, current_
>  static unsigned long kprobe_status;
>  static struct pt_regs jprobe_saved_regs;
> 
> +struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
> +
>  int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  {
>  	int ret = 0;
> Index: 2.6.23-rc2-mm2/arch/ia64/kernel/kprobes.c
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/arch/ia64/kernel/kprobes.c
> +++ 2.6.23-rc2-mm2/arch/ia64/kernel/kprobes.c
> @@ -40,6 +40,8 @@ extern void jprobe_inst_return(void);
>  DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
>  DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
> 
> +struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
> +
>  enum instruction_type {A, I, M, F, B, L, X, u};
>  static enum instruction_type bundle_encoding[32][3] = {
>    { M, I, I },				/* 00 */
> Index: 2.6.23-rc2-mm2/arch/powerpc/kernel/kprobes.c
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/arch/powerpc/kernel/kprobes.c
> +++ 2.6.23-rc2-mm2/arch/powerpc/kernel/kprobes.c
> @@ -38,6 +38,8 @@
>  DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
>  DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
> 
> +struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
> +
>  int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  {
>  	int ret = 0;
> Index: 2.6.23-rc2-mm2/arch/s390/kernel/kprobes.c
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/arch/s390/kernel/kprobes.c
> +++ 2.6.23-rc2-mm2/arch/s390/kernel/kprobes.c
> @@ -33,6 +33,8 @@
>  DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
>  DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
> 
> +struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
> +
>  int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  {
>  	/* Make sure the probe isn't going on a difficult instruction */
> Index: 2.6.23-rc2-mm2/arch/sparc64/kernel/kprobes.c
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/arch/sparc64/kernel/kprobes.c
> +++ 2.6.23-rc2-mm2/arch/sparc64/kernel/kprobes.c
> @@ -42,6 +42,8 @@
>  DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
>  DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
> 
> +struct kretprobe_blackpoint kretprobe_blacklist[] = {{NULL, NULL}};
> +
>  int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  {
>  	p->ainsn.insn[0] = *p->addr;
> Index: 2.6.23-rc2-mm2/include/asm-i386/kprobes.h
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/include/asm-i386/kprobes.h
> +++ 2.6.23-rc2-mm2/include/asm-i386/kprobes.h
> @@ -45,6 +45,8 @@ typedef u8 kprobe_opcode_t;
>  #define ARCH_SUPPORTS_KRETPROBES
>  #define flush_insn_slot(p)	do { } while (0)
> 
> +extern const int kretprobe_blacklist_size;
> +
>  void arch_remove_kprobe(struct kprobe *p);
>  void kretprobe_trampoline(void);
> 
> Index: 2.6.23-rc2-mm2/include/asm-x86_64/kprobes.h
> ===================================================================
> --- 2.6.23-rc2-mm2.orig/include/asm-x86_64/kprobes.h
> +++ 2.6.23-rc2-mm2/include/asm-x86_64/kprobes.h
> @@ -42,6 +42,7 @@ typedef u8 kprobe_opcode_t;
>  	: (((unsigned long)current_thread_info()) + THREAD_SIZE - (ADDR)))
> 
>  #define ARCH_SUPPORTS_KRETPROBES
> +extern const int kretprobe_blacklist_size;
> 
>  void kretprobe_trampoline(void);
>  extern void arch_remove_kprobe(struct kprobe *p);

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

* Re: [PATCH][kprobes] support kretprobe-blacklist
  2007-08-17 19:43 [PATCH][kprobes] support kretprobe-blacklist Masami Hiramatsu
  2007-08-17 22:03 ` Andrew Morton
@ 2007-08-27 13:31 ` Christoph Hellwig
  2007-08-27 15:49   ` Masami Hiramatsu
  1 sibling, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2007-08-27 13:31 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Andrew Morton, ananth, prasanna, anil.s.keshavamurthy,
	Jim Keniston, davem, linux-kernel

On Fri, Aug 17, 2007 at 03:43:04PM -0400, Masami Hiramatsu wrote:
> This patch introduces architecture dependent kretprobe
> blacklists to prohibit users from inserting return
> probes on the function in which kprobes can be inserted
> but kretprobes can not.

I don't like this at all.  If people want to shoot themselves into
their own foot using kernel modules they have an unlimited number
of ways to do this, and even more with kprobes, so there's no point
in making it a little harder for some cases.


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

* Re: [PATCH][kprobes] support kretprobe-blacklist
  2007-08-27 13:31 ` [PATCH][kprobes] support kretprobe-blacklist Christoph Hellwig
@ 2007-08-27 15:49   ` Masami Hiramatsu
  0 siblings, 0 replies; 7+ messages in thread
From: Masami Hiramatsu @ 2007-08-27 15:49 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Masami Hiramatsu, Andrew Morton, ananth, prasanna,
	anil.s.keshavamurthy, Jim Keniston, davem, linux-kernel,
	systemtap-ml

Hi Christoph,

Christoph Hellwig wrote:
> On Fri, Aug 17, 2007 at 03:43:04PM -0400, Masami Hiramatsu wrote:
>> This patch introduces architecture dependent kretprobe
>> blacklists to prohibit users from inserting return
>> probes on the function in which kprobes can be inserted
>> but kretprobes can not.
> 
> I don't like this at all.  If people want to shoot themselves into
> their own foot using kernel modules they have an unlimited number
> of ways to do this, and even more with kprobes, so there's no point
> in making it a little harder for some cases.

Would you mean that the kernel need not to check whether the probe
point is safe or not?

Actually, there are many ways to shoot themselves from kernel modules.
Even so, IMHO, it is benefit for people that the kernel rejects at least
those obviously (and known) dangerous operation.

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division
e-mail: mhiramat@redhat.com, masami.hiramatsu.pt@hitachi.com

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

end of thread, other threads:[~2007-08-27 16:03 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-08-17 19:43 [PATCH][kprobes] support kretprobe-blacklist Masami Hiramatsu
2007-08-17 22:03 ` Andrew Morton
2007-08-17 23:08   ` Masami Hiramatsu
2007-08-21 21:01   ` [PATCH][kprobes] support kretprobe-blacklist take2 Masami Hiramatsu
2007-08-23  4:12     ` Ananth N Mavinakayanahalli
2007-08-27 13:31 ` [PATCH][kprobes] support kretprobe-blacklist Christoph Hellwig
2007-08-27 15:49   ` Masami Hiramatsu

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