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

Powered by blists - more mailing lists