linux-kernel - Re: [PATCH v2 bpf-next 1/3] bpf: Add bpf_perf_prog_read

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:   Fri, 24 Jan 2020 00:09:40 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Andrii Nakryiko <andriin@...com>, Daniel Xu <dxu@...uu.xyz>,
        John Fastabend <john.fastabend@...il.com>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "ast@...nel.org" <ast@...nel.org>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Kernel Team <Kernel-team@...com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "acme@...nel.org" <acme@...nel.org>
Subject: Re: [PATCH v2 bpf-next 1/3] bpf: Add bpf_perf_prog_read_branches()
 helper

On 1/23/20 11:41 PM, Andrii Nakryiko wrote:
> On 1/23/20 2:30 PM, Daniel Xu wrote:
>> On Thu Jan 23, 2020 at 11:23 PM, Daniel Borkmann wrote:
>> [...]
>>>
>>> Yes, so we've been following this practice for all the BPF helpers no
>>> matter
>>> which program type. Though for tracing it may be up to debate whether it
>>> makes
>>> still sense given there's nothing to be leaked here since you can read
>>> this data
>>> anyway via probe read if you'd wanted to. So we might as well get rid of
>>> the
>>> clearing for all tracing helpers.
>>
>> Right, that makes sense. Do you want me to leave it in for this patchset
>> and then remove all of them in a followup patchset?
> 
> I don't think we can remove that for existing tracing helpers (e.g.,
> bpf_probe_read). There are applications that explicitly expect
> destination memory to be zeroed out on failure. It's a BPF world's
> memset(0).

Due to avoiding error checks that way if the expected outcome of the buf
is non-zero anyway? Agree, that those would break, so yeah they cannot be
removed then.

> I also wonder if BPF verifier has any extra assumptions for
> ARG_PTR_TO_UNINIT_MEM w.r.t. it being initialized after helper call
> (e.g., for liveness tracking).

There are no extra assumptions other than memory being written after the
helper call (whether success or failure of the helper itself doesn't matter,
so there are no assumptions about the content); the data that has been
written to the buffer is marked as initialized but unknown (e.g. in
check_stack_write() the case where reg remains NULL since value_regno is
negative).

Thanks,
Daniel