[PATCH bpf-next] bpf: Make trampolines W^X
luto at amacapital.net
Sat Jan 4 00:49:10 UTC 2020
> On Jan 4, 2020, at 8:47 AM, KP Singh <kpsingh at chromium.org> wrote:
> From: KP Singh <kpsingh at google.com>
> The image for the BPF trampolines is allocated with
> bpf_jit_alloc_exe_page which marks this allocated page executable. This
> means that the allocated memory is W and X at the same time making it
> susceptible to WX based attacks.
> Since the allocated memory is shared between two trampolines (the
> current and the next), 2 pages must be allocated to adhere to W^X and
> the following sequence is obeyed where trampolines are modified:
Can we please do better rather than piling garbage on top of garbage?
> - Mark memory as non executable (set_memory_nx). While module_alloc for
> x86 allocates the memory as PAGE_KERNEL and not PAGE_KERNEL_EXEC, not
> all implementations of module_alloc do so
How about fixing this instead?
> - Mark the memory as read/write (set_memory_rw)
Probably harmless, but see above about fixing it.
> - Modify the trampoline
Seems reasonable. It’s worth noting that this whole approach is suboptimal: the “module” allocator should really be returning a list of pages to be written (not at the final address!) with the actual executable mapping to be materialized later, but that’s a bigger project that you’re welcome to ignore for now. (Concretely, it should produce a vmap address with backing pages but with the vmap alias either entirely unmapped or read-only. A subsequent healer would, all at once, make the direct map pages RO or not-present and make the vmap alias RX.)
> - Mark the memory as read-only (set_memory_ro)
> - Mark the memory as executable (set_memory_x)
No, thanks. There’s very little excuse for doing two IPI flushes when one would suffice.
As far as I know, all architectures can do this with a single flush without races x86 certainly can. The module freeing code gets this sequence right. Please reuse its mechanism or, if needed, export the relevant interfaces.
More information about the Linux-security-module-archive