netdev - Re: [bpf-next PATCH] bpf: libbpf, support older style kprobe load

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 24 Oct 2019 11:55:05 -0700
From:   John Fastabend <john.fastabend@...il.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>
Cc:     John Fastabend <john.fastabend@...il.com>,
        Andrii Nakryiko <andriin@...com>,
        Alexei Starovoitov <ast@...nel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [bpf-next PATCH] bpf: libbpf, support older style kprobe load

Andrii Nakryiko wrote:
> On Tue, Oct 22, 2019 at 12:20 AM Daniel Borkmann <daniel@...earbox.net> wrote:
> >
> > On Mon, Oct 21, 2019 at 10:07:59PM -0700, John Fastabend wrote:
> > > Andrii Nakryiko wrote:
> > > > On Sat, Oct 19, 2019 at 1:30 AM John Fastabend <john.fastabend@...il.com> wrote:
> > > > >
> > > > > Following ./Documentation/trace/kprobetrace.rst add support for loading
> > > > > kprobes programs on older kernels.
> > > >
> > > > My main concern with this is that this code is born bit-rotten,
> > > > because selftests are never testing the legacy code path. How did you
> > > > think about testing this and ensuring that this keeps working going
> > > > forward?
> > >
> > > Well we use it, but I see your point and actually I even broke the retprobe
> > > piece hastily fixing merge conflicts in this patch. When I ran tests on it
> > > I missed running retprobe tests on the set of kernels that would hit that
> > > code.
> >
> > If it also gets explicitly exposed as bpf_program__attach_legacy_kprobe() or
> > such, it should be easy to add BPF selftests for that API to address the test
> > coverage concern. Generally more selftests for exposed libbpf APIs is good to
> > have anyway.
> >
> 
> Agree about tests. Disagree about more APIs, especially that the only
> difference will be which underlying kernel machinery they are using to
> set everything up. We should ideally avoid exposing that to users.

Maybe a build flag to build with only the older style supported for testing?
Then we could build, test in selftests at least. Be clear the flag is only
for testing and can not be relied upon.

> 
> > Cheers,
> > Daniel