| Message ID | 20191017092631.3739-1-ethercflow@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf-next] bpf: add new helper fd2path for mapping a file descriptor to a pathname | expand |
On 10/17/19 11:26 AM, Zwb wrote: > When people want to identify which file system files are being opened, > read, and written to, they can use this helper with file descriptor as > input to achieve this goal. Other pseudo filesystems are also supported. > > Signed-off-by: Zwb <ethercflow@gmail.com> SOB requires that there is a proper name, see Documentation/process/submitting-patches.rst +431. [...] > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index a65c3b0c6935..a4a5d432e572 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -2769,6 +2769,7 @@ union bpf_attr { > FN(get_current_pid_tgid), \ > FN(get_current_uid_gid), \ > FN(get_current_comm), \ > + FN(fd2path), \ Adding into the middle will break existing BPF programs. Helper description is also missing. > FN(get_cgroup_classid), \ > FN(skb_vlan_push), \ > FN(skb_vlan_pop), \
On 10/17/19 2:26 AM, Zwb wrote: > When people want to identify which file system files are being opened, > read, and written to, they can use this helper with file descriptor as > input to achieve this goal. Other pseudo filesystems are also supported. For the description, a link to related bcc issue may provide more background why this is needed. Please send to bpf mailing list bpf@vger.kernel.org. This way, more people can catch and look at this patch. Please provide a test case in tools/testing/selftests/bpf directory. > > Signed-off-by: Zwb <ethercflow@gmail.com> > --- > include/linux/bpf.h | 1 + > include/uapi/linux/bpf.h | 1 + > kernel/bpf/core.c | 1 + > kernel/bpf/helpers.c | 39 ++++++++++++++++++++++++++++++++++ > kernel/trace/bpf_trace.c | 2 ++ > tools/include/uapi/linux/bpf.h | 1 + > 6 files changed, 45 insertions(+) > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index 282e28bf41ec..c0a710cf2c88 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -1055,6 +1055,7 @@ extern const struct bpf_func_proto bpf_get_local_storage_proto; > extern const struct bpf_func_proto bpf_strtol_proto; > extern const struct bpf_func_proto bpf_strtoul_proto; > extern const struct bpf_func_proto bpf_tcp_sock_proto; > +extern const struct bpf_func_proto bpf_fd2path_proto; > > /* Shared helpers among cBPF and eBPF. */ > void bpf_user_rnd_init_once(void); > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index a65c3b0c6935..a4a5d432e572 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -2769,6 +2769,7 @@ union bpf_attr { > FN(get_current_pid_tgid), \ > FN(get_current_uid_gid), \ > FN(get_current_comm), \ > + FN(fd2path), \ As Daniel mentioned, please put this at the end of enum to avoid breaking backward compatibility. Also this needs proper documentation. > FN(get_cgroup_classid), \ > FN(skb_vlan_push), \ > FN(skb_vlan_pop), \ > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > index 66088a9e9b9e..349a8b1be232 100644 > --- a/kernel/bpf/core.c > +++ b/kernel/bpf/core.c > @@ -2042,6 +2042,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak; > const struct bpf_func_proto bpf_get_current_comm_proto __weak; > const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak; > const struct bpf_func_proto bpf_get_local_storage_proto __weak; > +const struct bpf_func_proto bpf_fd2path_proto __weak; > > const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void) > { > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > index 5e28718928ca..0832536c7ddb 100644 > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c > @@ -487,3 +487,42 @@ const struct bpf_func_proto bpf_strtoul_proto = { > .arg4_type = ARG_PTR_TO_LONG, > }; > #endif > + > +BPF_CALL_3(bpf_fd2path, char *, dst, u32, size, int, fd) > +{ > + struct fd f; > + int ret; > + char *p; > + > + ret = security_locked_down(LOCKDOWN_BPF_READ); > + if (ret < 0) > + goto out; The LOCKDOWN_BPF_READ is used when kernel internal data is exposed. That is why currently only bpf_probe_read() and bpf_probe_read_str() are checked. Not 100% sure whether this helper needs check of LOCKDOWN_BPF_READ or not. > + > + f = fdget_raw(fd); > + if (!f.file) > + goto out; > + > + p = d_path(&f.file->f_path, dst, size); inside d_path, spin_lock() could be called. I guess it is okay. > + if (IS_ERR_OR_NULL(p)) > + ret = PTR_ERR(p); > + else { > + ret = strlen(p); > + memmove(dst, p, ret); > + dst[ret] = 0; > + } > + > + if (unlikely(ret < 0)) > +out: > + memset(dst, '0', size); The coding style here is not great. Maybe you can do if (IS_ERR_OR_NULL(p)) { ret = PTRERR(p); goto error; } ret = strlen(p); memmove(dst, p, ret); dst[ret] = '\0'; return ret; error: memset(dst, '0', size); return ret; > + > + return ret; > +} > + > +const struct bpf_func_proto bpf_fd2path_proto = { > + .func = bpf_fd2path, > + .gpl_only = true, > + .ret_type = RET_INTEGER, > + .arg1_type = ARG_PTR_TO_UNINIT_MEM, > + .arg2_type = ARG_CONST_SIZE, > + .arg3_type = ARG_ANYTHING, > +}; > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 44bd08f2443b..0ca7fdefb8e5 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -735,6 +735,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > #endif > case BPF_FUNC_send_signal: > return &bpf_send_signal_proto; > + case BPF_FUNC_fd2path: > + return &bpf_fd2path_proto; > default: > return NULL; > } > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h > index a65c3b0c6935..a4a5d432e572 100644 > --- a/tools/include/uapi/linux/bpf.h > +++ b/tools/include/uapi/linux/bpf.h > @@ -2769,6 +2769,7 @@ union bpf_attr { > FN(get_current_pid_tgid), \ > FN(get_current_uid_gid), \ > FN(get_current_comm), \ > + FN(fd2path), \ > FN(get_cgroup_classid), \ > FN(skb_vlan_push), \ > FN(skb_vlan_pop), \ >
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 282e28bf41ec..c0a710cf2c88 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1055,6 +1055,7 @@ extern const struct bpf_func_proto bpf_get_local_storage_proto; extern const struct bpf_func_proto bpf_strtol_proto; extern const struct bpf_func_proto bpf_strtoul_proto; extern const struct bpf_func_proto bpf_tcp_sock_proto; +extern const struct bpf_func_proto bpf_fd2path_proto; /* Shared helpers among cBPF and eBPF. */ void bpf_user_rnd_init_once(void); diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index a65c3b0c6935..a4a5d432e572 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -2769,6 +2769,7 @@ union bpf_attr { FN(get_current_pid_tgid), \ FN(get_current_uid_gid), \ FN(get_current_comm), \ + FN(fd2path), \ FN(get_cgroup_classid), \ FN(skb_vlan_push), \ FN(skb_vlan_pop), \ diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 66088a9e9b9e..349a8b1be232 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2042,6 +2042,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak; const struct bpf_func_proto bpf_get_current_comm_proto __weak; const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak; const struct bpf_func_proto bpf_get_local_storage_proto __weak; +const struct bpf_func_proto bpf_fd2path_proto __weak; const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void) { diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 5e28718928ca..0832536c7ddb 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -487,3 +487,42 @@ const struct bpf_func_proto bpf_strtoul_proto = { .arg4_type = ARG_PTR_TO_LONG, }; #endif + +BPF_CALL_3(bpf_fd2path, char *, dst, u32, size, int, fd) +{ + struct fd f; + int ret; + char *p; + + ret = security_locked_down(LOCKDOWN_BPF_READ); + if (ret < 0) + goto out; + + f = fdget_raw(fd); + if (!f.file) + goto out; + + p = d_path(&f.file->f_path, dst, size); + if (IS_ERR_OR_NULL(p)) + ret = PTR_ERR(p); + else { + ret = strlen(p); + memmove(dst, p, ret); + dst[ret] = 0; + } + + if (unlikely(ret < 0)) +out: + memset(dst, '0', size); + + return ret; +} + +const struct bpf_func_proto bpf_fd2path_proto = { + .func = bpf_fd2path, + .gpl_only = true, + .ret_type = RET_INTEGER, + .arg1_type = ARG_PTR_TO_UNINIT_MEM, + .arg2_type = ARG_CONST_SIZE, + .arg3_type = ARG_ANYTHING, +}; diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 44bd08f2443b..0ca7fdefb8e5 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -735,6 +735,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) #endif case BPF_FUNC_send_signal: return &bpf_send_signal_proto; + case BPF_FUNC_fd2path: + return &bpf_fd2path_proto; default: return NULL; } diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index a65c3b0c6935..a4a5d432e572 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -2769,6 +2769,7 @@ union bpf_attr { FN(get_current_pid_tgid), \ FN(get_current_uid_gid), \ FN(get_current_comm), \ + FN(fd2path), \ FN(get_cgroup_classid), \ FN(skb_vlan_push), \ FN(skb_vlan_pop), \
When people want to identify which file system files are being opened, read, and written to, they can use this helper with file descriptor as input to achieve this goal. Other pseudo filesystems are also supported. Signed-off-by: Zwb <ethercflow@gmail.com> --- include/linux/bpf.h | 1 + include/uapi/linux/bpf.h | 1 + kernel/bpf/core.c | 1 + kernel/bpf/helpers.c | 39 ++++++++++++++++++++++++++++++++++ kernel/trace/bpf_trace.c | 2 ++ tools/include/uapi/linux/bpf.h | 1 + 6 files changed, 45 insertions(+)