netdev - Re: [PATCH bpf-next 1/2] bpf: switch most helper return values from 32-bit int to 64-bit long

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]

Message-ID: <45321002-2676-0f5b-c729-5526e503ebd2@iogearbox.net>
Date:   Fri, 19 Jun 2020 15:08:19 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     John Fastabend <john.fastabend@...il.com>,
        Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH bpf-next 1/2] bpf: switch most helper return values from
 32-bit int to 64-bit long

On 6/19/20 2:39 AM, John Fastabend wrote:
> John Fastabend wrote:
>> Andrii Nakryiko wrote:
>>> On Thu, Jun 18, 2020 at 11:58 AM John Fastabend
>>> <john.fastabend@...il.com> wrote:
> 
> [...]
> 
>>> That would be great. Self-tests do work, but having more testing with
>>> real-world application would certainly help as well.
>>
>> Thanks for all the follow up.
>>
>> I ran the change through some CI on my side and it passed so I can
>> complain about a few shifts here and there or just update my code or
>> just not change the return types on my side but I'm convinced its OK
>> in most cases and helps in some so...
>>
>> Acked-by: John Fastabend <john.fastabend@...il.com>
> 
> I'll follow this up with a few more selftests to capture a couple of our
> patterns. These changes are subtle and I worry a bit that additional
> <<,s>> pattern could have the potential to break something.
> 
> Another one we didn't discuss that I found in our code base is feeding
> the output of a probe_* helper back into the size field (after some
> alu ops) of subsequent probe_* call. Unfortunately, the tests I ran
> today didn't cover that case.
> 
> I'll put it on the list tomorrow and encode these in selftests. I'll
> let the mainainers decide if they want to wait for those or not.

Given potential fragility on verifier side, my preference would be that we
have the known variations all covered in selftests before moving forward in
order to make sure they don't break in any way. Back in [0] I've seen mostly
similar cases in the way John mentioned in other projects, iirc, sysdig was
another one. If both of you could hack up the remaining cases we need to
cover and then submit a combined series, that would be great. I don't think
we need to rush this optimization w/o necessary selftests.

Thanks everyone,
Daniel

   [0] https://lore.kernel.org/bpf/20200421125822.14073-1-daniel@iogearbox.net/