linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/6] tracing/probes: uaccess: Add support user-space access
@ 2019-02-28 12:30 Masami Hiramatsu
  2019-02-28 12:30 ` [PATCH v4 1/6] uaccess: Add user_access_ok() Masami Hiramatsu
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2019-02-28 12:30 UTC (permalink / raw)
  To: Steven Rostedt, Linus Torvalds
  Cc: mhiramat, linux-kernel, Andy Lutomirski, Ingo Molnar,
	Andrew Morton, Changbin Du, Jann Horn, Kees Cook,
	Andy Lutomirski, Alexei Starovoitov, Nadav Amit, Peter Zijlstra,
	Joel Fernandes

Hi,

Here is the v4 series of probe-event to support user-space access.
This version I fixed some issues pointed out by Steve (Thanks!)
and add a ftrace testcase for this feature.

- [4/6]: Add Steve's Ack
- [5/6]: Fix a bug and documentation
- [6/6]: New: add a new testcase

====
Kprobe event user-space memory access features:

For user-space access extension, this series adds 2 features,
"ustring" type and user-space dereference syntax. "ustring" is
used for recording a null-terminated string in user-space from
kprobe events.

"ustring" type is easy, it is able to use instead of "string"
type, so if you want to record a user-space string via
"__user char *", you can use ustring type instead of string.
For example,

echo 'p do_sys_open path=+0($arg2):ustring' >> kprobe_events

will record the path string from user-space.

The user-space dereference syntax is also simple. Thi just
adds 'u' prefix before an offset value.

   +|-u<OFFSET>(<FETCHARG>)

e.g. +u8(%ax), +u0(+0(%si))

This is more generic. If you want to refer the variable in user-
space from its address or access a field in data structure in
user-space, you need to use this.

For example, if you probe do_sched_setscheduler(pid, policy,
param) and record param->sched_priority, you can add new
probe as below;
    
   p do_sched_setscheduler priority=+u0($arg3)

Actually, with this feature, "ustring" type is not absolutely
necessary, because these are same meanings.

  +0($arg2):ustring == +u0($arg2):string

Note that kprobe event provides these methods, but it doesn't
change it from kernel to user automatically because we do not
know whether the given address is in userspace or kernel on
some arch.

For perf-probe, we can add some attribute for each argument
which indicate that the variable in user space. If gcc/clang
supports debuginfo for __user (address_space attribute),
perf-probe can support it and automatically choose the
correct dereference method. 

Thank you,

---

Masami Hiramatsu (5):
      uaccess: Use user_access_ok() in user_access_begin()
      uaccess: Add non-pagefault user-space read functions
      tracing/probe: Add ustring type for user-space string
      tracing/probe: Support user-space dereference
      selftests/ftrace: Add user-memory access syntax testcase

Peter Zijlstra (1):
      uaccess: Add user_access_ok()


 Documentation/trace/kprobetrace.rst                |   28 +++++-
 Documentation/trace/uprobetracer.rst               |    9 +-
 arch/x86/include/asm/uaccess.h                     |   10 ++
 include/linux/uaccess.h                            |   33 +++++++
 kernel/trace/trace.c                               |    7 +
 kernel/trace/trace_kprobe.c                        |   43 +++++++++
 kernel/trace/trace_probe.c                         |   36 ++++++--
 kernel/trace/trace_probe.h                         |    3 +
 kernel/trace/trace_probe_tmpl.h                    |   37 +++++++-
 kernel/trace/trace_uprobe.c                        |   19 ++++
 mm/maccess.c                                       |   94 +++++++++++++++++++-
 .../ftrace/test.d/kprobe/kprobe_args_user.tc       |   31 +++++++
 12 files changed, 317 insertions(+), 33 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v4 1/6] uaccess: Add user_access_ok()
  2019-02-28 12:30 [PATCH v4 0/6] tracing/probes: uaccess: Add support user-space access Masami Hiramatsu
@ 2019-02-28 12:30 ` Masami Hiramatsu
  2019-02-28 12:31 ` [PATCH v4 2/6] uaccess: Use user_access_ok() in user_access_begin() Masami Hiramatsu
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2019-02-28 12:30 UTC (permalink / raw)
  To: Steven Rostedt, Linus Torvalds
  Cc: mhiramat, linux-kernel, Andy Lutomirski, Ingo Molnar,
	Andrew Morton, Changbin Du, Jann Horn, Kees Cook,
	Andy Lutomirski, Alexei Starovoitov, Nadav Amit, Peter Zijlstra,
	Joel Fernandes

From: Peter Zijlstra <peterz@infradead.org>

Add user_access_ok() macro which ensures current context
is user context, or explicitly do set_fs(USER_DS).

This function is very much like access_ok(), except it (may)
have different context validation. In general we must be
very careful when using this.

Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/x86/include/asm/uaccess.h |    8 +++++++-
 include/linux/uaccess.h        |   18 ++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 780f2b42c8ef..3125d129d3b6 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -92,12 +92,18 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un
  * checks that the pointer is in the user space range - after calling
  * this function, memory access functions may still return -EFAULT.
  */
-#define access_ok(addr, size)					\
+#define access_ok(addr, size)						\
 ({									\
 	WARN_ON_IN_IRQ();						\
 	likely(!__range_not_ok(addr, size, user_addr_max()));		\
 })
 
+#define user_access_ok(addr, size)					\
+({									\
+	WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS));			\
+	likely(!__range_not_ok(addr, size, user_addr_max()));		\
+})
+
 /*
  * These are the main single-value transfer routines.  They automatically
  * use the right size if we just have the right pointer type.
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 37b226e8df13..bf762689658b 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -10,6 +10,24 @@
 
 #include <asm/uaccess.h>
 
+/**
+ * user_access_ok: Checks if a user space pointer is valid
+ * @addr: User space pointer to start of block to check
+ * @size: Size of block to check
+ *
+ * Context: User context or explicit set_fs(USER_DS).
+ *
+ * This function is very much like access_ok(), except it (may) have different
+ * context validation. In general we must be very careful when using this.
+ */
+#ifndef user_access_ok
+#define user_access_ok(addr, size)					\
+({									\
+	WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS));			\
+	access_ok(addr, size);						\
+})
+#endif
+
 /*
  * Architectures should provide two primitives (raw_copy_{to,from}_user())
  * and get rid of their private instances of copy_{to,from}_user() and


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 2/6] uaccess: Use user_access_ok() in user_access_begin()
  2019-02-28 12:30 [PATCH v4 0/6] tracing/probes: uaccess: Add support user-space access Masami Hiramatsu
  2019-02-28 12:30 ` [PATCH v4 1/6] uaccess: Add user_access_ok() Masami Hiramatsu
