Message ID | 20230606181714.532998-1-jolsa@kernel.org (mailing list archive) |
---|---|
State | Accepted |
Delegated to: | BPF |
Headers | show |
Series | [PATCHv2,bpf] bpf: Add extra path pointer check to d_path helper | expand |
On 6/6/23 11:17 AM, Jiri Olsa wrote: > Anastasios reported crash on stable 5.15 kernel with following > bpf attached to lsm hook: > > SEC("lsm.s/bprm_creds_for_exec") > int BPF_PROG(bprm_creds_for_exec, struct linux_binprm *bprm) > { > struct path *path = &bprm->executable->f_path; > char p[128] = { 0 }; > > bpf_d_path(path, p, 128); > return 0; > } > > but bprm->executable can be NULL, so bpf_d_path call will crash: > > BUG: kernel NULL pointer dereference, address: 0000000000000018 > #PF: supervisor read access in kernel mode > #PF: error_code(0x0000) - not-present page > PGD 0 P4D 0 > Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC NOPTI > ... > RIP: 0010:d_path+0x22/0x280 > ... > Call Trace: > <TASK> > bpf_d_path+0x21/0x60 > bpf_prog_db9cf176e84498d9_bprm_creds_for_exec+0x94/0x99 > bpf_trampoline_6442506293_0+0x55/0x1000 > bpf_lsm_bprm_creds_for_exec+0x5/0x10 > security_bprm_creds_for_exec+0x29/0x40 > bprm_execve+0x1c1/0x900 > do_execveat_common.isra.0+0x1af/0x260 > __x64_sys_execve+0x32/0x40 > > It's problem for all stable trees with bpf_d_path helper, which was > added in 5.9. > > This issue is fixed in current bpf code, where we identify and mark > trusted pointers, so the above code would fail even to load. > > For the sake of the stable trees and to workaround potentially broken > verifier in the future, adding the code that reads the path object from > the passed pointer and verifies it's valid in kernel space. > > Cc: stable@vger.kernel.org # v5.9+ > Fixes: 6e22ab9da793 ("bpf: Add d_path helper") > Acked-by: Stanislav Fomichev <sdf@google.com> > Suggested-by: Alexei Starovoitov <ast@kernel.org> > Reported-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com> > Signed-off-by: Jiri Olsa <jolsa@kernel.org> Acked-by: Yonghong Song <yhs@fb.com>
On 6/6/23 8:17 PM, Jiri Olsa wrote: > Anastasios reported crash on stable 5.15 kernel with following > bpf attached to lsm hook: [...] > Fixes: 6e22ab9da793 ("bpf: Add d_path helper") > Acked-by: Stanislav Fomichev <sdf@google.com> > Suggested-by: Alexei Starovoitov <ast@kernel.org> > Reported-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com> > Signed-off-by: Jiri Olsa <jolsa@kernel.org> Looks like patchbot is not replying.. applied, thanks!
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 9a050e36dc6c..1f4b07da327a 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -900,13 +900,23 @@ static const struct bpf_func_proto bpf_send_signal_thread_proto = { BPF_CALL_3(bpf_d_path, struct path *, path, char *, buf, u32, sz) { + struct path copy; long len; char *p; if (!sz) return 0; - p = d_path(path, buf, sz); + /* + * The path pointer is verified as trusted and safe to use, + * but let's double check it's valid anyway to workaround + * potentially broken verifier. + */ + len = copy_from_kernel_nofault(©, path, sizeof(*path)); + if (len < 0) + return len; + + p = d_path(©, buf, sz); if (IS_ERR(p)) { len = PTR_ERR(p); } else {