On 7/27/21 3:25 PM, Hengqi Chen wrote:
Add vfs_* and security_* to bpf_d_path allowlist, so that we can use
bpf_d_path helper to extract full file path from these functions' arguments.
This will help tools like BCC's filetop[1]/filelife to get full file path.
[1] https://github.com/iovisor/bcc/issues/3527
Acked-by: Yonghong Song <yhs@xxxxxx>
Signed-off-by: Hengqi Chen <hengqi.chen@xxxxxxxxx>
---
kernel/trace/bpf_trace.c | 60 +++++++++++++++++++++++++++++++++++++---
1 file changed, 56 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index c5e0b6a64091..e7b24abcf3bf 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -849,18 +849,70 @@ BPF_CALL_3(bpf_d_path, struct path *, path, char *, buf, u32, sz)
BTF_SET_START(btf_allowlist_d_path)
#ifdef CONFIG_SECURITY
+BTF_ID(func, security_bprm_check)
+BTF_ID(func, security_bprm_committed_creds)
+BTF_ID(func, security_bprm_committing_creds)
+BTF_ID(func, security_bprm_creds_for_exec)
+BTF_ID(func, security_bprm_creds_from_file)
+BTF_ID(func, security_file_alloc)
Did you actually try these out, e.g. attaching BPF progs invoking bpf_d_path() to all
these, then generate some workload like kernel build for testing?
I presume not, since something like security_file_alloc() would crash the kernel. Right
before it's called in __alloc_file() we fetch a struct file from kmemcache, and only
populate f->f_cred there. Most LSMs, for example, only populate their secblob through the
callback. If you call bpf_d_path(&file->f_path, ...) with it, you'll crash in d_path()
when path->dentry->d_op is checked.. given f->f_path is all zeroed structure at that
point.
Please do your due diligence and invest each of them manually, maybe the best way is
to hack up small selftests for each enabled function that our CI can test run? Bit of a
one-time effort, but at least it ensures that those additions are sane & checked.
+BTF_ID(func, security_file_fcntl)
+BTF_ID(func, security_file_free)
+BTF_ID(func, security_file_ioctl)
+BTF_ID(func, security_file_lock)
+BTF_ID(func, security_file_open)
BTF_ID(func, security_file_permission)
+BTF_ID(func, security_file_receive)
+BTF_ID(func, security_file_set_fowner)
BTF_ID(func, security_inode_getattr)
-BTF_ID(func, security_file_open)
+BTF_ID(func, security_sb_mount)
#endif
#ifdef CONFIG_SECURITY_PATH
+BTF_ID(func, security_path_chmod)
+BTF_ID(func, security_path_chown)
+BTF_ID(func, security_path_chroot)
+BTF_ID(func, security_path_link)
+BTF_ID(func, security_path_mkdir)
+BTF_ID(func, security_path_mknod)
+BTF_ID(func, security_path_notify)
+BTF_ID(func, security_path_rename)
+BTF_ID(func, security_path_rmdir)
+BTF_ID(func, security_path_symlink)
BTF_ID(func, security_path_truncate)
+BTF_ID(func, security_path_unlink)
#endif
-BTF_ID(func, vfs_truncate)
-BTF_ID(func, vfs_fallocate)
BTF_ID(func, dentry_open)
-BTF_ID(func, vfs_getattr)
BTF_ID(func, filp_close)
+BTF_ID(func, vfs_cancel_lock)
+BTF_ID(func, vfs_clone_file_range)
+BTF_ID(func, vfs_copy_file_range)
+BTF_ID(func, vfs_dedupe_file_range)
+BTF_ID(func, vfs_dedupe_file_range_one)
+BTF_ID(func, vfs_fadvise)
+BTF_ID(func, vfs_fallocate)
+BTF_ID(func, vfs_fchmod)
+BTF_ID(func, vfs_fchown)
+BTF_ID(func, vfs_fsync)
+BTF_ID(func, vfs_fsync_range)
+BTF_ID(func, vfs_getattr)
+BTF_ID(func, vfs_getattr_nosec)
+BTF_ID(func, vfs_iocb_iter_read)
+BTF_ID(func, vfs_iocb_iter_write)
+BTF_ID(func, vfs_ioctl)
+BTF_ID(func, vfs_iter_read)
+BTF_ID(func, vfs_iter_write)
+BTF_ID(func, vfs_llseek)
+BTF_ID(func, vfs_lock_file)
+BTF_ID(func, vfs_open)
+BTF_ID(func, vfs_read)
+BTF_ID(func, vfs_readv)
+BTF_ID(func, vfs_setlease)
+BTF_ID(func, vfs_setpos)
+BTF_ID(func, vfs_statfs)
+BTF_ID(func, vfs_test_lock)
+BTF_ID(func, vfs_truncate)
+BTF_ID(func, vfs_utimes)
+BTF_ID(func, vfs_write)
+BTF_ID(func, vfs_writev)
BTF_SET_END(btf_allowlist_d_path)
static bool bpf_d_path_allowed(const struct bpf_prog *prog)
