diff mbox series

[v3,bpf-next,7/8] bpf: add support for %pT format specifier for bpf_trace_printk() helper

Message ID 1592914031-31049-8-git-send-email-alan.maguire@oracle.com
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series bpf, printk: add BTF-based type printing | expand

Commit Message

Alan Maguire June 23, 2020, 12:07 p.m. UTC
Allow %pT[cNx0] format specifier for BTF-based display of data associated
with pointer.  The unsafe data modifier 'u' - where the source data
is traversed without copying it to a safe buffer via probe_kernel_read() -
is not supported.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 include/uapi/linux/bpf.h       | 27 ++++++++++++++++++++++-----
 kernel/trace/bpf_trace.c       | 24 +++++++++++++++++++++++-
 tools/include/uapi/linux/bpf.h | 27 ++++++++++++++++++++++-----
 3 files changed, 67 insertions(+), 11 deletions(-)

Comments

Andy Shevchenko June 23, 2020, 1:04 p.m. UTC | #1
On Tue, Jun 23, 2020 at 01:07:10PM +0100, Alan Maguire wrote:
> Allow %pT[cNx0] format specifier for BTF-based display of data associated
> with pointer.  The unsafe data modifier 'u' - where the source data
> is traversed without copying it to a safe buffer via probe_kernel_read() -
> is not supported.

I think I already saw this data structure definition...

> +/*
> + * 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;
> +};

So, this is 2nd...


> +/*
> + * 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;
> +};

,..and this is 3rd copy. Why?
diff mbox series

Patch

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 1968481..ea4fbf3 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -702,7 +702,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
@@ -738,10 +743,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
@@ -4260,4 +4265,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 e729c9e5..33ddb31 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -374,9 +374,13 @@  static void bpf_trace_copy_string(char *buf, void *unsafe_ptr, char fmt_ptype,
 	}
 }
 
+/* Unsafe BTF display ('u' modifier) is absent here. */
+#define is_btf_safe_modifier(c)		\
+	(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 %pks %pus %s
+ * %d %i %u %x %ld %li %lu %lx %lld %lli %llu %llx %p %pks %pus %s %pT
  */
 BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
 	   u64, arg2, u64, arg3)
@@ -412,6 +416,24 @@  static void bpf_trace_copy_string(char *buf, void *unsafe_ptr, char fmt_ptype,
 			i++;
 		} else if (fmt[i] == 'p') {
 			mod[fmt_cnt]++;
+
+			/*
+			 * allow BTF type-based printing, but disallow unsafe
+			 * mode - this ensures the data is copied safely
+			 * using probe_kernel_read() prior to traversing it.
+			 */
+			if (fmt[i + 1] == 'T') {
+				int ret;
+
+				ret = security_locked_down(LOCKDOWN_BPF_READ);
+				if (unlikely(ret < 0))
+					return ret;
+				i += 2;
+				while (is_btf_safe_modifier(fmt[i]))
+					i++;
+				goto fmt_next;
+			}
+
 			if ((fmt[i + 1] == 'k' ||
 			     fmt[i + 1] == 'u') &&
 			    fmt[i + 2] == 's') {
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 1968481..ea4fbf3 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -702,7 +702,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
@@ -738,10 +743,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
@@ -4260,4 +4265,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__ */