Re: [PATCH v2 bpf-next 6/7] bpf: add support for %pT format specifier for bpf_trace_printk() helper
From: Yonghong Song
Date: Wed May 13 2020 - 20:54:11 EST
On 5/11/20 10:56 PM, Alan Maguire wrote:
Allow %pT[cNx0] format specifier for BTF-based display of data associated
with pointer.
Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
---
include/uapi/linux/bpf.h | 27 ++++++++++++++++++++++-----
kernel/trace/bpf_trace.c | 21 ++++++++++++++++++---
tools/include/uapi/linux/bpf.h | 27 ++++++++++++++++++++++-----
3 files changed, 62 insertions(+), 13 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 9d1932e..ab3c86c 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -695,7 +695,12 @@ struct bpf_stack_build_id {
* to file *\/sys/kernel/debug/tracing/trace* from DebugFS, if
* available. It can take up to three additional **u64**
* arguments (as an eBPF helpers, the total number of arguments is
- * limited to five).
+ * limited to five), and also supports %pT (BTF-based type
+ * printing), as long as BPF_READ lockdown is not active.
+ * "%pT" takes a "struct __btf_ptr *" as an argument; it
+ * consists of a pointer value and specified BTF type string or id
+ * used to select the type for display. For more details, see
+ * Documentation/core-api/printk-formats.rst.
*
* Each time the helper is called, it appends a line to the trace.
* Lines are discarded while *\/sys/kernel/debug/tracing/trace* is
@@ -731,10 +736,10 @@ struct bpf_stack_build_id {
* The conversion specifiers supported by *fmt* are similar, but
* more limited than for printk(). They are **%d**, **%i**,
* **%u**, **%x**, **%ld**, **%li**, **%lu**, **%lx**, **%lld**,
- * **%lli**, **%llu**, **%llx**, **%p**, **%s**. No modifier (size
- * of field, padding with zeroes, etc.) is available, and the
- * helper will return **-EINVAL** (but print nothing) if it
- * encounters an unknown specifier.
+ * **%lli**, **%llu**, **%llx**, **%p**, **%pT[cNx0], **%s**.
+ * Only %pT supports modifiers, and the helper will return
+ * **-EINVAL** (but print nothing) if it encouters an unknown
+ * specifier.
*
* Also, note that **bpf_trace_printk**\ () is slow, and should
* only be used for debugging purposes. For this reason, a notice
@@ -4058,4 +4063,16 @@ struct bpf_pidns_info {
__u32 pid;
__u32 tgid;
};
+
+/*
+ * struct __btf_ptr is used for %pT (typed pointer) display; the
+ * additional type string/BTF id are used to render the pointer
+ * data as the appropriate type.
+ */
+struct __btf_ptr {
+ void *ptr;
+ const char *type;
+ __u32 id;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d961428..c032c58 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -321,9 +321,12 @@ static const struct bpf_func_proto *bpf_get_probe_write_proto(void)
return &bpf_probe_write_user_proto;
}
+#define isbtffmt(c) \
+ (c == 'T' || c == 'c' || c == 'N' || c == 'x' || c == '0')
+
/*
* Only limited trace_printk() conversion specifiers allowed:
- * %d %i %u %x %ld %li %lu %lx %lld %lli %llu %llx %p %s
+ * %d %i %u %x %ld %li %lu %lx %lld %lli %llu %llx %p %pT %s
*/
BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
u64, arg2, u64, arg3)
@@ -361,8 +364,20 @@ static const struct bpf_func_proto *bpf_get_probe_write_proto(void)
i++;
} else if (fmt[i] == 'p' || fmt[i] == 's') {
mod[fmt_cnt]++;
- /* disallow any further format extensions */
- if (fmt[i + 1] != 0 &&
+ /*
+ * allow BTF type-based printing, and disallow any
+ * further format extensions.
+ */
+ if (fmt[i] == 'p' && fmt[i + 1] == 'T') {
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_BPF_READ);
+ if (unlikely(ret < 0))
+ return ret;
+ i++;
+ while (isbtffmt(fmt[i]))
+ i++;
The pointer passed to the helper may not be valid pointer. I think you
need to do a probe_read_kernel() here. Do an atomic memory allocation
here should be okay as this is mostly for debugging only.
+ } else if (fmt[i + 1] != 0 &&
!isspace(fmt[i + 1]) &&
!ispunct(fmt[i + 1]))
return -EINVAL;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 9d1932e..ab3c86c 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -695,7 +695,12 @@ struct bpf_stack_build_id {
* to file *\/sys/kernel/debug/tracing/trace* from DebugFS, if
* available. It can take up to three additional **u64**
* arguments (as an eBPF helpers, the total number of arguments is
- * limited to five).
+ * limited to five), and also supports %pT (BTF-based type
+ * printing), as long as BPF_READ lockdown is not active.
+ * "%pT" takes a "struct __btf_ptr *" as an argument; it
+ * consists of a pointer value and specified BTF type string or id
+ * used to select the type for display. For more details, see
+ * Documentation/core-api/printk-formats.rst.
*
* Each time the helper is called, it appends a line to the trace.
* Lines are discarded while *\/sys/kernel/debug/tracing/trace* is
@@ -731,10 +736,10 @@ struct bpf_stack_build_id {
* The conversion specifiers supported by *fmt* are similar, but
* more limited than for printk(). They are **%d**, **%i**,
* **%u**, **%x**, **%ld**, **%li**, **%lu**, **%lx**, **%lld**,
- * **%lli**, **%llu**, **%llx**, **%p**, **%s**. No modifier (size
- * of field, padding with zeroes, etc.) is available, and the
- * helper will return **-EINVAL** (but print nothing) if it
- * encounters an unknown specifier.
+ * **%lli**, **%llu**, **%llx**, **%p**, **%pT[cNx0], **%s**.
+ * Only %pT supports modifiers, and the helper will return
+ * **-EINVAL** (but print nothing) if it encouters an unknown
+ * specifier.
*
* Also, note that **bpf_trace_printk**\ () is slow, and should
* only be used for debugging purposes. For this reason, a notice
@@ -4058,4 +4063,16 @@ struct bpf_pidns_info {
__u32 pid;
__u32 tgid;
};
+
+/*
+ * struct __btf_ptr is used for %pT (typed pointer) display; the
+ * additional type string/BTF id are used to render the pointer
+ * data as the appropriate type.
+ */
+struct __btf_ptr {
+ void *ptr;
+ const char *type;
+ __u32 id;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */