netdev - Re: [PATCH bpf v2] bpftool: Don't crash on missing jited insns or ksyms

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 10 Dec 2019 13:52:05 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        lkml <linux-kernel@...r.kernel.org>, netdev@...r.kernel.org,
        bpf@...r.kernel.org, Martin Lau <kafai@...com>
Subject: Re: [PATCH bpf v2] bpftool: Don't crash on missing jited insns or
 ksyms

On Tue, 10 Dec 2019 13:31:50 -0800, Alexei Starovoitov wrote:
> On Tue, Dec 10, 2019 at 01:24:28PM -0800, Jakub Kicinski wrote:
> > On Tue, 10 Dec 2019 22:09:55 +0100, Toke Høiland-Jørgensen wrote:  
> > > Jakub Kicinski <jakub.kicinski@...ronome.com> writes:  
> > > > On Tue, 10 Dec 2019 19:14:12 +0100, Toke Høiland-Jørgensen wrote:    
> > > >> When the kptr_restrict sysctl is set, the kernel can fail to return
> > > >> jited_ksyms or jited_prog_insns, but still have positive values in
> > > >> nr_jited_ksyms and jited_prog_len. This causes bpftool to crash when trying
> > > >> to dump the program because it only checks the len fields not the actual
> > > >> pointers to the instructions and ksyms.
> > > >> 
> > > >> Fix this by adding the missing checks.
> > > >> 
> > > >> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>    
> > > >
> > > > Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
> > > >
> > > > and
> > > >
> > > > Fixes: f84192ee00b7 ("tools: bpftool: resolve calls without using imm field")
> > > >
> > > > ?    
> > > 
> > > Yeah, guess so? Although I must admit it's not quite clear to me whether
> > > bpftool gets stable backports, or if it follows the "only moving
> > > forward" credo of libbpf?  
> > 
> > bpftool does not have a GH repo, and seeing strength of Alexei's
> > arguments in the recent discussion - I don't think it will. So no
> > reason for bpftool to be "special"  
> 
> bpftool always was and will be a special user of libbpf.

There we go again. Making proclamations without any justification or
explanation.

Maybe there is a language barrier between us, but I wrote the initial
bpftool code, so I don't see how you (who authored one patch) can say
what it was or is. Do you mean to say what you intend to make it?

bpftool was intended to be a CLI to BPF _kernel_ interface. libbpf was
just the library that we all agreed to use moving forward for ELF
loading.

I'm not going to argue with you again. You kept bad mouthing iproute2
and then your only argument was the reviews sometimes take longer than
24 hours. Which I'm sure you have a lot of experience with:

  iproute2$ git log --author=Starov --oneline 
4bfe68253670 iptnl: add support for collect_md flag in IPv4 and IPv6 tunnels

  iproute2$ git log --author=fb.com --oneline 
3da6d055d93f bpf: add btf func and func_proto kind support
7a04dd84a7f9 bpf: check map symbol type properly with newer llvm compiler
73451259daaa tc: fix ipv6 filter selector attribute for some prefix lengths
0b4ea60b5a48 bpf: Add support for IFLA_XDP_PROG_ID
4bfe68253670 iptnl: add support for collect_md flag in IPv4 and IPv6 tunnels
414aeec90f82 ss: Add tcp_info fields data_segs_in/out
409998c5a4eb iproute: ip-gue/ip-fou manpages

Upstreaming bpftool was a big mistake, but we live and we learn.