These interrupt functions are already non-attachable by kprobes.
Blacklist them explicitly so that they can show up in
/sys/kernel/debug/kprobes/blacklist and tools like BCC can use this
additional information.
Signed-off-by: Andrea Righi <[email protected]>
---
arch/x86/entry/entry_64.S | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index ce25d84023c0..1f0efdb7b629 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -566,6 +566,7 @@ ENTRY(interrupt_entry)
ret
END(interrupt_entry)
+_ASM_NOKPROBE(interrupt_entry)
/* Interrupt entry/exit. */
@@ -766,6 +767,7 @@ native_irq_return_ldt:
jmp native_irq_return_iret
#endif
END(common_interrupt)
+_ASM_NOKPROBE(common_interrupt)
/*
* APIC interrupts.
@@ -780,6 +782,7 @@ ENTRY(\sym)
call \do_sym /* rdi points to pt_regs */
jmp ret_from_intr
END(\sym)
+_ASM_NOKPROBE(\sym)
.endm
/* Make sure APIC interrupt handlers end up in the irqentry section: */
@@ -960,6 +963,7 @@ ENTRY(\sym)
jmp error_exit
.endif
+_ASM_NOKPROBE(\sym)
END(\sym)
.endm
--
2.17.1
Commit-ID: a50480cb6d61d5c5fc13308479407b628b6bc1c5
Gitweb: https://git.kernel.org/tip/a50480cb6d61d5c5fc13308479407b628b6bc1c5
Author: Andrea Righi <[email protected]>
AuthorDate: Thu, 6 Dec 2018 10:56:48 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 6 Dec 2018 16:52:03 +0100
kprobes/x86: Blacklist non-attachable interrupt functions
These interrupt functions are already non-attachable by kprobes.
Blacklist them explicitly so that they can show up in
/sys/kernel/debug/kprobes/blacklist and tools like BCC can use this
additional information.
Signed-off-by: Andrea Righi <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Anil S Keshavamurthy <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Naveen N. Rao <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Yonghong Song <[email protected]>
Link: http://lkml.kernel.org/r/20181206095648.GA8249@Dell
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/entry/entry_64.S | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index ce25d84023c0..1f0efdb7b629 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -566,6 +566,7 @@ ENTRY(interrupt_entry)
ret
END(interrupt_entry)
+_ASM_NOKPROBE(interrupt_entry)
/* Interrupt entry/exit. */
@@ -766,6 +767,7 @@ native_irq_return_ldt:
jmp native_irq_return_iret
#endif
END(common_interrupt)
+_ASM_NOKPROBE(common_interrupt)
/*
* APIC interrupts.
@@ -780,6 +782,7 @@ ENTRY(\sym)
call \do_sym /* rdi points to pt_regs */
jmp ret_from_intr
END(\sym)
+_ASM_NOKPROBE(\sym)
.endm
/* Make sure APIC interrupt handlers end up in the irqentry section: */
@@ -960,6 +963,7 @@ ENTRY(\sym)
jmp error_exit
.endif
+_ASM_NOKPROBE(\sym)
END(\sym)
.endm
On Thu, 6 Dec 2018 10:56:48 +0100
Andrea Righi <[email protected]> wrote:
> These interrupt functions are already non-attachable by kprobes.
> Blacklist them explicitly so that they can show up in
> /sys/kernel/debug/kprobes/blacklist and tools like BCC can use this
> additional information.
>
> Signed-off-by: Andrea Righi <[email protected]>
For the short term, this is OK. But this is not the best way to cover
other functions which are also prohibited by arch_within_kprobe_blacklist().
I'll fix this issue later.
Acked-by: Masami Hiramatsu <[email protected]>
Thank you,
> ---
> arch/x86/entry/entry_64.S | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index ce25d84023c0..1f0efdb7b629 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -566,6 +566,7 @@ ENTRY(interrupt_entry)
>
> ret
> END(interrupt_entry)
> +_ASM_NOKPROBE(interrupt_entry)
>
>
> /* Interrupt entry/exit. */
> @@ -766,6 +767,7 @@ native_irq_return_ldt:
> jmp native_irq_return_iret
> #endif
> END(common_interrupt)
> +_ASM_NOKPROBE(common_interrupt)
>
> /*
> * APIC interrupts.
> @@ -780,6 +782,7 @@ ENTRY(\sym)
> call \do_sym /* rdi points to pt_regs */
> jmp ret_from_intr
> END(\sym)
> +_ASM_NOKPROBE(\sym)
> .endm
>
> /* Make sure APIC interrupt handlers end up in the irqentry section: */
> @@ -960,6 +963,7 @@ ENTRY(\sym)
>
> jmp error_exit
> .endif
> +_ASM_NOKPROBE(\sym)
> END(\sym)
> .endm
>
> --
> 2.17.1
--
Masami Hiramatsu <[email protected]>
Hi Andrea and Ingo,
Here is the patch what I meant. I just ran it on qemu-x86, and seemed working.
After introducing this patch, I will start adding arch_populate_kprobe_blacklist()
to some arches.
Thank you,
[RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area
From: Masami Hiramatsu <[email protected]>
Blacklist symbols in arch-defined probe-prohibited areas.
With this change, user can see all symbols which are prohibited
to probe in debugfs.
All archtectures which have custom prohibit areas should define
its own arch_populate_kprobe_blacklist() function, but unless that,
all symbols marked __kprobes are blacklisted.
Reported-by: Andrea Righi <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
---
arch/x86/kernel/kprobes/core.c | 27 ++++++++++++++++++
include/linux/kprobes.h | 2 +
kernel/kprobes.c | 60 +++++++++++++++++++++++++++++++---------
3 files changed, 75 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index c33b06f..52771e3 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -1034,6 +1034,33 @@ bool arch_within_kprobe_blacklist(unsigned long addr)
addr < (unsigned long)__entry_text_end);
}
+int __init arch_populate_kprobe_blacklist(void)
+{
+ unsigned long entry;
+ int ret = 0;
+
+ for (entry = (unsigned long)__kprobes_text_start;
+ entry < (unsigned long)__kprobes_text_end;
+ entry += ret) {
+ ret = kprobe_add_ksym_blacklist(entry);
+ if (ret < 0)
+ return ret;
+ if (ret == 0) /* In case of alias symbol */
+ ret = 1;
+ }
+
+ for (entry = (unsigned long)__entry_text_start;
+ entry < (unsigned long)__entry_text_end;
+ entry += ret) {
+ ret = kprobe_add_ksym_blacklist(entry);
+ if (ret < 0)
+ return ret;
+ if (ret == 0) /* In case of alias symbol */
+ ret = 1;
+ }
+ return 0;
+}
+
int __init arch_init_kprobes(void)
{
return 0;
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index e909413..a68cbbe 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -242,10 +242,12 @@ extern int arch_init_kprobes(void);
extern void show_registers(struct pt_regs *regs);
extern void kprobes_inc_nmissed_count(struct kprobe *p);
extern bool arch_within_kprobe_blacklist(unsigned long addr);
+extern int arch_populate_kprobe_blacklist(void);
extern bool arch_kprobe_on_func_entry(unsigned long offset);
extern bool kprobe_on_func_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset);
extern bool within_kprobe_blacklist(unsigned long addr);
+extern int kprobe_add_ksym_blacklist(unsigned long entry);
struct kprobe_insn_cache {
struct mutex mutex;
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 90e98e2..8b2eb62 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -2093,6 +2093,44 @@ void dump_kprobe(struct kprobe *kp)
}
NOKPROBE_SYMBOL(dump_kprobe);
+int kprobe_add_ksym_blacklist(unsigned long entry)
+{
+ struct kprobe_blacklist_entry *ent;
+ unsigned long offset = 0, size = 0;
+
+ if (!kernel_text_address(entry) ||
+ !kallsyms_lookup_size_offset(entry, &size, &offset))
+ return -EINVAL;
+
+ ent = kmalloc(sizeof(*ent), GFP_KERNEL);
+ if (!ent)
+ return -ENOMEM;
+ ent->start_addr = entry - offset;
+ ent->end_addr = entry - offset + size;
+ INIT_LIST_HEAD(&ent->list);
+ list_add_tail(&ent->list, &kprobe_blacklist);
+
+ return (int)size;
+}
+
+/* Add functions in arch defined probe-prohibited area */
+int __weak arch_populate_kprobe_blacklist(void)
+{
+ unsigned long entry;
+ int ret = 0;
+
+ for (entry = (unsigned long)__kprobes_text_start;
+ entry < (unsigned long)__kprobes_text_end;
+ entry += ret) {
+ ret = kprobe_add_ksym_blacklist(entry);
+ if (ret < 0)
+ return ret;
+ if (ret == 0) /* In case of alias symbol */
+ ret = 1;
+ }
+ return 0;
+}
+
/*
* Lookup and populate the kprobe_blacklist.
*
@@ -2104,26 +2142,20 @@ NOKPROBE_SYMBOL(dump_kprobe);
static int __init populate_kprobe_blacklist(unsigned long *start,
unsigned long *end)
{
+ unsigned long entry;
unsigned long *iter;
- struct kprobe_blacklist_entry *ent;
- unsigned long entry, offset = 0, size = 0;
+ int ret;
for (iter = start; iter < end; iter++) {
entry = arch_deref_entry_point((void *)*iter);
-
- if (!kernel_text_address(entry) ||
- !kallsyms_lookup_size_offset(entry, &size, &offset))
+ ret = kprobe_add_ksym_blacklist(entry);
+ if (ret == -EINVAL)
continue;
-
- ent = kmalloc(sizeof(*ent), GFP_KERNEL);
- if (!ent)
- return -ENOMEM;
- ent->start_addr = entry;
- ent->end_addr = entry + size;
- INIT_LIST_HEAD(&ent->list);
- list_add_tail(&ent->list, &kprobe_blacklist);
+ if (ret < 0)
+ return ret;
}
- return 0;
+
+ return arch_populate_kprobe_blacklist();
}
/* Module notifier call back, checking kprobes on the module */
On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote:
> Hi Andrea and Ingo,
>
> Here is the patch what I meant. I just ran it on qemu-x86, and seemed working.
> After introducing this patch, I will start adding arch_populate_kprobe_blacklist()
> to some arches.
>
> Thank you,
>
> [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area
>
> From: Masami Hiramatsu <[email protected]>
>
> Blacklist symbols in arch-defined probe-prohibited areas.
> With this change, user can see all symbols which are prohibited
> to probe in debugfs.
>
> All archtectures which have custom prohibit areas should define
> its own arch_populate_kprobe_blacklist() function, but unless that,
> all symbols marked __kprobes are blacklisted.
>
> Reported-by: Andrea Righi <[email protected]>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> ---
[snip]
> +int kprobe_add_ksym_blacklist(unsigned long entry)
> +{
> + struct kprobe_blacklist_entry *ent;
> + unsigned long offset = 0, size = 0;
> +
> + if (!kernel_text_address(entry) ||
> + !kallsyms_lookup_size_offset(entry, &size, &offset))
> + return -EINVAL;
> +
> + ent = kmalloc(sizeof(*ent), GFP_KERNEL);
> + if (!ent)
> + return -ENOMEM;
> + ent->start_addr = entry - offset;
> + ent->end_addr = entry - offset + size;
Do we need to take offset into account? The code before wasn't using it.
> + INIT_LIST_HEAD(&ent->list);
> + list_add_tail(&ent->list, &kprobe_blacklist);
> +
> + return (int)size;
> +}
> +
> +/* Add functions in arch defined probe-prohibited area */
> +int __weak arch_populate_kprobe_blacklist(void)
> +{
> + unsigned long entry;
> + int ret = 0;
> +
> + for (entry = (unsigned long)__kprobes_text_start;
> + entry < (unsigned long)__kprobes_text_end;
> + entry += ret) {
> + ret = kprobe_add_ksym_blacklist(entry);
> + if (ret < 0)
> + return ret;
> + if (ret == 0) /* In case of alias symbol */
> + ret = 1;
> + }
> + return 0;
> +}
> +
> /*
> * Lookup and populate the kprobe_blacklist.
> *
> @@ -2104,26 +2142,20 @@ NOKPROBE_SYMBOL(dump_kprobe);
> static int __init populate_kprobe_blacklist(unsigned long *start,
> unsigned long *end)
> {
> + unsigned long entry;
> unsigned long *iter;
> - struct kprobe_blacklist_entry *ent;
> - unsigned long entry, offset = 0, size = 0;
> + int ret;
>
> for (iter = start; iter < end; iter++) {
> entry = arch_deref_entry_point((void *)*iter);
> -
> - if (!kernel_text_address(entry) ||
> - !kallsyms_lookup_size_offset(entry, &size, &offset))
> + ret = kprobe_add_ksym_blacklist(entry);
> + if (ret == -EINVAL)
> continue;
> -
> - ent = kmalloc(sizeof(*ent), GFP_KERNEL);
> - if (!ent)
> - return -ENOMEM;
> - ent->start_addr = entry;
> - ent->end_addr = entry + size;
> - INIT_LIST_HEAD(&ent->list);
> - list_add_tail(&ent->list, &kprobe_blacklist);
> + if (ret < 0)
> + return ret;
> }
> - return 0;
> +
> + return arch_populate_kprobe_blacklist();
> }
>
> /* Module notifier call back, checking kprobes on the module */
On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote:
> Hi Andrea and Ingo,
>
> Here is the patch what I meant. I just ran it on qemu-x86, and seemed working.
> After introducing this patch, I will start adding arch_populate_kprobe_blacklist()
> to some arches.
>
> Thank you,
>
> [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area
>
> From: Masami Hiramatsu <[email protected]>
>
> Blacklist symbols in arch-defined probe-prohibited areas.
> With this change, user can see all symbols which are prohibited
> to probe in debugfs.
>
> All archtectures which have custom prohibit areas should define
> its own arch_populate_kprobe_blacklist() function, but unless that,
> all symbols marked __kprobes are blacklisted.
What about iterating all symbols and use arch_within_kprobe_blacklist()
to check if we need to blacklist them or not.
In this way we don't have to introduce an
arch_populate_kprobe_blacklist() for each architecture.
Something like the following maybe.
Thanks.
[RFC] kprobes: blacklist all symbols in arch-defined prohibited area
From: Andrea Righi <[email protected]>
Blacklist symbols in arch-defined probe-prohibited areas.
With this change, user can see all symbols which are prohibited
to probe in debugfs.
Signed-off-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Andrea Righi <[email protected]>
---
kernel/kprobes.c | 55 +++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 41 insertions(+), 14 deletions(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 90e98e233647..e67598dd7468 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -2093,6 +2093,35 @@ void dump_kprobe(struct kprobe *kp)
}
NOKPROBE_SYMBOL(dump_kprobe);
+static int kprobe_blacklist_add(unsigned long entry)
+{
+ struct kprobe_blacklist_entry *ent;
+ unsigned long offset = 0, size = 0;
+
+ if (!kernel_text_address(entry) ||
+ !kallsyms_lookup_size_offset(entry, &size, &offset))
+ return -EINVAL;
+
+ ent = kmalloc(sizeof(*ent), GFP_KERNEL);
+ if (!ent)
+ return -ENOMEM;
+ ent->start_addr = entry;
+ ent->end_addr = entry + size;
+ INIT_LIST_HEAD(&ent->list);
+ list_add_tail(&ent->list, &kprobe_blacklist);
+
+ return 0;
+}
+
+static int arch_populate_kprobe_blacklist(void *data, const char *name,
+ struct module *mod,
+ unsigned long entry)
+{
+ if (arch_within_kprobe_blacklist(entry))
+ kprobe_blacklist_add(entry);
+ return 0;
+}
+
/*
* Lookup and populate the kprobe_blacklist.
*
@@ -2104,24 +2133,22 @@ NOKPROBE_SYMBOL(dump_kprobe);
static int __init populate_kprobe_blacklist(unsigned long *start,
unsigned long *end)
{
- unsigned long *iter;
- struct kprobe_blacklist_entry *ent;
- unsigned long entry, offset = 0, size = 0;
+ unsigned long entry, *iter;
+ int ret;
+ /* Blacklist all arch_within_kprobe_blacklist() symbols */
+ mutex_lock(&module_mutex);
+ kallsyms_on_each_symbol(arch_populate_kprobe_blacklist, NULL);
+ mutex_unlock(&module_mutex);
+
+ /* Add explicitly blacklisted symbols */
for (iter = start; iter < end; iter++) {
entry = arch_deref_entry_point((void *)*iter);
-
- if (!kernel_text_address(entry) ||
- !kallsyms_lookup_size_offset(entry, &size, &offset))
+ ret = kprobe_blacklist_add(entry);
+ if (ret == -EINVAL)
continue;
-
- ent = kmalloc(sizeof(*ent), GFP_KERNEL);
- if (!ent)
- return -ENOMEM;
- ent->start_addr = entry;
- ent->end_addr = entry + size;
- INIT_LIST_HEAD(&ent->list);
- list_add_tail(&ent->list, &kprobe_blacklist);
+ if (ret < 0)
+ return ret;
}
return 0;
}
On Fri, 7 Dec 2018 18:00:26 +0100
Andrea Righi <[email protected]> wrote:
> On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote:
> > Hi Andrea and Ingo,
> >
> > Here is the patch what I meant. I just ran it on qemu-x86, and seemed working.
> > After introducing this patch, I will start adding arch_populate_kprobe_blacklist()
> > to some arches.
> >
> > Thank you,
> >
> > [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area
> >
> > From: Masami Hiramatsu <[email protected]>
> >
> > Blacklist symbols in arch-defined probe-prohibited areas.
> > With this change, user can see all symbols which are prohibited
> > to probe in debugfs.
> >
> > All archtectures which have custom prohibit areas should define
> > its own arch_populate_kprobe_blacklist() function, but unless that,
> > all symbols marked __kprobes are blacklisted.
> >
> > Reported-by: Andrea Righi <[email protected]>
> > Signed-off-by: Masami Hiramatsu <[email protected]>
> > ---
>
> [snip]
>
> > +int kprobe_add_ksym_blacklist(unsigned long entry)
> > +{
> > + struct kprobe_blacklist_entry *ent;
> > + unsigned long offset = 0, size = 0;
> > +
> > + if (!kernel_text_address(entry) ||
> > + !kallsyms_lookup_size_offset(entry, &size, &offset))
> > + return -EINVAL;
> > +
> > + ent = kmalloc(sizeof(*ent), GFP_KERNEL);
> > + if (!ent)
> > + return -ENOMEM;
> > + ent->start_addr = entry - offset;
> > + ent->end_addr = entry - offset + size;
>
> Do we need to take offset into account? The code before wasn't using it.
Yes, if we hit an alias symbol (zero-size), we forcibly increment address
and retry it. In that case, offset will be 1.
>
> > + INIT_LIST_HEAD(&ent->list);
> > + list_add_tail(&ent->list, &kprobe_blacklist);
> > +
> > + return (int)size;
> > +}
> > +
> > +/* Add functions in arch defined probe-prohibited area */
> > +int __weak arch_populate_kprobe_blacklist(void)
> > +{
> > + unsigned long entry;
> > + int ret = 0;
> > +
> > + for (entry = (unsigned long)__kprobes_text_start;
> > + entry < (unsigned long)__kprobes_text_end;
> > + entry += ret) {
> > + ret = kprobe_add_ksym_blacklist(entry);
> > + if (ret < 0)
> > + return ret;
> > + if (ret == 0) /* In case of alias symbol */
> > + ret = 1;
Here, we incremented.
Thank you,
> > + }
> > + return 0;
> > +}
> > +
> > /*
> > * Lookup and populate the kprobe_blacklist.
> > *
> > @@ -2104,26 +2142,20 @@ NOKPROBE_SYMBOL(dump_kprobe);
> > static int __init populate_kprobe_blacklist(unsigned long *start,
> > unsigned long *end)
> > {
> > + unsigned long entry;
> > unsigned long *iter;
> > - struct kprobe_blacklist_entry *ent;
> > - unsigned long entry, offset = 0, size = 0;
> > + int ret;
> >
> > for (iter = start; iter < end; iter++) {
> > entry = arch_deref_entry_point((void *)*iter);
> > -
> > - if (!kernel_text_address(entry) ||
> > - !kallsyms_lookup_size_offset(entry, &size, &offset))
> > + ret = kprobe_add_ksym_blacklist(entry);
> > + if (ret == -EINVAL)
> > continue;
> > -
> > - ent = kmalloc(sizeof(*ent), GFP_KERNEL);
> > - if (!ent)
> > - return -ENOMEM;
> > - ent->start_addr = entry;
> > - ent->end_addr = entry + size;
> > - INIT_LIST_HEAD(&ent->list);
> > - list_add_tail(&ent->list, &kprobe_blacklist);
> > + if (ret < 0)
> > + return ret;
> > }
> > - return 0;
> > +
> > + return arch_populate_kprobe_blacklist();
> > }
> >
> > /* Module notifier call back, checking kprobes on the module */
--
Masami Hiramatsu <[email protected]>
On Fri, 7 Dec 2018 18:58:05 +0100
Andrea Righi <[email protected]> wrote:
> On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote:
> > Hi Andrea and Ingo,
> >
> > Here is the patch what I meant. I just ran it on qemu-x86, and seemed working.
> > After introducing this patch, I will start adding arch_populate_kprobe_blacklist()
> > to some arches.
> >
> > Thank you,
> >
> > [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area
> >
> > From: Masami Hiramatsu <[email protected]>
> >
> > Blacklist symbols in arch-defined probe-prohibited areas.
> > With this change, user can see all symbols which are prohibited
> > to probe in debugfs.
> >
> > All archtectures which have custom prohibit areas should define
> > its own arch_populate_kprobe_blacklist() function, but unless that,
> > all symbols marked __kprobes are blacklisted.
>
> What about iterating all symbols and use arch_within_kprobe_blacklist()
> to check if we need to blacklist them or not.
Sorry, I don't want to iterate all ksyms since it may take a long time
(especially embedded small devices.)
>
> In this way we don't have to introduce an
> arch_populate_kprobe_blacklist() for each architecture.
Hmm, I had a same idea, but there are some arch which prohibit probing
extable entries (e.g. arm64.) For correctness of the blacklist, I think
it should be listed (not entire the function body).
I also rather like to remove arch_within_kprobe_blacklist() instead.
Thank you,
>
> Something like the following maybe.
>
> Thanks.
>
> [RFC] kprobes: blacklist all symbols in arch-defined prohibited area
>
> From: Andrea Righi <[email protected]>
>
> Blacklist symbols in arch-defined probe-prohibited areas.
> With this change, user can see all symbols which are prohibited
> to probe in debugfs.
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Signed-off-by: Andrea Righi <[email protected]>
> ---
> kernel/kprobes.c | 55 +++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 41 insertions(+), 14 deletions(-)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 90e98e233647..e67598dd7468 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -2093,6 +2093,35 @@ void dump_kprobe(struct kprobe *kp)
> }
> NOKPROBE_SYMBOL(dump_kprobe);
>
> +static int kprobe_blacklist_add(unsigned long entry)
> +{
> + struct kprobe_blacklist_entry *ent;
> + unsigned long offset = 0, size = 0;
> +
> + if (!kernel_text_address(entry) ||
> + !kallsyms_lookup_size_offset(entry, &size, &offset))
> + return -EINVAL;
> +
> + ent = kmalloc(sizeof(*ent), GFP_KERNEL);
> + if (!ent)
> + return -ENOMEM;
> + ent->start_addr = entry;
> + ent->end_addr = entry + size;
> + INIT_LIST_HEAD(&ent->list);
> + list_add_tail(&ent->list, &kprobe_blacklist);
> +
> + return 0;
> +}
> +
> +static int arch_populate_kprobe_blacklist(void *data, const char *name,
> + struct module *mod,
> + unsigned long entry)
> +{
> + if (arch_within_kprobe_blacklist(entry))
> + kprobe_blacklist_add(entry);
> + return 0;
> +}
> +
> /*
> * Lookup and populate the kprobe_blacklist.
> *
> @@ -2104,24 +2133,22 @@ NOKPROBE_SYMBOL(dump_kprobe);
> static int __init populate_kprobe_blacklist(unsigned long *start,
> unsigned long *end)
> {
> - unsigned long *iter;
> - struct kprobe_blacklist_entry *ent;
> - unsigned long entry, offset = 0, size = 0;
> + unsigned long entry, *iter;
> + int ret;
>
> + /* Blacklist all arch_within_kprobe_blacklist() symbols */
> + mutex_lock(&module_mutex);
> + kallsyms_on_each_symbol(arch_populate_kprobe_blacklist, NULL);
> + mutex_unlock(&module_mutex);
> +
> + /* Add explicitly blacklisted symbols */
> for (iter = start; iter < end; iter++) {
> entry = arch_deref_entry_point((void *)*iter);
> -
> - if (!kernel_text_address(entry) ||
> - !kallsyms_lookup_size_offset(entry, &size, &offset))
> + ret = kprobe_blacklist_add(entry);
> + if (ret == -EINVAL)
> continue;
> -
> - ent = kmalloc(sizeof(*ent), GFP_KERNEL);
> - if (!ent)
> - return -ENOMEM;
> - ent->start_addr = entry;
> - ent->end_addr = entry + size;
> - INIT_LIST_HEAD(&ent->list);
> - list_add_tail(&ent->list, &kprobe_blacklist);
> + if (ret < 0)
> + return ret;
> }
> return 0;
> }
--
Masami Hiramatsu <[email protected]>
On Sat, Dec 08, 2018 at 12:42:10PM +0900, Masami Hiramatsu wrote:
> On Fri, 7 Dec 2018 18:00:26 +0100
> Andrea Righi <[email protected]> wrote:
>
> > On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote:
> > > Hi Andrea and Ingo,
> > >
> > > Here is the patch what I meant. I just ran it on qemu-x86, and seemed working.
> > > After introducing this patch, I will start adding arch_populate_kprobe_blacklist()
> > > to some arches.
> > >
> > > Thank you,
> > >
> > > [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area
> > >
> > > From: Masami Hiramatsu <[email protected]>
> > >
> > > Blacklist symbols in arch-defined probe-prohibited areas.
> > > With this change, user can see all symbols which are prohibited
> > > to probe in debugfs.
> > >
> > > All archtectures which have custom prohibit areas should define
> > > its own arch_populate_kprobe_blacklist() function, but unless that,
> > > all symbols marked __kprobes are blacklisted.
> > >
> > > Reported-by: Andrea Righi <[email protected]>
> > > Signed-off-by: Masami Hiramatsu <[email protected]>
> > > ---
> >
> > [snip]
> >
> > > +int kprobe_add_ksym_blacklist(unsigned long entry)
> > > +{
> > > + struct kprobe_blacklist_entry *ent;
> > > + unsigned long offset = 0, size = 0;
> > > +
> > > + if (!kernel_text_address(entry) ||
> > > + !kallsyms_lookup_size_offset(entry, &size, &offset))
> > > + return -EINVAL;
> > > +
> > > + ent = kmalloc(sizeof(*ent), GFP_KERNEL);
> > > + if (!ent)
> > > + return -ENOMEM;
> > > + ent->start_addr = entry - offset;
> > > + ent->end_addr = entry - offset + size;
> >
> > Do we need to take offset into account? The code before wasn't using it.
>
> Yes, if we hit an alias symbol (zero-size), we forcibly increment address
> and retry it. In that case, offset will be 1.
>
> >
> > > + INIT_LIST_HEAD(&ent->list);
> > > + list_add_tail(&ent->list, &kprobe_blacklist);
> > > +
> > > + return (int)size;
> > > +}
> > > +
> > > +/* Add functions in arch defined probe-prohibited area */
> > > +int __weak arch_populate_kprobe_blacklist(void)
> > > +{
> > > + unsigned long entry;
> > > + int ret = 0;
> > > +
> > > + for (entry = (unsigned long)__kprobes_text_start;
> > > + entry < (unsigned long)__kprobes_text_end;
> > > + entry += ret) {
> > > + ret = kprobe_add_ksym_blacklist(entry);
> > > + if (ret < 0)
> > > + return ret;
> > > + if (ret == 0) /* In case of alias symbol */
> > > + ret = 1;
>
> Here, we incremented.
>
> Thank you,
Makes sense, thanks for the clarification.
-Andrea
On Sat, Dec 08, 2018 at 12:48:59PM +0900, Masami Hiramatsu wrote:
> On Fri, 7 Dec 2018 18:58:05 +0100
> Andrea Righi <[email protected]> wrote:
>
> > On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote:
> > > Hi Andrea and Ingo,
> > >
> > > Here is the patch what I meant. I just ran it on qemu-x86, and seemed working.
> > > After introducing this patch, I will start adding arch_populate_kprobe_blacklist()
> > > to some arches.
> > >
> > > Thank you,
> > >
> > > [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area
> > >
> > > From: Masami Hiramatsu <[email protected]>
> > >
> > > Blacklist symbols in arch-defined probe-prohibited areas.
> > > With this change, user can see all symbols which are prohibited
> > > to probe in debugfs.
> > >
> > > All archtectures which have custom prohibit areas should define
> > > its own arch_populate_kprobe_blacklist() function, but unless that,
> > > all symbols marked __kprobes are blacklisted.
> >
> > What about iterating all symbols and use arch_within_kprobe_blacklist()
> > to check if we need to blacklist them or not.
>
> Sorry, I don't want to iterate all ksyms since it may take a long time
> (especially embedded small devices.)
>
> >
> > In this way we don't have to introduce an
> > arch_populate_kprobe_blacklist() for each architecture.
>
> Hmm, I had a same idea, but there are some arch which prohibit probing
> extable entries (e.g. arm64.) For correctness of the blacklist, I think
> it should be listed (not entire the function body).
> I also rather like to remove arch_within_kprobe_blacklist() instead.
OK. Thanks.
-Andrea
On Sat, 8 Dec 2018 08:09:22 +0100
Andrea Righi <[email protected]> wrote:
> On Sat, Dec 08, 2018 at 12:48:59PM +0900, Masami Hiramatsu wrote:
> > On Fri, 7 Dec 2018 18:58:05 +0100
> > Andrea Righi <[email protected]> wrote:
> >
> > > On Sat, Dec 08, 2018 at 01:01:20AM +0900, Masami Hiramatsu wrote:
> > > > Hi Andrea and Ingo,
> > > >
> > > > Here is the patch what I meant. I just ran it on qemu-x86, and seemed working.
> > > > After introducing this patch, I will start adding arch_populate_kprobe_blacklist()
> > > > to some arches.
> > > >
> > > > Thank you,
> > > >
> > > > [RFC] kprobes: x86/kprobes: Blacklist symbols in arch-defined prohibited area
> > > >
> > > > From: Masami Hiramatsu <[email protected]>
> > > >
> > > > Blacklist symbols in arch-defined probe-prohibited areas.
> > > > With this change, user can see all symbols which are prohibited
> > > > to probe in debugfs.
> > > >
> > > > All archtectures which have custom prohibit areas should define
> > > > its own arch_populate_kprobe_blacklist() function, but unless that,
> > > > all symbols marked __kprobes are blacklisted.
> > >
> > > What about iterating all symbols and use arch_within_kprobe_blacklist()
> > > to check if we need to blacklist them or not.
> >
> > Sorry, I don't want to iterate all ksyms since it may take a long time
> > (especially embedded small devices.)
> >
> > >
> > > In this way we don't have to introduce an
> > > arch_populate_kprobe_blacklist() for each architecture.
> >
> > Hmm, I had a same idea, but there are some arch which prohibit probing
> > extable entries (e.g. arm64.) For correctness of the blacklist, I think
> > it should be listed (not entire the function body).
Aah, my bad memory. I remember that the extable should not be in blacklist,
since it is a kind of "unsupported instruction to be probed" on some arch.
I'll move that in arch_prepare_kprobe() on arm64.
> > I also rather like to remove arch_within_kprobe_blacklist() instead.
Anyway, this is true. If I could make a complete list, I will remove
this function.
Thank you,
>
> OK. Thanks.
>
> -Andrea
--
Masami Hiramatsu <[email protected]>