linux-kernel - Re: [PATCH bpf] bpf: invalidate unused part of bpf_prog

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: <CAHk-=whFeBezdSrPy31iYv-UZNnNavymrhqrwCptE4uW8aeaHw@mail.gmail.com>
Date:   Thu, 21 Apr 2022 11:59:06 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Song Liu <song@...nel.org>, bpf <bpf@...r.kernel.org>,
        Linux-MM <linux-mm@...ck.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
        Christoph Hellwig <hch@...radead.org>,
        Andrii Nakryiko <andrii@...nel.org>
Subject: Re: [PATCH bpf] bpf: invalidate unused part of bpf_prog_pack

On Thu, Apr 21, 2022 at 11:24 AM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> Let's not complicate the logic by dragging jit_fill_hole
> further into generic allocation.

I agree that just zeroing the page is probably perfectly fine in
practice on x86, but I'm also not really seeing the "complication" of
just doing things right.

> The existing bpf_prog_pack code still does memset(0xcc)
> a random range of bytes before and after jit-ed bpf code.

That is actually wishful thinking, and not based on reality.

>From what I can tell, the end of the jit'ed bpf code is actually the
exception table entries, so we have that data being marked executable.

Honestly, what is wrong with this trivial patch?

I've not *tested* it, but it looks really really simple to me. Take it
as a "something like this" rather than anything else.

And yes, it would be better if bpf_jit_binary_pack_free did it too, so
that you don't have random old JIT'ed code lying around (and possibly
still in CPU branch history caches or whatever).

And it would be lovely if the exception table entries would be part of
another allocation and not marked executable.

But I certainly don't see the _downside_ (or complexity) of just doing
this, instead of zeroing things.

So this is by no means perfect, but it seems at least incrementally
_better_ than just zeroing. No?

                    Linus

View attachment "patch.diff" of type "text/x-patch" (1975 bytes)