netdev - Re: [PATCH v2 2/2] samples, selftests/bpf: add NULL check for ksym

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAEKGpzjPBQgDX1XyvcuG=vopg=xHZ9uNFxdDeei939GsBEt2yw@mail.gmail.com>
Date:   Tue, 2 Apr 2019 21:13:43 +0900
From:   "Daniel T. Lee" <danieltimlee@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Alexei Starovoitov <ast@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH v2 2/2] samples, selftests/bpf: add NULL check for ksym_search

ping?

On Fri, Mar 29, 2019 at 11:54 AM Daniel T. Lee <danieltimlee@...il.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@...earbox.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@...il.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);