Skip to content

Commit f46fab0

Browse files
olsajiriborkmann
authored andcommitted
bpf: Add extra path pointer check to d_path helper
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. Fixes: 6e22ab9 ("bpf: Add d_path helper") Reported-by: Anastasios Papagiannis <[email protected]> Suggested-by: Alexei Starovoitov <[email protected]> Signed-off-by: Jiri Olsa <[email protected]> Signed-off-by: Daniel Borkmann <[email protected]> Acked-by: Stanislav Fomichev <[email protected]> Acked-by: Yonghong Song <[email protected]> Link: https://lore.kernel.org/bpf/[email protected]
1 parent 69844e3 commit f46fab0

File tree

1 file changed

+11
-1
lines changed

1 file changed

+11
-1
lines changed

kernel/trace/bpf_trace.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -900,13 +900,23 @@ static const struct bpf_func_proto bpf_send_signal_thread_proto = {
900900

901901
BPF_CALL_3(bpf_d_path, struct path *, path, char *, buf, u32, sz)
902902
{
903+
struct path copy;
903904
long len;
904905
char *p;
905906

906907
if (!sz)
907908
return 0;
908909

909-
p = d_path(path, buf, sz);
910+
/*
911+
* The path pointer is verified as trusted and safe to use,
912+
* but let's double check it's valid anyway to workaround
913+
* potentially broken verifier.
914+
*/
915+
len = copy_from_kernel_nofault(&copy, path, sizeof(*path));
916+
if (len < 0)
917+
return len;
918+
919+
p = d_path(&copy, buf, sz);
910920
if (IS_ERR(p)) {
911921
len = PTR_ERR(p);
912922
} else {

0 commit comments

Comments
 (0)