On 7/18/21 12:43 AM, Yonghong Song wrote: > > > 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. Thanks for the review and suggestions. I will send a v2 for review. Thanks, Hengqi
