linux-kernel - Re: [PATCH v4 bpf 0/4] vmalloc: bpf: introduce VM_ALLOW_HUGE

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: <B995F7EB-2019-4290-9C09-AE19C5BA3A70@fb.com>
Date:   Sat, 16 Apr 2022 22:26:08 +0000
From:   Song Liu <songliubraving@...com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
CC:     Christoph Hellwig <hch@...radead.org>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Song Liu <song@...nel.org>, bpf <bpf@...r.kernel.org>,
        Linux Memory Management List <linux-mm@...ck.org>,
        open 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>,
        Claudio Imbrenda <imbrenda@...ux.ibm.com>
Subject: Re: [PATCH v4 bpf 0/4] vmalloc: bpf: introduce VM_ALLOW_HUGE_VMAP

Hi Linus,

Thanks a lot for your kind reply. 

> On Apr 16, 2022, at 1:30 PM, Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> 
> On Sat, Apr 16, 2022 at 12:55 PM Song Liu <songliubraving@...com> wrote:
>> 
>> Based on this analysis, I think we should either
>>  1) ship the whole set with 5.18; or
>>  2) ship 1/4, 3/4, and 4/4 with 5.18, and 2/4 with 5.19.
> 
> Honestly, I think the proper thing to do is
> 
> - apply #1, because yes, that "use huge pages" should be an opt-in.
> 
> - but just disable hugepages for now.

Hmm.. This sure is an option...

> 
> I think those games with set_memory_nx() and friends just show how
> rough this all is right now.
> 
> In fact, I personally think that the whole bpf 'prog_pack' stuff
> should probably be disabled. It looks incredible broken to me right
> now.
> 
> Unless I mis-read it, it does a "module_alloc()" to allocate the vmap
> area, and then just marks it executable without having even
> initialized the pages. Am I missing something? So now we have random
> kernel memory that is marked executable.
> 
> Sure, it's also marked RO, but who cares? It's random data that is now
> executable.

While this is a serious issue (my fault), it is relatively easy to fix.
We can fill the whole space with invalid instructions when the page
is allocated and when a BPF program is freed. Both these operations are
on the slow paths, so the overhead is minimal.  

> 
> Maybe I am missing something, but I really don't think this is ready
> for prime-time. We should effectively disable it all, and have people
> think through it a lot more.

This has been discussed on lwn.net: https://lwn.net/Articles/883454/. 
AFAICT, the biggest concern is whether reserving minimal 2MB for BPF
programs is a good trade-off for memory usage. This is again my fault
not to state the motivation clearly: the primary gain comes from less 
page table fragmentation and thus better iTLB efficiency. 

Other folks (in recent thread on this topic and offline in other 
discussions) also showed strong interests in using similar technical 
for text of kernel modules. So I would really like to learn your 
opinion on this. There are many details we can optimize, but I guess 
the general mechanism has to be something like:
 - allocate a huge page, make it safe, and set it as executable;
 - as users (BPF, kernel module, etc.) request memory for text, give
   a chunk of the huge page to the user. 
 - use some mechanism to update the chunk of memory safely. 

As you correctly noted, I missed the "make it safe" step (which 
should be easy to fix). But the rest of the flow is more or less the 
best solution to me. 

Did I totally miss some better option here? Or is this really not a 
viable option (IOW, bpf programs and kernel modules have to use 
regular pages)? 

Thanks again,
Song