On Mon, Jul 19, 2021 at 8:18 AM Hengqi Chen <hengqi.chen@xxxxxxxxx> 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' > `struct path *` and `struct file *` arguments. This will help tools > like IOVisor's filetop[2]/filelife to get full file path. > > Changes since v1: [1] > - Alexei and Yonghong suggested that bpf_d_path helper could also > apply to vfs_* and security_file_* kernel functions. Added them. > > [1] https://lore.kernel.org/bpf/20210712162424.2034006-1-hengqi.chen@xxxxxxxxx/ > [2] https://github.com/iovisor/bcc/issues/3527 > > Signed-off-by: Hengqi Chen <hengqi.chen@xxxxxxxxx> > --- > kernel/trace/bpf_trace.c | 50 ++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 48 insertions(+), 2 deletions(-) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 08906007306d..c784f3c7143f 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -850,16 +850,62 @@ 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_file_permission) > -BTF_ID(func, security_inode_getattr) > BTF_ID(func, security_file_open) > +BTF_ID(func, security_file_ioctl) > +BTF_ID(func, security_file_free) > +BTF_ID(func, security_file_alloc) > +BTF_ID(func, security_file_lock) > +BTF_ID(func, security_file_fcntl) > +BTF_ID(func, security_file_set_fowner) > +BTF_ID(func, security_file_receive) > +BTF_ID(func, security_inode_getattr) > #endif > #ifdef CONFIG_SECURITY_PATH > BTF_ID(func, security_path_truncate) > +BTF_ID(func, security_path_notify) > +BTF_ID(func, security_path_unlink) > +BTF_ID(func, security_path_mkdir) > +BTF_ID(func, security_path_rmdir) > +BTF_ID(func, security_path_mknod) > +BTF_ID(func, security_path_symlink) > +BTF_ID(func, security_path_link) > +BTF_ID(func, security_path_rename) > +BTF_ID(func, security_path_chmod) > +BTF_ID(func, security_path_chown) > +BTF_ID(func, security_path_chroot) > #endif > BTF_ID(func, vfs_truncate) > BTF_ID(func, vfs_fallocate) > -BTF_ID(func, dentry_open) > BTF_ID(func, vfs_getattr) > +BTF_ID(func, vfs_fadvise) > +BTF_ID(func, vfs_fchmod) > +BTF_ID(func, vfs_fchown) > +BTF_ID(func, vfs_open) > +BTF_ID(func, vfs_setpos) > +BTF_ID(func, vfs_llseek) > +BTF_ID(func, vfs_read) > +BTF_ID(func, vfs_write) > +BTF_ID(func, vfs_iocb_iter_read) > +BTF_ID(func, vfs_iter_read) > +BTF_ID(func, vfs_readv) > +BTF_ID(func, vfs_iocb_iter_write) > +BTF_ID(func, vfs_iter_write) > +BTF_ID(func, vfs_writev) > +BTF_ID(func, vfs_copy_file_range) > +BTF_ID(func, vfs_getattr_nosec) > +BTF_ID(func, vfs_ioctl) > +BTF_ID(func, vfs_fsync_range) > +BTF_ID(func, vfs_fsync) > +BTF_ID(func, vfs_utimes) > +BTF_ID(func, vfs_statfs) > +BTF_ID(func, vfs_dedupe_file_range_one) > +BTF_ID(func, vfs_dedupe_file_range) > +BTF_ID(func, vfs_clone_file_range) > +BTF_ID(func, vfs_cancel_lock) > +BTF_ID(func, vfs_test_lock) > +BTF_ID(func, vfs_setlease) > +BTF_ID(func, vfs_lock_file) > +BTF_ID(func, dentry_open) > BTF_ID(func, filp_close) > BTF_SET_END(btf_allowlist_d_path) > Before we lend this expanded list of allowed functions, I think we should address an issue that comes up from time to time with .BTF_ids. Sometimes the referenced function can be changed from global to static and get inlined by the compiler, and thus disappears from BTF altogether. This will result in kernel build failure causing a lot of confusion, because the change might be done by people unfamiliar with the BTF_ID() stuff and not even aware of it. This came up a few times before and it's frustrating for everyone involved. Before we proceed with extending the list further, let's teach resolve_btfids to warn on such missing function (so that we are at least aware) but otherwise ignore it (probably leaving ID as zero, but let's also confirm that all the users of BTF_ID() stuff handle those zeros correctly). > -- > 2.25.1 >