@ 2019-02-28 12:31 ` Masami Hiramatsu
  2019-03-03 12:48   ` kbuild test robot
  2019-02-28 12:31 ` [PATCH v4 3/6] uaccess: Add non-pagefault user-space read functions Masami Hiramatsu
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Masami Hiramatsu @ 2019-02-28 12:31 UTC (permalink / raw)
  To: Steven Rostedt, Linus Torvalds
  Cc: mhiramat, linux-kernel, Andy Lutomirski, Ingo Molnar,
	Andrew Morton, Changbin Du, Jann Horn, Kees Cook,
	Andy Lutomirski, Alexei Starovoitov, Nadav Amit, Peter Zijlstra,
	Joel Fernandes

Use user_access_ok() instead of access_ok() in user_access_begin()
to validate the access context is user. This also allow us to
use (generic) strncpy_from_user() and strnlen_user() in atomic
state with setting USER_DS and disable pagefaults.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/x86/include/asm/uaccess.h |    2 +-
 include/linux/uaccess.h        |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 3125d129d3b6..3b7502a34114 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -715,7 +715,7 @@ extern struct movsl_mask {
  */
 static __must_check inline bool user_access_begin(const void __user *ptr, size_t len)
 {
-	if (unlikely(!access_ok(ptr,len)))
+	if (unlikely(!user_access_ok(ptr, len)))
 		return 0;
 	__uaccess_begin_nospec();
 	return 1;
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index bf762689658b..1afd9dfabe67 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -282,7 +282,7 @@ extern long strncpy_from_unsafe(char *dst, const void *unsafe_addr, long count);
 	probe_kernel_read(&retval, addr, sizeof(retval))
 
 #ifndef user_access_begin
-#define user_access_begin(ptr,len) access_ok(ptr, len)
+#define user_access_begin(ptr, len) user_access_ok(ptr, len)
 #define user_access_end() do { } while (0)
 #define unsafe_get_user(x, ptr, err) do { if (unlikely(__get_user(x, ptr))) goto err; } while (0)
 #define unsafe_put_user(x, ptr, err) do { if (unlikely(__put_user(x, ptr))) goto err; } while (0)


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 3/6] uaccess: Add non-pagefault user-space read functions
  2019-02-28 12:30 [PATCH v4 0/6] tracing/probes: uaccess: Add support user-space access Masami Hiramatsu
  2019-02-28 12:30 ` [PATCH v4 1/6] uaccess: Add user_access_ok() Masami Hiramatsu
  2019-02-28 12:31 ` [PATCH v4 2/6] uaccess: Use user_access_ok() in user_access_begin() Masami Hiramatsu
@ 2019-02-28 12:31 ` Masami Hiramatsu
  2019-02-28 12:32 ` [PATCH v4 4/6] tracing/probe: Add ustring type for user-space string Masami Hiramatsu
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2019-02-28 12:31 UTC (permalink / raw)
  To: Steven Rostedt, Linus Torvalds
  Cc: mhiramat, linux-kernel, Andy Lutomirski, Ingo Molnar,
	Andrew Morton, Changbin Du, Jann Horn, Kees Cook,
	Andy Lutomirski, Alexei Starovoitov, Nadav Amit, Peter Zijlstra,
	Joel Fernandes

Add probe_user_read() and strncpy_from_unsafe_user() which
allows caller to access user-space in IRQ context.

Current probe_kernel_read() and strncpy_from_unsafe() are
not available for user-space memory, because it sets
KERNEL_DS while accessing data. On some arch, user address
space and kernel address space can be co-exist, but others
can not. In that case, setting KERNEL_DS means given
address is treated as a kernel address space.

To access user-space memory without pagefault, we need
these new functions which sets USER_DS while accessing
the data.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
  Changes in v3:
   - Use user_access_ok() for probe_user_read().
  Changes in v2:
   - Simplify strncpy_from_unsafe_user() using strncpy_from_user()
     according to Linus's suggestion.
   - Simplify probe_user_read() not using intermediate function.
---
 include/linux/uaccess.h |   13 +++++++
 mm/maccess.c            |   94 ++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 101 insertions(+), 6 deletions(-)

diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 1afd9dfabe67..598a96210d04 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -258,6 +258,17 @@ extern long probe_kernel_read(void *dst, const void *src, size_t size);
 extern long __probe_kernel_read(void *dst, const void *src, size_t size);
 
 /*
+ * probe_user_read(): safely attempt to read from a location in user space
+ * @dst: pointer to the buffer that shall take the data
+ * @src: address to read from
+ * @size: size of the data chunk
+ *
+ * Safely read from address @src to the buffer at @dst.  If a kernel fault
+ * happens, handle that and return -EFAULT.
+ */
+extern long probe_user_read(void *dst, const void __user *src, size_t size);
+
+/*
  * probe_kernel_write(): safely attempt to write to a location
  * @dst: address to write to
  * @src: pointer to the data that shall be written
@@ -270,6 +281,8 @@ extern long notrace probe_kernel_write(void *dst, const void *src, size_t size);
 extern long notrace __probe_kernel_write(void *dst, const void *src, size_t size);
 
 extern long strncpy_from_unsafe(char *dst, const void *unsafe_addr, long count);
+extern long strncpy_from_unsafe_user(char *dst, const void __user *unsafe_addr,
+				     long count);
 
 /**
  * probe_kernel_address(): safely attempt to read from a location
diff --git a/mm/maccess.c b/mm/maccess.c
index ec00be51a24f..b24df3e731c8 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -5,8 +5,20 @@
 #include <linux/mm.h>
 #include <linux/uaccess.h>
 
+static __always_inline long
+probe_read_common(void *dst, const void __user *src, size_t size)
+{
+	long ret;
+
+	pagefault_disable();
+	ret = __copy_from_user_inatomic(dst, src, size);
+	pagefault_enable();
+
+	return ret ? -EFAULT : 0;
+}
+
 /**
- * probe_kernel_read(): safely attempt to read from a location
+ * probe_kernel_read(): safely attempt to read from a kernel-space location
  * @dst: pointer to the buffer that shall take the data
  * @src: address to read from
  * @size: size of the data chunk
@@ -29,17 +41,47 @@ long __probe_kernel_read(void *dst, const void *src, size_t size)
 	mm_segment_t old_fs = get_fs();
 
 	set_fs(KERNEL_DS);
-	pagefault_disable();
-	ret = __copy_from_user_inatomic(dst,
-			(__force const void __user *)src, size);
-	pagefault_enable();
+	ret = probe_read_common(dst, (__force const void __user *)src, size);
 	set_fs(old_fs);
 
-	return ret ? -EFAULT : 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(probe_kernel_read);
 
 /**
+ * probe_user_read(): safely attempt to read from a user-space location
+ * @dst: pointer to the buffer that shall take the data
+ * @src: address to read from. This must be a user address.
+ * @size: size of the data chunk
+ *
+ * Safely read from user address @src to the buffer at @dst. If a kernel fault
+ * happens, handle that and return -EFAULT.
+ */
+
+long __weak probe_user_read(void *dst, const void __user *src, size_t size)
+    __attribute__((alias("__probe_user_read")));
+
+long __probe_user_read(void *dst, const void __user *src, size_t size)
+{
+	long ret;
+	mm_segment_t old_fs = get_fs();
+
+	/*
+	 * Since this can be called in IRQ context, we carefully set the
+	 * USER_DS and use user_access_ok() which checks segment setting
+	 * instead of task context.
+	 */
+	set_fs(USER_DS);
+	if (!user_access_ok(src, size))
+		ret = -EFAULT;
+	else
+		ret = probe_read_common(dst, src, size);
+	set_fs(old_fs);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(probe_user_read);
+
+/**
  * probe_kernel_write(): safely attempt to write to a location
  * @dst: address to write to
  * @src: pointer to the data that shall be written
@@ -66,6 +108,7 @@ long __probe_kernel_write(void *dst, const void *src, size_t size)
 }
 EXPORT_SYMBOL_GPL(probe_kernel_write);
 
+
 /**
  * strncpy_from_unsafe: - Copy a NUL terminated string from unsafe address.
  * @dst:   Destination address, in kernel space.  This buffer must be at
@@ -105,3 +148,42 @@ long strncpy_from_unsafe(char *dst, const void *unsafe_addr, long count)
 
 	return ret ? -EFAULT : src - unsafe_addr;
 }
+
+/**
+ * strncpy_from_unsafe_user: - Copy a NUL terminated string from unsafe user
+ *				address.
+ * @dst:   Destination address, in kernel space.  This buffer must be at
+ *         least @count bytes long.
+ * @unsafe_addr: Unsafe user address.
+ * @count: Maximum number of bytes to copy, including the trailing NUL.
+ *
+ * Copies a NUL-terminated string from unsafe user address to kernel buffer.
+ *
+ * On success, returns the length of the string INCLUDING the trailing NUL.
+ *
+ * If access fails, returns -EFAULT (some data may have been copied
+ * and the trailing NUL added).
+ *
+ * If @count is smaller than the length of the string, copies @count-1 bytes,
+ * sets the last byte of @dst buffer to NUL and returns @count.
+ */
+long strncpy_from_unsafe_user(char *dst, const void __user *src, long count)
+{
+	mm_segment_t old_fs = get_fs();
+	long ret;
+
+	if (unlikely(count <= 0))
+		return 0;
+
+	set_fs(USER_DS);
+	pagefault_disable();
+	ret = strncpy_from_user(dst, src, count);
+	pagefault_enable();
+	set_fs(old_fs);
+	if (ret >= count) {
+		ret = count;
+		dst[ret - 1] = '\0';
+	} else if (ret > 0)
+		ret++;
+	return ret;
+}


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 4/6] tracing/probe: Add ustring type for user-space string
  2019-02-28 12:30 [PATCH v4 0/6] tracing/probes: uaccess: Add support user-space access Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2019-02-28 12:31 ` [PATCH v4 3/6] uaccess: Add non-pagefault user-space read functions Masami Hiramatsu
