On Fri, Jun 15, 2018 at 02:30:48AM +0200, Daniel Borkmann wrote:
> We currently lock any JITed image as read-only via bpf_jit_binary_lock_ro()
> as well as the BPF image as read-only through bpf_prog_lock_ro(). In
> the case any of these would fail we throw a WARN_ON_ONCE() in order to
> yell loudly to the log. Perhaps, to some extend, this may be comparable
> to an allocation where __GFP_NOWARN is explicitly not set.
>
> Added via 65869a47f348 ("bpf: improve read-only handling"), this behavior
> is slightly different compared to any of the other in-kernel set_memory_ro()
> users who do not check the return code of set_memory_ro() and friends /at
> all/ (e.g. in the case of module_enable_ro() / module_disable_ro()). Given
> in BPF this is mandatory hardening step, we want to know whether there
> are any issues that would leave both BPF data writable. So it happens
> that syzkaller enabled fault injection and it triggered memory allocation
> failure deep inside x86's change_page_attr_set_clr() which was triggered
> from set_memory_ro().
>
> Now, there are two options: i) leaving everything as is, and ii) reworking
> the image locking code in order to have a final checkpoint out of the
> central bpf_prog_select_runtime() which probes whether any of the calls
> during prog setup weren't successful, and then bailing out with an error.
> Option ii) is a better approach since this additional paranoia avoids
> altogether leaving any potential W+X pages from BPF side in the system.
> Therefore, lets be strict about it, and reject programs in such unlikely
> occasion. While testing I noticed also that one bpf_prog_lock_ro()
> call was missing on the outer dummy prog in case of calls, e.g. in the
> destructor we call bpf_prog_free_deferred() on the main prog where we
> try to bpf_prog_unlock_free() the program, and since we go via
> bpf_prog_select_runtime() do that as well.
>
> Reported-by: syzbot+3b889862e65a98317...@syzkaller.appspotmail.com
> Reported-by: syzbot+9e762b52dd17e616a...@syzkaller.appspotmail.com
> Signed-off-by: Daniel Borkmann <dan...@iogearbox.net>
Acked-by: Martin KaFai Lau <ka...@fb.com>
