[PATCH RFC 2/2] selftests/bpf: test f_inode walk around bpf_put_file()

[Chris Mason <mason@kernel.org>]

No selftest covers a trusted pointer that is loaded from a referenced
object and used after the reference is released. The verifier change
in the previous patch is therefore untested.

The previous patch links a trusted pointer loaded from a referenced
object to that reference through parent_id, so that
release_reference() invalidates it together with its parent. Before
that, a program could keep f->f_inode from a bpf_get_task_exe_file()
file and dereference it after bpf_put_file().

Test both sides of the release on that f_inode walk. The reject test
dereferences the inode after the put and expects "invalid mem access
'scalar'". Without the previous patch the verifier accepts the program,
so the test fails. The accept control reads inode->i_ino while the
reference is held and uses only the scalar after the put, which catches
a verifier that invalidates too much.

Walked pointers have been able to stay trusted since commit
57539b1c0ac2 ("bpf: Enable annotating trusted nested pointers").

Assisted-by: kres
Signed-off-by: Chris Mason <mason@kernel.org>
---
 .../selftests/bpf/progs/verifier_vfs_accept.c | 26 ++++++++++++++++++
 .../selftests/bpf/progs/verifier_vfs_reject.c | 27 +++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c b/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c
index 55398c04290a..6fb0d2ed3d5f 100644
--- a/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c
+++ b/tools/testing/selftests/bpf/progs/verifier_vfs_accept.c
@@ -100,4 +100,30 @@ int BPF_PROG(inode_rename, struct inode *old_dir, struct dentry *old_dentry,
 	return 0;
 }
 
+SEC("lsm.s/file_open")
+__success
+int BPF_PROG(f_inode_walk_used_before_put_file)
+{
+	struct file *acquired;
+	struct inode *inode;
+	ino_t ino;
+
+	acquired = bpf_get_task_exe_file(bpf_get_current_task_btf());
+	if (!acquired)
+		return 0;
+
+	/*
+	 * f_inode walked from a referenced file can be dereferenced while
+	 * the file reference is held. Only the scalar read from it is used
+	 * after the put.
+	 */
+	inode = acquired->f_inode;
+	ino = inode->i_ino;
+	bpf_put_file(acquired);
+
+	if (ino == 0)
+		return -EACCES;
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
index b5f456d57669..536acbe15357 100644
--- a/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
+++ b/tools/testing/selftests/bpf/progs/verifier_vfs_reject.c
@@ -173,4 +173,31 @@ int BPF_PROG(inode_rename, struct inode *old_dir, struct dentry *old_dentry,
 		return -EACCES;
 	return 0;
 }
+
+SEC("lsm.s/file_open")
+__failure __msg("invalid mem access 'scalar'")
+int BPF_PROG(f_inode_walk_used_after_put_file)
+{
+	struct file *acquired;
+	struct inode *inode;
+	ino_t ino;
+
+	acquired = bpf_get_task_exe_file(bpf_get_current_task_btf());
+	if (!acquired)
+		return 0;
+
+	/*
+	 * f_inode walked from a referenced file is trusted, but only for
+	 * as long as the file reference is held.
+	 */
+	inode = acquired->f_inode;
+	bpf_put_file(acquired);
+
+	/* The put invalidated inode, it can't be dereferenced anymore. */
+	ino = inode->i_ino;
+	if (ino == 0)
+		return -EACCES;
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.54.0