Re: [PATCH v4 bpf-next 10/14] bpf: Add d_path helper
From: Andrii Nakryiko <hidden>
Date: 2020-06-26 20:38:40
Also in:
bpf
On Thu, Jun 25, 2020 at 4:49 PM Jiri Olsa [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Adding d_path helper function that returns full path for give 'struct path' object, which needs to be the kernel BTF 'path' object. The helper calls directly d_path function. Updating also bpf.h tools uapi header and adding 'path' to bpf_helpers_doc.py script. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- include/uapi/linux/bpf.h | 14 +++++++++- kernel/trace/bpf_trace.c | 47 ++++++++++++++++++++++++++++++++++ scripts/bpf_helpers_doc.py | 2 ++ tools/include/uapi/linux/bpf.h | 14 +++++++++- 4 files changed, 75 insertions(+), 2 deletions(-)diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 0cb8ec948816..23274c81f244 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h@@ -3285,6 +3285,17 @@ union bpf_attr { * Dynamically cast a *sk* pointer to a *udp6_sock* pointer. * Return * *sk* if casting is valid, or NULL otherwise. + * + * int bpf_d_path(struct path *path, char *buf, u32 sz) + * Description + * Return full path for given 'struct path' object, which + * needs to be the kernel BTF 'path' object. The path is + * returned in buffer provided 'buf' of size 'sz'. + * + * Return + * length of returned string on success, or a negative + * error in case of failure
It's important to note whether string is always zero-terminated (I'm guessing it is, right?).
quoted hunk ↗ jump to hunk
+ * */ #define __BPF_FUNC_MAPPER(FN) \ FN(unspec), \@@ -3427,7 +3438,8 @@ union bpf_attr { FN(skc_to_tcp_sock), \ FN(skc_to_tcp_timewait_sock), \ FN(skc_to_tcp_request_sock), \ - FN(skc_to_udp6_sock), + FN(skc_to_udp6_sock), \ + FN(d_path), /* integer value in 'imm' field of BPF_CALL instruction selects which helper * function eBPF program intends to calldiff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index b124d468688c..6f31e21565b6 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c@@ -1060,6 +1060,51 @@ static const struct bpf_func_proto bpf_send_signal_thread_proto = { .arg1_type = ARG_ANYTHING, }; +BPF_CALL_3(bpf_d_path, struct path *, path, char *, buf, u32, sz) +{ + char *p = d_path(path, buf, sz - 1); + int len; + + if (IS_ERR(p)) { + len = PTR_ERR(p); + } else { + len = strlen(p); + if (len && p != buf) { + memmove(buf, p, len); + buf[len] = 0;
if len above is zero, you won't zero-terminate it, so probably better to move buf[len] = 0 out of if to do unconditionally
+ }
+ }
+
+ return len;
+}
+
+BTF_SET_START(btf_whitelist_d_path)
+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_SET_END(btf_whitelist_d_path)
+
+static bool bpf_d_path_allowed(const struct bpf_prog *prog)
+{
+ return btf_id_set_contains(&btf_whitelist_d_path, prog->aux->attach_btf_id);
+}
+This looks pretty great and clean, considering what's happening under the covers. Nice work, thanks a lot!
+BTF_ID_LIST(bpf_d_path_btf_ids) +BTF_ID(struct, path)
this is a bit more confusing to read and error-prone, but I couldn't come up with any better way to do this... Still better than alternatives.
+
+static const struct bpf_func_proto bpf_d_path_proto = {
+ .func = bpf_d_path,
+ .gpl_only = true,Does it have to be GPL-only? What's the criteria? Sorry if this was brought up previously.
+ .ret_type = RET_INTEGER, + .arg1_type = ARG_PTR_TO_BTF_ID, + .arg2_type = ARG_PTR_TO_MEM, + .arg3_type = ARG_CONST_SIZE, + .btf_id = bpf_d_path_btf_ids, + .allowed = bpf_d_path_allowed, +}; +
[...]