* [PATCH v2 1/2] selftests/bpf: ksym_search won't check symbols exists @ 2019-03-27 14:45 Daniel T. Lee 2019-03-27 14:45 ` [PATCH v2 2/2] samples, selftests/bpf: add NULL check for ksym_search Daniel T. Lee 0 siblings, 1 reply; 5+ messages in thread From: Daniel T. Lee @ 2019-03-27 14:45 UTC (permalink / raw) To: Daniel Borkmann, Alexei Starovoitov; +Cc: netdev Currently, ksym_search located at trace_helpers won't check symbols are existing or not. In ksym_search, when symbol is not found, it will return &syms[0](_stext). But when the kernel symbols are not loaded, it will return NULL, which is not a desired action. This commit will add verification logic whether symbols are loaded prior to the symbol search. Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> --- tools/testing/selftests/bpf/trace_helpers.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c index 4cdb63bf0521..9a9fc6c9b70b 100644 --- a/tools/testing/selftests/bpf/trace_helpers.c +++ b/tools/testing/selftests/bpf/trace_helpers.c @@ -52,6 +52,10 @@ struct ksym *ksym_search(long key) int start = 0, end = sym_cnt; int result; + /* kallsyms not loaded. return NULL */ + if (sym_cnt <= 0) + return NULL; + while (start < end) { size_t mid = start + (end - start) / 2; -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] samples, selftests/bpf: add NULL check for ksym_search 2019-03-27 14:45 [PATCH v2 1/2] selftests/bpf: ksym_search won't check symbols exists Daniel T. Lee @ 2019-03-27 14:45 ` Daniel T. Lee 2019-03-28 16:30 ` Daniel Borkmann 0 siblings, 1 reply; 5+ messages in thread From: Daniel T. Lee @ 2019-03-27 14:45 UTC (permalink / raw) To: Daniel Borkmann, Alexei Starovoitov; +Cc: netdev Since, ksym_search added with verification logic for symbols existence, it could return NULL when the kernel symbols are not loaded. This commit will add NULL check logic after ksym_search. Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> --- Changes in v2: - Added NULL check for selftests/bpf/prog_tests/get_stack_raw_tp.c samples/bpf/offwaketime_user.c | 5 +++++ samples/bpf/sampleip_user.c | 5 +++++ samples/bpf/trace_event_user.c | 5 +++++ tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c | 4 ++-- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/samples/bpf/offwaketime_user.c b/samples/bpf/offwaketime_user.c index f06063af9fcb..bb315ce1b866 100644 --- a/samples/bpf/offwaketime_user.c +++ b/samples/bpf/offwaketime_user.c @@ -28,6 +28,11 @@ static void print_ksym(__u64 addr) if (!addr) return; sym = ksym_search(addr); + if (!sym) { + printf("ksym not found. Is kallsyms loaded?\n"); + return; + } + if (PRINT_RAW_ADDR) printf("%s/%llx;", sym->name, addr); else diff --git a/samples/bpf/sampleip_user.c b/samples/bpf/sampleip_user.c index 216c7ecbbbe9..23b90a45c802 100644 --- a/samples/bpf/sampleip_user.c +++ b/samples/bpf/sampleip_user.c @@ -109,6 +109,11 @@ static void print_ip_map(int fd) for (i = 0; i < max; i++) { if (counts[i].ip > PAGE_OFFSET) { sym = ksym_search(counts[i].ip); + if (!sym) { + printf("ksym not found. Is kallsyms loaded?\n"); + continue; + } + printf("0x%-17llx %-32s %u\n", counts[i].ip, sym->name, counts[i].count); } else { diff --git a/samples/bpf/trace_event_user.c b/samples/bpf/trace_event_user.c index d08046ab81f0..d4178f60e075 100644 --- a/samples/bpf/trace_event_user.c +++ b/samples/bpf/trace_event_user.c @@ -34,6 +34,11 @@ static void print_ksym(__u64 addr) if (!addr) return; sym = ksym_search(addr); + if (!sym) { + printf("ksym not found. Is kallsyms loaded?\n"); + return; + } + printf("%s;", sym->name); if (!strcmp(sym->name, "sys_read")) sys_read_seen = true; diff --git a/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c b/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c index d7bb5beb1c57..c2a0a9d5591b 100644 --- a/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c +++ b/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c @@ -39,7 +39,7 @@ static int get_stack_print_output(void *data, int size) } else { for (i = 0; i < num_stack; i++) { ks = ksym_search(raw_data[i]); - if (strcmp(ks->name, nonjit_func) == 0) { + if (ks && (strcmp(ks->name, nonjit_func) == 0)) { found = true; break; } @@ -56,7 +56,7 @@ static int get_stack_print_output(void *data, int size) } else { for (i = 0; i < num_stack; i++) { ks = ksym_search(e->kern_stack[i]); - if (strcmp(ks->name, nonjit_func) == 0) { + if (ks && (strcmp(ks->name, nonjit_func) == 0)) { good_kern_stack = true; break; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] samples, selftests/bpf: add NULL check for ksym_search 2019-03-27 14:45 ` [PATCH v2 2/2] samples, selftests/bpf: add NULL check for ksym_search Daniel T. Lee @ 2019-03-28 16:30 ` Daniel Borkmann 2019-03-29 2:54 ` Daniel T. Lee 0 siblings, 1 reply; 5+ messages in thread From: Daniel Borkmann @ 2019-03-28 16:30 UTC (permalink / raw) To: Daniel T. Lee, Alexei Starovoitov; +Cc: netdev On 03/27/2019 03:45 PM, Daniel T. Lee wrote: > Since, ksym_search added with verification logic for symbols existence, > it could return NULL when the kernel symbols are not loaded. > > This commit will add NULL check logic after ksym_search. > > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> > --- > Changes in v2: > - Added NULL check for selftests/bpf/prog_tests/get_stack_raw_tp.c > > samples/bpf/offwaketime_user.c | 5 +++++ > samples/bpf/sampleip_user.c | 5 +++++ > samples/bpf/trace_event_user.c | 5 +++++ > tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c | 4 ++-- > 4 files changed, 17 insertions(+), 2 deletions(-) Still not all occurrences from samples covered: $ git grep -n ksym_search samples/bpf/ samples/bpf/offwaketime_user.c:30: sym = ksym_search(addr); samples/bpf/sampleip_user.c:111: sym = ksym_search(counts[i].ip); samples/bpf/spintest_user.c:39: sym = ksym_search(value); samples/bpf/trace_event_user.c:36: sym = ksym_search(addr); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] samples, selftests/bpf: add NULL check for ksym_search 2019-03-28 16:30 ` Daniel Borkmann @ 2019-03-29 2:54 ` Daniel T. Lee 2019-04-02 12:13 ` Daniel T. Lee 0 siblings, 1 reply; 5+ messages in thread From: Daniel T. Lee @ 2019-03-29 2:54 UTC (permalink / raw) To: Daniel Borkmann; +Cc: Alexei Starovoitov, netdev I'm aware that I didn't cover the "spintest_user.c". Because, spintest sample isn't running currently since there is no such symbol called 'spin_lock'. At "spintest_kern.c", it has 'kprobe/spin_lock' and 'kprobe/spin_unlock' and some others kernel symbols like below. SEC("kprobe/spin_unlock")PROG(p1) SEC("kprobe/spin_lock")PROG(p2) SEC("kprobe/mutex_spin_on_owner")PROG(p3) SEC("kprobe/rwsem_spin_on_owner")PROG(p4) SEC("kprobe/spin_unlock_irqrestore")PROG(p5) SEC("kprobe/_raw_spin_unlock_irqrestore")PROG(p6) SEC("kprobe/_raw_spin_unlock_bh")PROG(p7) SEC("kprobe/_raw_spin_unlock")PROG(p8) SEC("kprobe/_raw_spin_lock_irqsave")PROG(p9) .... But the thing is, 'spin_lock' kernel symbol won't exist, as it has been __always_inline'd at commit 349056 at Jul,13,2015. https://cregit.linuxsources.org/code/4.20/include/linux/spinlock.h.html#327 $ uname -r 5.0.0+ $ grep "spin_lock" /proc/kallsyms 0000000000000000 T queued_spin_lock_slowpath 0000000000000000 t process_spin_lock 0000000000000000 T bpf_spin_lock 0000000000000000 T btf_find_spin_lock 0000000000000000 T _raw_spin_lock_bh 0000000000000000 T _raw_spin_lock_irqsave 0000000000000000 T _raw_spin_lock 0000000000000000 T _raw_spin_lock_irq ... I'm pretty sure there is something I don't know due to my lack of skills. Is there any way to run this sample without having such kernel symbol? On Fri, Mar 29, 2019 at 1:30 AM Daniel Borkmann <daniel@iogearbox.net> wrote: > > On 03/27/2019 03:45 PM, Daniel T. Lee wrote: > > Since, ksym_search added with verification logic for symbols existence, > > it could return NULL when the kernel symbols are not loaded. > > > > This commit will add NULL check logic after ksym_search. > > > > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> > > --- > > Changes in v2: > > - Added NULL check for selftests/bpf/prog_tests/get_stack_raw_tp.c > > > > samples/bpf/offwaketime_user.c | 5 +++++ > > samples/bpf/sampleip_user.c | 5 +++++ > > samples/bpf/trace_event_user.c | 5 +++++ > > tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c | 4 ++-- > > 4 files changed, 17 insertions(+), 2 deletions(-) > > Still not all occurrences from samples covered: > > $ git grep -n ksym_search samples/bpf/ > samples/bpf/offwaketime_user.c:30: sym = ksym_search(addr); > samples/bpf/sampleip_user.c:111: sym = ksym_search(counts[i].ip); > samples/bpf/spintest_user.c:39: sym = ksym_search(value); > samples/bpf/trace_event_user.c:36: sym = ksym_search(addr); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] samples, selftests/bpf: add NULL check for ksym_search 2019-03-29 2:54 ` Daniel T. Lee @ 2019-04-02 12:13 ` Daniel T. Lee 0 siblings, 0 replies; 5+ messages in thread From: Daniel T. Lee @ 2019-04-02 12:13 UTC (permalink / raw) To: Daniel Borkmann; +Cc: Alexei Starovoitov, netdev ping? On Fri, Mar 29, 2019 at 11:54 AM Daniel T. Lee <danieltimlee@gmail.com> wrote: > > I'm aware that I didn't cover the "spintest_user.c". > Because, spintest sample isn't running currently since there is no > such symbol called 'spin_lock'. > > At "spintest_kern.c", it has 'kprobe/spin_lock' and > 'kprobe/spin_unlock' and some > others kernel symbols like below. > > SEC("kprobe/spin_unlock")PROG(p1) > SEC("kprobe/spin_lock")PROG(p2) > SEC("kprobe/mutex_spin_on_owner")PROG(p3) > SEC("kprobe/rwsem_spin_on_owner")PROG(p4) > SEC("kprobe/spin_unlock_irqrestore")PROG(p5) > SEC("kprobe/_raw_spin_unlock_irqrestore")PROG(p6) > SEC("kprobe/_raw_spin_unlock_bh")PROG(p7) > SEC("kprobe/_raw_spin_unlock")PROG(p8) > SEC("kprobe/_raw_spin_lock_irqsave")PROG(p9) > .... > > But the thing is, 'spin_lock' kernel symbol won't exist, as it has > been __always_inline'd > at commit 349056 at Jul,13,2015. > > https://cregit.linuxsources.org/code/4.20/include/linux/spinlock.h.html#327 > > $ uname -r > 5.0.0+ > $ grep "spin_lock" /proc/kallsyms > 0000000000000000 T queued_spin_lock_slowpath > 0000000000000000 t process_spin_lock > 0000000000000000 T bpf_spin_lock > 0000000000000000 T btf_find_spin_lock > 0000000000000000 T _raw_spin_lock_bh > 0000000000000000 T _raw_spin_lock_irqsave > 0000000000000000 T _raw_spin_lock > 0000000000000000 T _raw_spin_lock_irq > ... > > I'm pretty sure there is something I don't know due to my lack of skills. > Is there any way to run this sample without having such kernel symbol? > > On Fri, Mar 29, 2019 at 1:30 AM Daniel Borkmann <daniel@iogearbox.net> wrote: > > > > On 03/27/2019 03:45 PM, Daniel T. Lee wrote: > > > Since, ksym_search added with verification logic for symbols existence, > > > it could return NULL when the kernel symbols are not loaded. > > > > > > This commit will add NULL check logic after ksym_search. > > > > > > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> > > > --- > > > Changes in v2: > > > - Added NULL check for selftests/bpf/prog_tests/get_stack_raw_tp.c > > > > > > samples/bpf/offwaketime_user.c | 5 +++++ > > > samples/bpf/sampleip_user.c | 5 +++++ > > > samples/bpf/trace_event_user.c | 5 +++++ > > > tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c | 4 ++-- > > > 4 files changed, 17 insertions(+), 2 deletions(-) > > > > Still not all occurrences from samples covered: > > > > $ git grep -n ksym_search samples/bpf/ > > samples/bpf/offwaketime_user.c:30: sym = ksym_search(addr); > > samples/bpf/sampleip_user.c:111: sym = ksym_search(counts[i].ip); > > samples/bpf/spintest_user.c:39: sym = ksym_search(value); > > samples/bpf/trace_event_user.c:36: sym = ksym_search(addr); ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-02 12:16 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-03-27 14:45 [PATCH v2 1/2] selftests/bpf: ksym_search won't check symbols exists Daniel T. Lee 2019-03-27 14:45 ` [PATCH v2 2/2] samples, selftests/bpf: add NULL check for ksym_search Daniel T. Lee 2019-03-28 16:30 ` Daniel Borkmann 2019-03-29 2:54 ` Daniel T. Lee 2019-04-02 12:13 ` Daniel T. Lee
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).