@ 2019-02-28 12:32 ` Masami Hiramatsu
  2019-02-28 12:32 ` [PATCH v4 5/6] tracing/probe: Support user-space dereference Masami Hiramatsu
  2019-02-28 12:33 ` [PATCH v4 6/6] selftests/ftrace: Add user-memory access syntax testcase Masami Hiramatsu
  5 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2019-02-28 12:32 UTC (permalink / raw)
  To: Steven Rostedt, Linus Torvalds
  Cc: mhiramat, linux-kernel, Andy Lutomirski, Ingo Molnar,
	Andrew Morton, Changbin Du, Jann Horn, Kees Cook,
	Andy Lutomirski, Alexei Starovoitov, Nadav Amit, Peter Zijlstra,
	Joel Fernandes

Add "ustring" type for fetching user-space string from kprobe event.
User can specify ustring type at uprobe event, and it is same as
"string" for uprobe.

Note that probe-event provides this option but it doesn't choose the
correct type automatically since we have not way to decide the address
is in user-space or not on some arch (and on some other arch, you can
fetch the string by "string" type). So user must carefully check the
target code (e.g. if you see __user on the target variable) and
use this new type.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

---
 Changes in v2:
 - Use strnlen_user() instead of open code for fetch_store_strlen_user().
---
 Documentation/trace/kprobetrace.rst |    9 +++++++--
 kernel/trace/trace.c                |    2 +-
 kernel/trace/trace_kprobe.c         |   37 +++++++++++++++++++++++++++++++++++
 kernel/trace/trace_probe.c          |   14 ++++++++++---
 kernel/trace/trace_probe.h          |    1 +
 kernel/trace/trace_probe_tmpl.h     |   14 ++++++++++++-
 kernel/trace/trace_uprobe.c         |   12 +++++++++++
 7 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
index 235ce2ab131a..a3ac7c9ac242 100644
--- a/Documentation/trace/kprobetrace.rst
+++ b/Documentation/trace/kprobetrace.rst
@@ -55,7 +55,8 @@ Synopsis of kprobe_events
   NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
   FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
 		  (u8/u16/u32/u64/s8/s16/s32/s64), hexadecimal types
-		  (x8/x16/x32/x64), "string" and bitfield are supported.
+		  (x8/x16/x32/x64), "string", "ustring" and bitfield
+		  are supported.
 
   (\*1) only for the probe on function entry (offs == 0).
   (\*2) only for return probe.
@@ -77,7 +78,11 @@ apply it to registers/stack-entries etc. (for example, '$stack1:x8[8]' is
 wrong, but '+8($stack):x8[8]' is OK.)
 String type is a special type, which fetches a "null-terminated" string from
 kernel space. This means it will fail and store NULL if the string container
-has been paged out.
+has been paged out. "ustring" type is an alternative of string for user-space.
+Note that kprobe-event provides string/ustring types, but doesn't change it
+automatically. So user has to decide if the targe string in kernel or in user
+space carefully. On some arch, if you choose wrong one, it always fails to
+record string data.
 The string array type is a bit different from other types. For other base
 types, <base-type>[1] is equal to <base-type> (e.g. +0(%di):x32[1] is same
 as +0(%di):x32.) But string[1] is not equal to string. The string type itself
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c4238b441624..4cacbb0e1538 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4643,7 +4643,7 @@ static const char readme_msg[] =
 	"\t           $stack<index>, $stack, $retval, $comm\n"
 #endif
 	"\t     type: s8/16/32/64, u8/16/32/64, x8/16/32/64, string, symbol,\n"
-	"\t           b<bit-width>@<bit-offset>/<container-size>,\n"
+	"\t           b<bit-width>@<bit-offset>/<container-size>, ustring,\n"
 	"\t           <type>\\[<array-size>\\]\n"
 #ifdef CONFIG_HIST_TRIGGERS
 	"\t    field: <stype> <name>;\n"
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 9eaf07f99212..8b04cae5fd86 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -872,6 +872,22 @@ fetch_store_strlen(unsigned long addr)
 	return (ret < 0) ? ret : len;
 }
 
+/* Return the length of string -- including null terminal byte */
+static nokprobe_inline int
+fetch_store_strlen_user(unsigned long addr)
+{
+	mm_segment_t old_fs = get_fs();
+	int ret;
+
+	set_fs(USER_DS);
+	pagefault_disable();
+	ret = strnlen_user((__force const char __user *)addr, MAX_STRING_SIZE);
+	pagefault_enable();
+	set_fs(old_fs);
+
+	return ret;
+}
+
 /*
  * Fetch a null-terminated string. Caller MUST set *(u32 *)buf with max
  * length and relative data location.
@@ -896,6 +912,27 @@ fetch_store_string(unsigned long addr, void *dest, void *base)
 	return ret;
 }
 
+/*
+ * Fetch a null-terminated string from user. Caller MUST set *(u32 *)buf
+ * with max length and relative data location.
+ */
+static nokprobe_inline int
+fetch_store_string_user(unsigned long addr, void *dest, void *base)
+{
+	int maxlen = get_loc_len(*(u32 *)dest);
+	u8 *dst = get_loc_data(dest, base);
+	long ret;
+
+	if (unlikely(!maxlen))
+		return -ENOMEM;
+	ret = strncpy_from_unsafe_user(dst, (__force const void __user *)addr,
+				       maxlen);
+
+	if (ret >= 0)
+		*(u32 *)dest = make_data_loc(ret, (void *)dst - base);
+	return ret;
+}
+
 static nokprobe_inline int
 probe_mem_read(void *dest, void *src, size_t size)
 {
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 9962cb5da8ac..a7012de37a00 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -73,6 +73,8 @@ static const struct fetch_type probe_fetch_types[] = {
 	/* Special types */
 	__ASSIGN_FETCH_TYPE("string", string, string, sizeof(u32), 1,
 			    "__data_loc char[]"),
+	__ASSIGN_FETCH_TYPE("ustring", string, string, sizeof(u32), 1,
+			    "__data_loc char[]"),
 	/* Basic types */
 	ASSIGN_FETCH_TYPE(u8,  u8,  0),
 	ASSIGN_FETCH_TYPE(u16, u16, 0),
@@ -440,7 +442,8 @@ static int traceprobe_parse_probe_arg_body(char *arg, ssize_t *size,
 		goto fail;
 
 	/* Store operation */
-	if (!strcmp(parg->type->name, "string")) {
+	if (!strcmp(parg->type->name, "string") ||
+	    !strcmp(parg->type->name, "ustring")) {
 		if (code->op != FETCH_OP_DEREF && code->op != FETCH_OP_IMM &&
 		    code->op != FETCH_OP_COMM) {
 			pr_info("string only accepts memory or address.\n");
@@ -459,7 +462,11 @@ static int traceprobe_parse_probe_arg_body(char *arg, ssize_t *size,
 				goto fail;
 			}
 		}
-		code->op = FETCH_OP_ST_STRING;	/* In DEREF case, replace it */
+		/* If op == DEREF, replace it with STRING */
+		if (!strcmp(parg->type->name, "ustring"))
+			code->op = FETCH_OP_ST_USTRING;
+		else
+			code->op = FETCH_OP_ST_STRING;
 		code->size = parg->type->size;
 		parg->dynamic = true;
 	} else if (code->op == FETCH_OP_DEREF) {
@@ -484,7 +491,8 @@ static int traceprobe_parse_probe_arg_body(char *arg, ssize_t *size,
 	/* Loop(Array) operation */
 	if (parg->count) {
 		if (scode->op != FETCH_OP_ST_MEM &&
-		    scode->op != FETCH_OP_ST_STRING) {
+		    scode->op != FETCH_OP_ST_STRING &&
+		    scode->op != FETCH_OP_ST_USTRING) {
 			pr_info("array only accepts memory or address\n");
 			ret = -EINVAL;
 			goto fail;
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 8a63f8bc01bc..cf4ba8bbb841 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -95,6 +95,7 @@ enum fetch_op {
 	FETCH_OP_ST_RAW,	/* Raw: .size */
 	FETCH_OP_ST_MEM,	/* Mem: .offset, .size */
 	FETCH_OP_ST_STRING,	/* String: .offset, .size */
+	FETCH_OP_ST_USTRING,	/* User String: .offset, .size */
 	// Stage 4 (modify) op
 	FETCH_OP_MOD_BF,	/* Bitfield: .basesize, .lshift, .rshift */
 	// Stage 5 (loop) op
diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h
index 4737bb8c07a3..7526f6f8d7b0 100644
--- a/kernel/trace/trace_probe_tmpl.h
+++ b/kernel/trace/trace_probe_tmpl.h
@@ -59,6 +59,9 @@ process_fetch_insn(struct fetch_insn *code, struct pt_regs *regs,
 static nokprobe_inline int fetch_store_strlen(unsigned long addr);
 static nokprobe_inline int
 fetch_store_string(unsigned long addr, void *dest, void *base);
+static nokprobe_inline int fetch_store_strlen_user(unsigned long addr);
+static nokprobe_inline int
+fetch_store_string_user(unsigned long addr, void *dest, void *base);
 static nokprobe_inline int
 probe_mem_read(void *dest, void *src, size_t size);
 
@@ -91,6 +94,10 @@ process_fetch_insn_bottom(struct fetch_insn *code, unsigned long val,
 			ret += fetch_store_strlen(val + code->offset);
 			code++;
 			goto array;
+		} else if (code->op == FETCH_OP_ST_USTRING) {
+			ret += fetch_store_strlen_user(val + code->offset);
+			code++;
+			goto array;
 		} else
 			return -EILSEQ;
 	}
@@ -106,6 +113,10 @@ process_fetch_insn_bottom(struct fetch_insn *code, unsigned long val,
 		loc = *(u32 *)dest;
 		ret = fetch_store_string(val + code->offset, dest, base);
 		break;
+	case FETCH_OP_ST_USTRING:
+		loc = *(u32 *)dest;
+		ret = fetch_store_string_user(val + code->offset, dest, base);
+		break;
 	default:
 		return -EILSEQ;
 	}
@@ -123,7 +134,8 @@ process_fetch_insn_bottom(struct fetch_insn *code, unsigned long val,
 		total += ret;
 		if (++i < code->param) {
 			code = s3;
-			if (s3->op != FETCH_OP_ST_STRING) {
+			if (s3->op != FETCH_OP_ST_STRING &&
+			    s3->op != FETCH_OP_ST_USTRING) {
 				dest += s3->size;
 				val += s3->size;
 				goto stage3;
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 9bde07c06362..92facae8c3d8 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -173,6 +173,12 @@ fetch_store_string(unsigned long addr, void *dest, void *base)
 	return ret;
 }
 
+static nokprobe_inline int
+fetch_store_string_user(unsigned long addr, void *dest, void *base)
+{
+	return fetch_store_string(addr, dest, base);
+}
+
 /* Return the length of string -- including null terminal byte */
 static nokprobe_inline int
 fetch_store_strlen(unsigned long addr)
@@ -185,6 +191,12 @@ fetch_store_strlen(unsigned long addr)
 	return (len > MAX_STRING_SIZE) ? 0 : len;
 }
 
+static nokprobe_inline int
+fetch_store_strlen_user(unsigned long addr)
+{
+	return fetch_store_strlen(addr);
+}
+
 static unsigned long translate_user_vaddr(unsigned long file_offset)
 {
 	unsigned long base_addr;


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 5/6] tracing/probe: Support user-space dereference
  2019-02-28 12:30 [PATCH v4 0/6] tracing/probes: uaccess: Add support user-space access Masami Hiramatsu
                   ` (3 preceding siblings ...)
  2019-02-28 12:32 ` [PATCH v4 4/6] tracing/probe: Add ustring type for user-space string Masami Hiramatsu
@ 2019-02-28 12:32 ` Masami Hiramatsu
  2019-02-28 12:33 ` [PATCH v4 6/6] selftests/ftrace: Add user-memory access syntax testcase Masami Hiramatsu
  5 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2019-02-28 12:32 UTC (permalink / raw)
  To: Steven Rostedt, Linus Torvalds
  Cc: mhiramat, linux-kernel, Andy Lutomirski, Ingo Molnar,
	Andrew Morton, Changbin Du, Jann Horn, Kees Cook,
	Andy Lutomirski, Alexei Starovoitov, Nadav Amit, Peter Zijlstra,
	Joel Fernandes

Support user-space dereference syntax for probe event arguments
to dereference the data-structure or array in user-space.

The syntax is just adding 'u' before an offset value.

 +|-u<OFFSET>(<FETCHARG>)

e.g. +u8(%ax), +u0(+0(%si))

For example, if you probe do_sched_setscheduler(pid, policy,
param) and record param->sched_priority, you can add new
probe as below;

 p do_sched_setscheduler priority=+u0($arg3)

Note that kprobe event provides this and it doesn't change the
dereference method automatically because we do not know whether
the given address is in userspace or kernel on some arch.

So as same as "ustring", this is an option for user, who has to
carefully choose the dereference method.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Changes in v4:
  - Fix a bug for parsing argument and simplify code.
  - Fix documents accroding to Steve's comment.
 Changes in v3:
  - Add a section for user memory access to the document.
---
 Documentation/trace/kprobetrace.rst  |   27 ++++++++++++++++++++++-----
 Documentation/trace/uprobetracer.rst |    9 +++++----
 kernel/trace/trace.c                 |    5 +++--
 kernel/trace/trace_kprobe.c          |    6 ++++++
 kernel/trace/trace_probe.c           |   24 ++++++++++++++++++------
 kernel/trace/trace_probe.h           |    2 ++
 kernel/trace/trace_probe_tmpl.h      |   23 ++++++++++++++++++-----
 kernel/trace/trace_uprobe.c          |    7 +++++++
 8 files changed, 81 insertions(+), 22 deletions(-)

diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
index a3ac7c9ac242..0639ae492932 100644
--- a/Documentation/trace/kprobetrace.rst
+++ b/Documentation/trace/kprobetrace.rst
@@ -51,7 +51,7 @@ Synopsis of kprobe_events
   $argN		: Fetch the Nth function argument. (N >= 1) (\*1)
   $retval	: Fetch return value.(\*2)
   $comm		: Fetch current task comm.
-  +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(\*3)
+  +|-[u]OFFS(FETCHARG) : Fetch memory at FETCHARG +|- OFFS address.(\*3)(\*4)
   NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
   FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
 		  (u8/u16/u32/u64/s8/s16/s32/s64), hexadecimal types
@@ -61,6 +61,7 @@ Synopsis of kprobe_events
   (\*1) only for the probe on function entry (offs == 0).
   (\*2) only for return probe.
   (\*3) this is useful for fetching a field of data structures.
+  (\*4) "u" means user-space dereference. See :ref:`user_mem_access`.
 
 Types
 -----
@@ -79,10 +80,7 @@ wrong, but '+8($stack):x8[8]' is OK.)
 String type is a special type, which fetches a "null-terminated" string from
 kernel space. This means it will fail and store NULL if the string container
 has been paged out. "ustring" type is an alternative of string for user-space.
-Note that kprobe-event provides string/ustring types, but doesn't change it
-automatically. So user has to decide if the targe string in kernel or in user
-space carefully. On some arch, if you choose wrong one, it always fails to
-record string data.
+See :ref:`user_mem_access` for more info..
 The string array type is a bit different from other types. For other base
 types, <base-type>[1] is equal to <base-type> (e.g. +0(%di):x32[1] is same
 as +0(%di):x32.) But string[1] is not equal to string. The string type itself
@@ -97,6 +95,25 @@ Symbol type('symbol') is an alias of u32 or u64 type (depends on BITS_PER_LONG)
 which shows given pointer in "symbol+offset" style.
 For $comm, the default type is "string"; any other type is invalid.
 
+.. _user_mem_access:
+User Memory Access
+------------------
+Kprobe events supports user-space memory access. For that purpose, you can use
+either user-space dereference syntax or 'ustring' type.
+
+The user-space dereference syntax allows you to access a field of a data
+structure in user-space. This is done by adding the "u" prefix to the
+dereference syntax. For example, +u4(%si) means it will read memory from the
+address in the register %si offset by 4, and the mory is expected to be in
+user-space. You can use this for strings too, e.g. +u0(%si):string will read
+a string from the address in the register %si that is expected to be in user-
+space. 'ustring' is a shortcut way of performing the same task. That is,
++0(%si):ustring is equivalent to +u0(%si):string.
+
+Note that kprobe-event provides the user-memory access syntax but it doesn't
+use it transparently. This means if you use normal dereference or string type
+for user memory, it might fail, and always fails on some arch. So user has to
+check if the targe data is in kernel or in user space carefully.
 
 Per-Probe Event Filtering
 -------------------------
diff --git a/Documentation/trace/uprobetracer.rst b/Documentation/trace/uprobetracer.rst
index 4c3bfde2ba47..4656624a3da9 100644
--- a/Documentation/trace/uprobetracer.rst
+++ b/Documentation/trace/uprobetracer.rst
@@ -42,16 +42,17 @@ Synopsis of uprobe_tracer
    @+OFFSET	: Fetch memory at OFFSET (OFFSET from same file as PATH)
    $stackN	: Fetch Nth entry of stack (N >= 0)
    $stack	: Fetch stack address.
-   $retval	: Fetch return value.(*)
+   $retval	: Fetch return value.(\*1)
    $comm	: Fetch current task comm.
-   +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
+   +|-[u]OFFS(FETCHARG) : Fetch memory at FETCHARG +|- OFFS address.(\*2)(\*3)
    NAME=FETCHARG     : Set NAME as the argument name of FETCHARG.
    FETCHARG:TYPE     : Set TYPE as the type of FETCHARG. Currently, basic types
 		       (u8/u16/u32/u64/s8/s16/s32/s64), hexadecimal types
 		       (x8/x16/x32/x64), "string" and bitfield are supported.
 
-  (*) only for return probe.
-  (**) this is useful for fetching a field of data structures.
+  (\*1) only for return probe.
+  (\*2) this is useful for fetching a field of data structures.
+  (\*3) Unlike kprobe event, "u" prefix will just be ignored.
 
 Types
 -----
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 4cacbb0e1538..5408a82a015d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4638,10 +4638,11 @@ static const char readme_msg[] =
 	"\t     args: <name>=fetcharg[:type]\n"
 	"\t fetcharg: %<register>, @<address>, @<symbol>[+|-<offset>],\n"
 #ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
-	"\t           $stack<index>, $stack, $retval, $comm, $arg<N>\n"
+	"\t           $stack<index>, $stack, $retval, $comm, $arg<N>,\n"
 #else
-	"\t           $stack<index>, $stack, $retval, $comm\n"
+	"\t           $stack<index>, $stack, $retval, $comm,\n"
 #endif
+	"\t           +|-[u]<offset>(<fetcharg>)\n"
 	"\t     type: s8/16/32/64, u8/16/32/64, x8/16/32/64, string, symbol,\n"
 	"\t           b<bit-width>@<bit-offset>/<container-size>, ustring,\n"
 	"\t           <type>\\[<array-size>\\]\n"
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 8b04cae5fd86..c3b8564c263f 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -939,6 +939,12 @@ probe_mem_read(void *dest, void *src, size_t size)
 	return probe_kernel_read(dest, src, size);
 }
 
+static nokprobe_inline int
+probe_mem_read_user(void *dest, void *src, size_t size)
+{
+	return probe_user_read(dest, src, size);
+}
+
 /* Note that we don't verify it, since the code does not come from user space */
 static int
 process_fetch_insn(struct fetch_insn *code, struct pt_regs *regs, void *dest,
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index a7012de37a00..13593c6fe0ef 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -239,6 +239,7 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
 {
 	struct fetch_insn *code = *pcode;
 	unsigned long param;
+	int deref = FETCH_OP_DEREF;
 	long offset = 0;
 	char *tmp;
 	int ret = 0;
@@ -301,8 +302,14 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
 		break;
 
 	case '+':	/* deref memory */
-		arg++;	/* Skip '+', because kstrtol() rejects it. */
 	case '-':
+		if (arg[1] == 'u') {
+			deref = FETCH_OP_UDEREF;
+			arg[1] = arg[0];
+			arg++;
+		}
+		if (arg[0] == '+')
+			arg++;	/* Skip '+', because kstrtol() rejects it. */
 		tmp = strchr(arg, '(');
 		if (!tmp)
 			return -EINVAL;
@@ -328,7 +335,7 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
 				return -E2BIG;
 			*pcode = code;
 
-			code->op = FETCH_OP_DEREF;
+			code->op = deref;
 			code->offset = offset;
 		}
 		break;
@@ -444,13 +451,14 @@ static int traceprobe_parse_probe_arg_body(char *arg, ssize_t *size,
 	/* Store operation */
 	if (!strcmp(parg->type->name, "string") ||
 	    !strcmp(parg->type->name, "ustring")) {
-		if (code->op != FETCH_OP_DEREF && code->op != FETCH_OP_IMM &&
-		    code->op != FETCH_OP_COMM) {
+		if (code->op != FETCH_OP_DEREF && code->op != FETCH_OP_UDEREF
+		    && code->op != FETCH_OP_IMM && code->op != FETCH_OP_COMM) {
 			pr_info("string only accepts memory or address.\n");
 			ret = -EINVAL;
 			goto fail;
 		}
-		if (code->op != FETCH_OP_DEREF || parg->count) {
+		if ((code->op == FETCH_OP_IMM || code->op == FETCH_OP_COMM)
+		    || parg->count) {
 			/*
 			 * IMM and COMM is pointing actual address, those must
 			 * be kept, and if parg->count != 0, this is an array
@@ -463,7 +471,8 @@ static int traceprobe_parse_probe_arg_body(char *arg, ssize_t *size,
 			}
 		}
 		/* If op == DEREF, replace it with STRING */
-		if (!strcmp(parg->type->name, "ustring"))
+		if (!strcmp(parg->type->name, "ustring") ||
+		    code->op == FETCH_OP_UDEREF)
 			code->op = FETCH_OP_ST_USTRING;
 		else
 			code->op = FETCH_OP_ST_STRING;
@@ -472,6 +481,9 @@ static int traceprobe_parse_probe_arg_body(char *arg, ssize_t *size,
 	} else if (code->op == FETCH_OP_DEREF) {
 		code->op = FETCH_OP_ST_MEM;
 		code->size = parg->type->size;
+	} else if (code->op == FETCH_OP_UDEREF) {
+		code->op = FETCH_OP_ST_UMEM;
+		code->size = parg->type->size;
 	} else {
 		code++;
 		if (code->op != FETCH_OP_NOP) {
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index cf4ba8bbb841..a5e8b2ac2c97 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -91,9 +91,11 @@ enum fetch_op {
 	FETCH_OP_FOFFS,		/* File offset: .immediate */
 	// Stage 2 (dereference) op
 	FETCH_OP_DEREF,		/* Dereference: .offset */
+	FETCH_OP_UDEREF,	/* User-space Dereference: .offset */
 	// Stage 3 (store) ops
 	FETCH_OP_ST_RAW,	/* Raw: .size */
 	FETCH_OP_ST_MEM,	/* Mem: .offset, .size */
+	FETCH_OP_ST_UMEM,	/* Mem: .offset, .size */
 	FETCH_OP_ST_STRING,	/* String: .offset, .size */
 	FETCH_OP_ST_USTRING,	/* User String: .offset, .size */
 	// Stage 4 (modify) op
diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h
index 7526f6f8d7b0..06f2d901c4cf 100644
--- a/kernel/trace/trace_probe_tmpl.h
+++ b/kernel/trace/trace_probe_tmpl.h
@@ -64,6 +64,8 @@ static nokprobe_inline int
 fetch_store_string_user(unsigned long addr, void *dest, void *base);
 static nokprobe_inline int
 probe_mem_read(void *dest, void *src, size_t size);
+static nokprobe_inline int
+probe_mem_read_user(void *dest, void *src, size_t size);
 
 /* From the 2nd stage, routine is same */
 static nokprobe_inline int
@@ -77,14 +79,21 @@ process_fetch_insn_bottom(struct fetch_insn *code, unsigned long val,
 
 stage2:
 	/* 2nd stage: dereference memory if needed */
-	while (code->op == FETCH_OP_DEREF) {
-		lval = val;
-		ret = probe_mem_read(&val, (void *)val + code->offset,
-					sizeof(val));
+	do {
+		if (code->op == FETCH_OP_DEREF) {
+			lval = val;
+			ret = probe_mem_read(&val, (void *)val + code->offset,
+					     sizeof(val));
+		} else if (code->op == FETCH_OP_UDEREF) {
+			lval = val;
+			ret = probe_mem_read_user(&val,
+				 (void *)val + code->offset, sizeof(val));
+		} else
+			break;
 		if (ret)
 			return ret;
 		code++;
-	}
+	} while (1);
 
 	s3 = code;
 stage3:
@@ -109,6 +118,10 @@ process_fetch_insn_bottom(struct fetch_insn *code, unsigned long val,
 	case FETCH_OP_ST_MEM:
 		probe_mem_read(dest, (void *)val + code->offset, code->size);
 		break;
+	case FETCH_OP_ST_UMEM:
+		probe_mem_read_user(dest, (void *)val + code->offset,
+				    code->size);
+		break;
 	case FETCH_OP_ST_STRING:
 		loc = *(u32 *)dest;
 		ret = fetch_store_string(val + code->offset, dest, base);
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 92facae8c3d8..a86afc9e2a6a 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -140,6 +140,13 @@ probe_mem_read(void *dest, void *src, size_t size)
 
 	return copy_from_user(dest, vaddr, size) ? -EFAULT : 0;
 }
+
+static nokprobe_inline int
+probe_mem_read_user(void *dest, void *src, size_t size)
+{
+	return probe_mem_read(dest, src, size);
+}
+
 /*
  * Fetch a null-terminated string. Caller MUST set *(u32 *)dest with max
  * length and relative data location.


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 6/6] selftests/ftrace: Add user-memory access syntax testcase
  2019-02-28 12:30 [PATCH v4 0/6] tracing/probes: uaccess: Add support user-space access Masami Hiramatsu
                   ` (4 preceding siblings ...)
  2019-02-28 12:32 ` [PATCH v4 5/6] tracing/probe: Support user-space dereference Masami Hiramatsu
@ 2019-02-28 12:33 ` Masami Hiramatsu
  2019-02-28 15:12   ` Masami Hiramatsu
  5 siblings, 1 reply; 9+ messages in thread
From: Masami Hiramatsu @ 2019-02-28 12:33 UTC (permalink / raw)
  To: Steven Rostedt, Linus Torvalds
  Cc: mhiramat, linux-kernel, Andy Lutomirski, Ingo Molnar,
	Andrew Morton, Changbin Du, Jann Horn, Kees Cook,
	Andy Lutomirski, Alexei Starovoitov, Nadav Amit, Peter Zijlstra,
	Joel Fernandes

Add a user-memory access syntax testcase which checks
new user-memory access syntax and ustring type.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 .../ftrace/test.d/kprobe/kprobe_args_user.tc       |   31 ++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc

diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
new file mode 100644
index 000000000000..996da9d1eec5
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
@@ -0,0 +1,31 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Kprobe event user-memory access
+
+[ -f kprobe_events ] || exit_unsupported # this is configurable
+
+grep -A10 "fetcharg:" README | grep -q 'ustring' || exit_unsupported
+grep -A10 "fetcharg:" README | grep -q '\[u\]<offset>' || exit_unsupported
+
+:;: "user-memory access syntax and ustring working on user memory";:
+echo 'p:myevent do_sys_open path=+0($arg2):ustring path2=+u0($arg2):string' \
+	> kprobe_events
+
+grep myevent kprobe_events | \
+	grep -q 'path=+0($arg2):ustring path2=+u0($arg2):string'
+echo 1 > events/kprobes/myevent/enable
+echo > /dev/null
+echo 0 > events/kprobes/myevent/enable
+
+grep myevent trace | grep -q 'path="/dev/null" path2="/dev/null"'
+
+:;: "user-memory access syntax and ustring not working with kernel memory";:
+echo 'p:myevent vfs_symlink path=+0($arg3):ustring path2=+u0($arg3):string' \
+	> kprobe_events
+echo 1 > events/kprobes/myevent/enable
+ln -s foo $TMPDIR/bar
+echo 0 > events/kprobes/myevent/enable
+
+grep myevent trace | grep -q 'path=(fault) path2=(fault)'
+
+exit 0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 6/6] selftests/ftrace: Add user-memory access syntax testcase
  2019-02-28 12:33 ` [PATCH v4 6/6] selftests/ftrace: Add user-memory access syntax testcase Masami Hiramatsu
