Messages in this thread | | | Subject | Re: [PATCH v2 bpf-next 6/7] bpf: add support for %pT format specifier for bpf_trace_printk() helper | From | Yonghong Song <> | Date | Wed, 13 May 2020 17:53:42 -0700 |
| |
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@oracle.com> > --- > 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__ */ >
| |