[PATCH RFC 1/2] bpf: fix UAF via trusted BTF walk after release

[Chris Mason <mason@kernel.org>]

A BPF program can keep dereferencing a trusted pointer that it loaded
from a referenced object after it has released that reference. With a
file from bpf_get_task_exe_file(), f->f_inode stays usable after
bpf_put_file(), and loads through it read freed or reused inode memory.

A pointer derived from a referenced object is only valid while the
reference is held. release_reference() enforces that by invalidating
every register whose id or parent_id matches the released id. However,
check_ptr_to_btf_access() builds the loaded register with
mark_btf_ld_reg(), which is not given the source register and zeroes
both fields. The verifier state for the f_inode case:

    file = bpf_get_task_exe_file(task)   file:  id=N, referenced
    inode = file->f_inode                inode: trusted, id=0,
                                                parent_id=0
    bpf_put_file(file)                   release_reference(N)
                                           skips inode
    inode->i_ino                         accepted, unprobed load

Reproduced by reading inode->i_ino after bpf_put_file() once the task
owning the file had exited and new fds were created.

Fix this by recording the source's reference id in the parent_id of a
trusted PTR_TO_BTF_ID load. A program that dereferences such a pointer
after the release is now rejected with "invalid mem access 'scalar'".

Assisted-by: kres
Fixes: 57539b1c0ac2 ("bpf: Enable annotating trusted nested pointers")
Signed-off-by: Chris Mason <mason@kernel.org>
---
 kernel/bpf/verifier.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6c6b8d8520cd..8b48bf796823 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6219,9 +6219,25 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
 	}
 
 	if (atype == BPF_READ && value_regno >= 0) {
+		u32 link_id = 0;
+
+		/*
+		 * A trusted pointer walked from a referenced object is only
+		 * valid while that reference is held. Remember the reference
+		 * id of the source, or the one it inherited, so that
+		 * release_reference() invalidates the loaded register too.
+		 * Read it now, reg may alias regs[value_regno].
+		 */
+		if (ret == PTR_TO_BTF_ID && (flag & PTR_TRUSTED))
+			link_id = reg_is_referenced(env, reg) ? reg->id : reg->parent_id;
+
 		ret = mark_btf_ld_reg(env, regs, value_regno, ret, reg->btf, btf_id, flag);
 		if (ret < 0)
 			return ret;
+
+		/* mark_btf_ld_reg() cleared parent_id, set it afterwards. */
+		if (link_id)
+			regs[value_regno].parent_id = link_id;
 	}
 
 	return 0;
-- 
2.54.0