On 7/15/21 6:44 PM, Alexei Starovoitov wrote:
On Wed, Jul 14, 2021 at 5:55 PM Yonghong Song <yhs@xxxxxx> wrote:
On 7/12/21 12:12 PM, John Fastabend wrote:
Hengqi Chen wrote:
Add vfs_read and vfs_write to bpf_d_path allowlist.
This will help tools like IOVisor's filetop to get
full file path.
Signed-off-by: Hengqi Chen <hengqi.chen@xxxxxxxxx>
---
As I understand it dpath helper is allowed as long as we
are not in NMI/interrupt context, so these should be fine
to add.
Acked-by: John Fastabend <john.fastabend@xxxxxxxxx>
The corresponding bcc discussion thread is here:
https://github.com/iovisor/bcc/issues/3527
Acked-by: Yonghong Song <yhs@xxxxxx>
kernel/trace/bpf_trace.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 64bd2d84367f..6d3f951f38c5 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -861,6 +861,8 @@ BTF_ID(func, vfs_fallocate)
BTF_ID(func, dentry_open)
BTF_ID(func, vfs_getattr)
BTF_ID(func, filp_close)
+BTF_ID(func, vfs_read)
+BTF_ID(func, vfs_write)
BTF_SET_END(btf_allowlist_d_path)
That feels incomplete.
I know we can add more later, but why these two and not vfs_readv ?
security_file_permission should probably be added as well ?
Along with all sys_* entry points ?
The first argument of bpf_d_path is "struct path *, path"
which needs to be a BTF_ID w.r.t. verifier.
I think maybe we should target the common kernel functions which
has "struct path *" or "struct file *" arguments?
vfs_readv and security_file_permission or other possible candidates
are not added since there are no use case for those yet. But agree
that adding some vfs_* calls and security_file* options are a good
call since they could be used in a different situation and adding
them may save another kernel patch.
The syscall entry points typically only contains fd. Although
bpf program might hack to do something to convert fd to a file,
I still think this is a unlikely use case.