netdev - Re: [PATCH bpf-next 0/2] bpf: Support variable offset stack access from helpers

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:   Mon, 1 Apr 2019 20:57:54 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Alexei Starovoitov <ast@...com>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Andrey Ignatov <rdna@...com>
Cc:     Network Development <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH bpf-next 0/2] bpf: Support variable offset stack access
 from helpers

On 04/01/2019 07:23 PM, Alexei Starovoitov wrote:
> On 4/1/19 9:09 AM, Daniel Borkmann wrote:
>> On 03/29/2019 08:10 PM, Alexei Starovoitov wrote:
>>> On Thu, Mar 28, 2019 at 6:02 PM Andrey Ignatov <rdna@...com> wrote:
>>>>
>>>> The patch set adds support for stack access with variable offset from helpers.
>>>>
>>>> Patch 1 is the main patch in the set and provides more details.
>>>> Patch 2 adds selftests for new functionality.
>>>
>>> Applied. Thanks
>>
>> Hmm, I think this series needs more work unfortunately. The selftests are only
>> checking root-only programs, which is way to little. For !root we do the spectre
>> masking for map and stack ALU, and that hasn't been adapted here, so it will
>> generate a wrong masking for runtime since it doesn't take variable part into
>> account. Andrey, please take a look.
> 
> right. may be we should allow this for root only then?

Probably yeah, though thinking more about it, what about the case where we pass
in raw (uninitialized) buffers from stack into a helper? Our assumption has
been thus far that given the size is const, we can mark them in verifier as
initialized after the call (as helpers memset it on error). With variable access
it could be within a given range from verification side, but at runtime it's
concrete value, meaning, upon function return we could leak uninitialized stack
where verifier thinks it has been initialized by the helper. I think the set
doesn't address this either, unfortunately. (So would need to be restricted to
helpers where we pass always initialized buffers into it.)

Thanks,
Daniel