@ 2019-02-28 15:12   ` Masami Hiramatsu
  0 siblings, 0 replies; 9+ messages in thread
From: Masami Hiramatsu @ 2019-02-28 15:12 UTC (permalink / raw)
  To: Masami Hiramatsu, Shuah Khan
  Cc: Steven Rostedt, Linus Torvalds, linux-kernel, Andy Lutomirski,
	Ingo Molnar, Andrew Morton, Changbin Du, Jann Horn, Kees Cook,
	Andy Lutomirski, Alexei Starovoitov, Nadav Amit, Peter Zijlstra,
	Joel Fernandes

Hi,

Oops, I forgot to Cc Shuah.
I'll update a bit this series and send v5.

Thanks,

On Thu, 28 Feb 2019 21:33:00 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Add a user-memory access syntax testcase which checks
> new user-memory access syntax and ustring type.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  .../ftrace/test.d/kprobe/kprobe_args_user.tc       |   31 ++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
> new file mode 100644
> index 000000000000..996da9d1eec5
> --- /dev/null
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
> @@ -0,0 +1,31 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +# description: Kprobe event user-memory access
> +
> +[ -f kprobe_events ] || exit_unsupported # this is configurable
> +
> +grep -A10 "fetcharg:" README | grep -q 'ustring' || exit_unsupported
> +grep -A10 "fetcharg:" README | grep -q '\[u\]<offset>' || exit_unsupported
> +
> +:;: "user-memory access syntax and ustring working on user memory";:
> +echo 'p:myevent do_sys_open path=+0($arg2):ustring path2=+u0($arg2):string' \
> +	> kprobe_events
> +
> +grep myevent kprobe_events | \
> +	grep -q 'path=+0($arg2):ustring path2=+u0($arg2):string'
> +echo 1 > events/kprobes/myevent/enable
> +echo > /dev/null
> +echo 0 > events/kprobes/myevent/enable
> +
> +grep myevent trace | grep -q 'path="/dev/null" path2="/dev/null"'
> +
> +:;: "user-memory access syntax and ustring not working with kernel memory";:
> +echo 'p:myevent vfs_symlink path=+0($arg3):ustring path2=+u0($arg3):string' \
> +	> kprobe_events
> +echo 1 > events/kprobes/myevent/enable
> +ln -s foo $TMPDIR/bar
> +echo 0 > events/kprobes/myevent/enable
> +
> +grep myevent trace | grep -q 'path=(fault) path2=(fault)'
> +
> +exit 0
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 2/6] uaccess: Use user_access_ok() in user_access_begin()
  2019-02-28 12:31 ` [PATCH v4 2/6] uaccess: Use user_access_ok() in user_access_begin() Masami Hiramatsu
