netdev - Re: [PATCH v3 bpf-next 02/18] bpf: Add bpf_arch_text

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: <59d3af80-a781-9765-4d01-4c8006cd574f@fb.com>
Date:   Fri, 8 Nov 2019 13:41:42 +0000
From:   Alexei Starovoitov <ast@...com>
To:     Peter Zijlstra <peterz@...radead.org>,
        Alexei Starovoitov <ast@...nel.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "x86@...nel.org" <x86@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH v3 bpf-next 02/18] bpf: Add bpf_arch_text_poke() helper

On 11/8/19 1:36 AM, Peter Zijlstra wrote:
> On Fri, Nov 08, 2019 at 10:11:56AM +0100, Peter Zijlstra wrote:
>> On Thu, Nov 07, 2019 at 10:40:23PM -0800, Alexei Starovoitov wrote:
>>> Add bpf_arch_text_poke() helper that is used by BPF trampoline logic to patch
>>> nops/calls in kernel text into calls into BPF trampoline and to patch
>>> calls/nops inside BPF programs too.
>>
>> This thing assumes the text is unused, right? That isn't spelled out
>> anywhere. The implementation is very much unsafe vs concurrent execution
>> of the text.
> 
> Also, what NOP/CALL instructions will you be hijacking? If you're
> planning on using the fentry nops, then what ensures this and ftrace
> don't trample on one another? Similar for kprobes.
> 
> In general, what ensures every instruction only has a single modifier?

Looks like you didn't bother reading cover letter and missed a month
of discussions between my and Steven regarding exactly this topic
though you were directly cc-ed in all threads :(
tldr for kernel fentry nops it will be converted to use 
register_ftrace_direct() whenever it's available.
For all other nops, calls, jumps that are inside BPF programs BPF infra
will continue modifying them through this helper.
Daniel's upcoming bpf_tail_call() optimization will use text_poke as well.

 > I'm very uncomfortable letting random bpf proglets poke around in the
kernel text.

1. There is no such thing as 'proglet'. Please don't invent meaningless 
names.
2. BPF programs have no ability to modify kernel text.
3. BPF infra taking all necessary measures to make sure that poking
kernel's and BPF generated text is safe.
If you see specific issue please say so. We'll be happy to address
all issues. Being 'uncomfortable' is not constructive.