Re: [PATCH bpf-next] bpf: Make trampolines W^X
From: Andy Lutomirski
Date: Fri Jan 03 2020 - 19:49:18 EST
> On Jan 4, 2020, at 8:47 AM, KP Singh <kpsingh@xxxxxxxxxxxx> wrote:
>
> ïFrom: KP Singh <kpsingh@xxxxxxxxxx>
>
> 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.