Skip to contentSkip to conversation

FAILED: patch "[PATCH] bpf: Restrict bpf_trace_printk()'s %s usage and add %pks," failed to apply to 5.4-stable tree

PATCH [email protected] 2 participants 3 messages May 18, 2020 – May 19, 2020

Subsystems (8)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From b2a5212fb634561bb734c6356904e37f6665b955 Mon Sep 17 00:00:00 2001
From: Daniel Borkmann <[email protected]>
Date: Fri, 15 May 2020 12:11:18 +0200
Subject: [PATCH] bpf: Restrict bpf_trace_printk()'s %s usage and add %pks,
%pus specifier

Usage of plain %s conversion specifier in bpf_trace_printk() suffers from the very same issue as bpf_probe_read{,str}() helpers, that is, it is broken on archs with overlapping address ranges.

While the helpers have been addressed through work in 6ae08ae3dea2 ("bpf: Add probe_read_{user, kernel} and probe_read_{user, kernel}_str helpers"), we need an option for bpf_trace_printk() as well to fix it.

Similarly as with the helpers, force users to make an explicit choice by adding %pks and %pus specifier to bpf_trace_printk() which will then pick the corresponding strncpy_from_unsafe*() variant to perform the access under KERNEL_DS or USER_DS. The %pk* (kernel specifier) and %pu* (user specifier) can later also be extended for other objects aside strings that are probed and printed under tracing, and reused out of other facilities like bpf_seq_printf() or BTF based type printing.

Existing behavior of %s for current users is still kept working for archs where it is not broken and therefore gated through CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE. For archs not having this property we fall-back to pick probing under KERNEL_DS as a sensible default.

Fixes 8d3b7dce8622 ("bpf: add support for %s specifier to bpf_trace_printk()")
Cc Masami Hiramatsu <[email protected]>
Cc Brendan Gregg <[email protected]>
Documentation/core-api/printk-formats.rst +14 -0
index 8ebe46b1af39..5dfcc4592b23 100644
@@ -112,6 +112,20 @@ used when printing stack backtraces. The specifier takes into
 consideration the effect of compiler optimisations which may occur
 when tail-calls are used and marked with the noreturn GCC attribute.
 
+Probed Pointers from BPF / tracing
+----------------------------------
+
+::
+
+	%pks	kernel string
+	%pus	user string
+
+The ``k`` and ``u`` specifiers are used for printing prior probed memory from
+either kernel memory (k) or user memory (u). The subsequent ``s`` specifier
+results in printing a string. For direct use in regular vsnprintf() the (k)
+and (u) annotation is ignored, however, when used out of BPF's bpf_trace_printk(),
+for example, it reads the memory it is pointing to without faulting.
+
 Kernel Pointers
 ---------------
 
kernel/trace/bpf_trace.c +62 -32

125 lines not on this page · raw message

lib/vsprintf.c +12 -0
index 7c488a1ce318..532b6606a18a 100644
@@ -2168,6 +2168,10 @@ char *fwnode_string(char *buf, char *end, struct fwnode_handle *fwnode,
  *		f full name
  *		P node name, including a possible unit address
  * - 'x' For printing the address. Equivalent to "%lx".
+ * - '[ku]s' For a BPF/tracing related format specifier, e.g. used out of
+ *           bpf_trace_printk() where [ku] prefix specifies either kernel (k)
+ *           or user (u) memory to probe, and:
+ *              s a string, equivalent to "%s" on direct vsnprintf() use
  *
  * ** When making changes please also update:
  *	Documentation/core-api/printk-formats.rst
@@ -2251,6 +2255,14 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 		if (!IS_ERR(ptr))
 			break;
 		return err_ptr(buf, end, ptr, spec);
+	case 'u':
+	case 'k':
+		switch (fmt[1]) {
+		case 's':
+			return string(buf, end, ptr, spec);
+		default:
+			return error_string(buf, end, "(einval)", spec);
+		}
 	}
 
 	/* default is to _not_ leak addresses, hash before printing */
maintainer
#1 lore raw

Hey Greg,

Quoted 10 lines · On 5/18/20 5:54 PM, [email protected] wrote:

The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

Quoted 7 lines

------------------ original commit in Linus's tree ------------------

From b2a5212fb634561bb734c6356904e37f6665b955 Mon Sep 17 00:00:00 2001
From: Daniel Borkmann <[email protected]>
Date: Fri, 15 May 2020 12:11:18 +0200
Subject: [PATCH] bpf: Restrict bpf_trace_printk()'s %s usage and add %pks,
%pus specifier

The whole series related to this commit is:

- 0ebeea8ca8a4 ("bpf: Restrict bpf_probe_read{, str}() only to archs where they work")
- 47cc0ed574ab ("bpf: Add bpf_probe_read_{user, kernel}_str() to do_refine_retval_range")
- b2a5212fb634 ("bpf: Restrict bpf_trace_printk()'s %s usage and add %pks, %pus specifier")

It would only apply to 5.6.y stable branch, but not older ones.

Thanks,
Daniel

author maintainer
#2 lore raw
Quoted 21 lines · Daniel Borkmann

Hey Greg,

Quoted 10 lines · On 5/18/20 5:54 PM, [email protected] wrote:

The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

Quoted 7 lines

------------------ original commit in Linus's tree ------------------

From b2a5212fb634561bb734c6356904e37f6665b955 Mon Sep 17 00:00:00 2001
From: Daniel Borkmann <[email protected]>
Date: Fri, 15 May 2020 12:11:18 +0200
Subject: [PATCH] bpf: Restrict bpf_trace_printk()'s %s usage and add %pks,
%pus specifier

Quoted 7 lines

The whole series related to this commit is:

- 0ebeea8ca8a4 ("bpf: Restrict bpf_probe_read{, str}() only to archs where they work")
- 47cc0ed574ab ("bpf: Add bpf_probe_read_{user, kernel}_str() to do_refine_retval_range")
- b2a5212fb634 ("bpf: Restrict bpf_trace_printk()'s %s usage and add %pks, %pus specifier")

It would only apply to 5.6.y stable branch, but not older ones.

Thanks for the information, only the last patch was queued up for 5.6.y, I'll queue the other two for the next round after this one.

greg k-h