@ 2019-03-03 12:48   ` kbuild test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2019-03-03 12:48 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: kbuild-all, Steven Rostedt, Linus Torvalds, mhiramat,
	linux-kernel, Andy Lutomirski, Ingo Molnar, Andrew Morton,
	Changbin Du, Jann Horn, Kees Cook, Andy Lutomirski,
	Alexei Starovoitov, Nadav Amit, Peter Zijlstra, Joel Fernandes

[-- Attachment #1: Type: text/plain, Size: 7277 bytes --]

Hi Masami,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v5.0-rc8 next-20190301]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Masami-Hiramatsu/tracing-probes-uaccess-Add-support-user-space-access/20190303-150015
config: riscv-defconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=riscv 

All errors (new ones prefixed by >>):

   In file included from arch/riscv/include/asm/bug.h:75,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:12,
                    from lib/strncpy_from_user.c:5:
   lib/strncpy_from_user.c: In function 'strncpy_from_user':
>> arch/riscv/include/asm/uaccess.h:42:19: error: 'TASK_SIZE' undeclared (first use in this function); did you mean 'TASK_SIZE_OF'?
    #define USER_DS  (TASK_SIZE)
                      ^~~~~~~~~
   include/asm-generic/bug.h:148:27: note: in definition of macro 'WARN_ON_ONCE'
     int __ret_warn_once = !!(condition);   \
                              ^~~~~~~~~
   include/linux/uaccess.h:26:16: note: in expansion of macro 'segment_eq'
     WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS));   \
                   ^~~~~~~~~~
   include/linux/uaccess.h:26:37: note: in expansion of macro 'USER_DS'
     WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS));   \
                                        ^~~~~~~
   include/linux/uaccess.h:285:37: note: in expansion of macro 'user_access_ok'
    #define user_access_begin(ptr, len) user_access_ok(ptr, len)
                                        ^~~~~~~~~~~~~~
   lib/strncpy_from_user.c:117:7: note: in expansion of macro 'user_access_begin'
      if (user_access_begin(src, max)) {
          ^~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/uaccess.h:42:19: note: each undeclared identifier is reported only once for each function it appears in
    #define USER_DS  (TASK_SIZE)
                      ^~~~~~~~~
   include/asm-generic/bug.h:148:27: note: in definition of macro 'WARN_ON_ONCE'
     int __ret_warn_once = !!(condition);   \
                              ^~~~~~~~~
   include/linux/uaccess.h:26:16: note: in expansion of macro 'segment_eq'
     WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS));   \
                   ^~~~~~~~~~
   include/linux/uaccess.h:26:37: note: in expansion of macro 'USER_DS'
     WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS));   \
                                        ^~~~~~~
   include/linux/uaccess.h:285:37: note: in expansion of macro 'user_access_ok'
    #define user_access_begin(ptr, len) user_access_ok(ptr, len)
                                        ^~~~~~~~~~~~~~
   lib/strncpy_from_user.c:117:7: note: in expansion of macro 'user_access_begin'
      if (user_access_begin(src, max)) {
          ^~~~~~~~~~~~~~~~~
--
   In file included from arch/riscv/include/asm/bug.h:75,
                    from include/linux/bug.h:5,
                    from arch/riscv/include/asm/current.h:21,
                    from include/linux/sched.h:12,
                    from include/linux/uaccess.h:5,
                    from lib/strnlen_user.c:4:
   lib/strnlen_user.c: In function 'strnlen_user':
>> arch/riscv/include/asm/uaccess.h:42:19: error: 'TASK_SIZE' undeclared (first use in this function); did you mean 'TASK_SIZE_OF'?
    #define USER_DS  (TASK_SIZE)
                      ^~~~~~~~~
   include/asm-generic/bug.h:148:27: note: in definition of macro 'WARN_ON_ONCE'
     int __ret_warn_once = !!(condition);   \
                              ^~~~~~~~~
   include/linux/uaccess.h:26:16: note: in expansion of macro 'segment_eq'
     WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS));   \
                   ^~~~~~~~~~
   include/linux/uaccess.h:26:37: note: in expansion of macro 'USER_DS'
     WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS));   \
                                        ^~~~~~~
   include/linux/uaccess.h:285:37: note: in expansion of macro 'user_access_ok'
    #define user_access_begin(ptr, len) user_access_ok(ptr, len)
                                        ^~~~~~~~~~~~~~
   lib/strnlen_user.c:117:7: note: in expansion of macro 'user_access_begin'
      if (user_access_begin(str, max)) {
          ^~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/uaccess.h:42:19: note: each undeclared identifier is reported only once for each function it appears in
    #define USER_DS  (TASK_SIZE)
                      ^~~~~~~~~
   include/asm-generic/bug.h:148:27: note: in definition of macro 'WARN_ON_ONCE'
     int __ret_warn_once = !!(condition);   \
                              ^~~~~~~~~
   include/linux/uaccess.h:26:16: note: in expansion of macro 'segment_eq'
     WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS));   \
                   ^~~~~~~~~~
   include/linux/uaccess.h:26:37: note: in expansion of macro 'USER_DS'
     WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS));   \
                                        ^~~~~~~
   include/linux/uaccess.h:285:37: note: in expansion of macro 'user_access_ok'
    #define user_access_begin(ptr, len) user_access_ok(ptr, len)
                                        ^~~~~~~~~~~~~~
   lib/strnlen_user.c:117:7: note: in expansion of macro 'user_access_begin'
      if (user_access_begin(str, max)) {
          ^~~~~~~~~~~~~~~~~

vim +42 arch/riscv/include/asm/uaccess.h

5d8544e2 Palmer Dabbelt 2017-07-10  27  
5d8544e2 Palmer Dabbelt 2017-07-10  28  #define __enable_user_access()							\
5d8544e2 Palmer Dabbelt 2017-07-10  29  	__asm__ __volatile__ ("csrs sstatus, %0" : : "r" (SR_SUM) : "memory")
5d8544e2 Palmer Dabbelt 2017-07-10  30  #define __disable_user_access()							\
5d8544e2 Palmer Dabbelt 2017-07-10  31  	__asm__ __volatile__ ("csrc sstatus, %0" : : "r" (SR_SUM) : "memory")
5d8544e2 Palmer Dabbelt 2017-07-10  32  
5d8544e2 Palmer Dabbelt 2017-07-10  33  /*
5d8544e2 Palmer Dabbelt 2017-07-10  34   * The fs value determines whether argument validity checking should be
5d8544e2 Palmer Dabbelt 2017-07-10  35   * performed or not.  If get_fs() == USER_DS, checking is performed, with
5d8544e2 Palmer Dabbelt 2017-07-10  36   * get_fs() == KERNEL_DS, checking is bypassed.
5d8544e2 Palmer Dabbelt 2017-07-10  37   *
5d8544e2 Palmer Dabbelt 2017-07-10  38   * For historical reasons, these macros are grossly misnamed.
5d8544e2 Palmer Dabbelt 2017-07-10  39   */
5d8544e2 Palmer Dabbelt 2017-07-10  40  
5d8544e2 Palmer Dabbelt 2017-07-10  41  #define KERNEL_DS	(~0UL)
5d8544e2 Palmer Dabbelt 2017-07-10 @42  #define USER_DS		(TASK_SIZE)
5d8544e2 Palmer Dabbelt 2017-07-10  43  

:::::: The code at line 42 was first introduced by commit
:::::: 5d8544e2d0075a5f3c9a2cf27152354d54360da1 RISC-V: Generic library routines and assembly

:::::: TO: Palmer Dabbelt <palmer@dabbelt.com>
:::::: CC: Palmer Dabbelt <palmer@dabbelt.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 16804 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-03-03 12:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-28 12:30 [PATCH v4 0/6] tracing/probes: uaccess: Add support user-space access Masami Hiramatsu
2019-02-28 12:30 ` [PATCH v4 1/6] uaccess: Add user_access_ok() Masami Hiramatsu
2019-02-28 12:31 ` [PATCH v4 2/6] uaccess: Use user_access_ok() in user_access_begin() Masami Hiramatsu
2019-03-03 12:48   ` kbuild test robot
2019-02-28 12:31 ` [PATCH v4 3/6] uaccess: Add non-pagefault user-space read functions Masami Hiramatsu
2019-02-28 12:32 ` [PATCH v4 4/6] tracing/probe: Add ustring type for user-space string Masami Hiramatsu
2019-02-28 12:32 ` [PATCH v4 5/6] tracing/probe: Support user-space dereference Masami Hiramatsu
2019-02-28 12:33 ` [PATCH v4 6/6] selftests/ftrace: Add user-memory access syntax testcase Masami Hiramatsu
2019-02-28 15:12   ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).