On 5/14/20 9:16 AM, Daniel Borkmann wrote:
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
%psK and %psU 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.
In bpf_trace_printk(), we only print strings.
In bpf-next bpf_iter bpf_seq_printf() helper, introduced by
commit 492e639f0c22 ("bpf: Add bpf_seq_printf and bpf_seq_write
helpers"), print strings and ip addresses %p{i,I}{4,6}.
Alan in
https://lore.kernel.org/bpf/alpine.LRH.2.21.2005141738050.23867@localhost/T
proposed BTF based type printing with a new format specifier
%pT, which potentially will be used in bpf_trace_printk() and
bpf_seq_printf().
In the future, we may want to support more %p<...> format in these
helpers. I am wondering whether we can have generic way so we only need
to change lib/vsprintf.c once.
Maybe using %pk<...> to specify the kernel address and %pu<...> to
specify user address space. In the above example, we will have
%pks, %pus, %pki4 or %pui4, etc. Does this make sense?
Existing %s for legacy users is still kept working for archs where it is not
broken and therefore gated through CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE.
Fixes: 8d3b7dce8622 ("bpf: add support for %s specifier to bpf_trace_printk()")
Reported-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Reported-by: Christoph Hellwig <hch@xxxxxx>
Signed-off-by: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
Cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
Cc: Brendan Gregg <brendan.d.gregg@xxxxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxx>
---
Documentation/core-api/printk-formats.rst | 14 ++++
kernel/trace/bpf_trace.c | 92 +++++++++++++++--------
lib/vsprintf.c | 7 +-
3 files changed, 81 insertions(+), 32 deletions(-)
diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 8ebe46b1af39..76b5f4f265cb 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -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 Strings from BPF
+-----------------------
+
+::
+
+ %psK kernel_string
+ %psU user_string
+
+The ``sK`` and ``sU`` specifiers are used for printing a string from probed
+memory. From regular vsnprintf(), they are equivalent to ``%s``, however,
+when used out of BPF's bpf_trace_printk() it reads a string of up to 64 bytes
+in memory without faulting. For ``K`` specifier, the string is probed out of
+kernel memory whereas for ``U`` specifier, it is probed out of user memory.
+
Kernel Pointers
---------------
[...]
