Re: [PATCH][kprobes] support kretprobe-blacklist
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: [EMAIL PROTECTED], [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][kprobes] support kretprobe-blacklist
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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][kprobes] support kretprobe-blacklist
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. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][kprobes] support kretprobe-blacklist take2
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 <[EMAIL PROTECTED]>
Acked-by: Ananth N Mavinakayanahalli <[EMAIL PROTECTED]>
>
> ---
> 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(_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
> @@
Re: [PATCH][kprobes] support kretprobe-blacklist take2
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 [EMAIL PROTECTED]
Acked-by: Ananth N Mavinakayanahalli [EMAIL PROTECTED]
---
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
[PATCH][kprobes] support kretprobe-blacklist take2
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 <[EMAIL PROTECTED]>
---
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(_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,
[PATCH][kprobes] support kretprobe-blacklist take2
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 [EMAIL PROTECTED]
---
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,
Re: [PATCH][kprobes] support kretprobe-blacklist
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: [EMAIL PROTECTED], [EMAIL PROTECTED]
Tel: +1-978-392-2419
Tel: +1-508-982-2642 (cell phone)
Fax: +1-978-392-1001
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][kprobes] support kretprobe-blacklist
On Fri, 17 Aug 2007 15:43:04 -0400
Masami Hiramatsu <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
>
> ---
>
> When a kretprobe is inserted in the entry of the "__switch_to()",
> it causes kernel panic on i386 with recent kernel.
>
>
> 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.
>
>
> 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.
[PATCH][kprobes] support kretprobe-blacklist
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 <[EMAIL PROTECTED]>
---
When a kretprobe is inserted in the entry of the "__switch_to()",
it causes kernel panic on i386 with recent kernel.
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.
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(_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,
+
[PATCH][kprobes] support kretprobe-blacklist
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 [EMAIL PROTECTED]
---
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,
+
Re: [PATCH][kprobes] support kretprobe-blacklist
On Fri, 17 Aug 2007 15:43:04 -0400
Masami Hiramatsu [EMAIL PROTECTED] 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 [EMAIL PROTECTED]
---
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?
-
To unsubscribe from this list:
Re: [PATCH][kprobes] support kretprobe-blacklist
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: [EMAIL PROTECTED], [EMAIL PROTECTED]
Tel: +1-978-392-2419
Tel: +1-508-982-2642 (cell phone)
Fax: +1-978-392-1001
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
