[PATCH bpf-next 1/1] bpf: Add bpf_copy_from_user_remote to read a process VM given its PID.

[PATCH bpf-next 1/1] bpf: Add bpf_copy_from_user_remote to read a process VM given its PID.

[Gabriele N. Tornetta]

Add a new BPF helper to read the VM of a process identified by PID.
Whilst PIDs are ambiguous without a namespace, many traditional
observability tools, like profilers and debuggers, accept a PID to
attach to a running process. The new helper proposed by this patch
is aimed at providing the capability of reading a remote process VM
to similar tools.

Signed-off-by: Gabriele N. Tornetta <phoenix1987@gmail.com>
---
 include/linux/bpf.h                           |  1 +
 include/uapi/linux/bpf.h                      |  9 +++++++
 kernel/bpf/helpers.c                          | 26 +++++++++++++++++++
 kernel/trace/bpf_trace.c                      |  2 ++
 scripts/bpf_doc.py                            |  1 +
 tools/include/uapi/linux/bpf.h                |  9 +++++++
 .../selftests/bpf/prog_tests/test_lsm.c       |  7 +++++
 tools/testing/selftests/bpf/progs/lsm.c       | 18 +++++++++++++
 8 files changed, 73 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 6e947cd91152..96efa8912bbd 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2222,6 +2222,7 @@ extern const struct bpf_func_proto bpf_kallsyms_lookup_name_proto;
 extern const struct bpf_func_proto bpf_find_vma_proto;
 extern const struct bpf_func_proto bpf_loop_proto;
 extern const struct bpf_func_proto bpf_strncmp_proto;
+extern const struct bpf_func_proto bpf_copy_from_user_remote_proto;
 
 const struct bpf_func_proto *tracing_prog_func_proto(
   enum bpf_func_id func_id, const struct bpf_prog *prog);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index b0383d371b9a..167ec1bc7248 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5018,6 +5018,14 @@ union bpf_attr {
  *
  *	Return
  *		The number of arguments of the traced function.
+ *
+ * long bpf_copy_from_user_remote(void *dst, u32 size, pid_t pid, const void *user_ptr)
+ *	Description
+ *		Read *size* bytes from user space address *user_ptr* of the prodess
+ *		*pid* and store the data in *dst*. This is essentially a wrapper of
+ *		**access_process_vm**\ ().
+ *	Return
+ *		The number of bytes read on success, or a negative error in case of failure.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -5206,6 +5214,7 @@ union bpf_attr {
 	FN(get_func_arg),		\
 	FN(get_func_ret),		\
 	FN(get_func_arg_cnt),		\
+	FN(copy_from_user_remote),	\
 	/* */
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 01cfdf40c838..c055eec77466 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -16,6 +16,7 @@
 #include <linux/pid_namespace.h>
 #include <linux/proc_ns.h>
 #include <linux/security.h>
+#include <linux/mm.h>
 
 #include "../../lib/kstrtox.h"
 
@@ -671,6 +672,31 @@ const struct bpf_func_proto bpf_copy_from_user_proto = {
 	.arg3_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_4(bpf_copy_from_user_remote, void *, dst, u32, size,
+	   pid_t, pid, const void __user *, user_ptr)
+{
+	struct task_struct *task;
+
+	if (unlikely(size == 0))
+		return 0;
+
+	task = find_get_task_by_vpid(pid);
+	if (!task)
+		return -ESRCH;
+
+	return access_process_vm(task, (unsigned long)user_ptr, dst, size, 0);
+}
+
+const struct bpf_func_proto bpf_copy_from_user_remote_proto = {
+	.func		= bpf_copy_from_user_remote,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_UNINIT_MEM,
+	.arg2_type	= ARG_CONST_SIZE_OR_ZERO,
+	.arg3_type	= ARG_ANYTHING,
+	.arg4_type	= ARG_ANYTHING,
+};
+
 BPF_CALL_2(bpf_per_cpu_ptr, const void *, ptr, u32, cpu)
 {
 	if (cpu >= nr_cpu_ids)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 21aa30644219..b30cd5e6d703 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1257,6 +1257,8 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_find_vma_proto;
 	case BPF_FUNC_trace_vprintk:
 		return bpf_get_trace_vprintk_proto();
+	case BPF_FUNC_copy_from_user_remote:
+		return prog->aux->sleepable ? &bpf_copy_from_user_remote_proto : NULL;
 	default:
 		return bpf_base_func_proto(func_id);
 	}
diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py
index a6403ddf5de7..bd092f1692e2 100755
--- a/scripts/bpf_doc.py
+++ b/scripts/bpf_doc.py
@@ -614,6 +614,7 @@ class PrinterHelpers(Printer):
             'const struct sk_buff': 'const struct __sk_buff',
             'struct sk_msg_buff': 'struct sk_msg_md',
             'struct xdp_buff': 'struct xdp_md',
+            "pid_t": "int",
     }
     # Helpers overloaded for different context types.
     overloaded_helpers = [
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index b0383d371b9a..167ec1bc7248 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5018,6 +5018,14 @@ union bpf_attr {
  *
  *	Return
  *		The number of arguments of the traced function.
+ *
+ * long bpf_copy_from_user_remote(void *dst, u32 size, pid_t pid, const void *user_ptr)
+ *	Description
+ *		Read *size* bytes from user space address *user_ptr* of the prodess
+ *		*pid* and store the data in *dst*. This is essentially a wrapper of
+ *		**access_process_vm**\ ().
+ *	Return
+ *		The number of bytes read on success, or a negative error in case of failure.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -5206,6 +5214,7 @@ union bpf_attr {
 	FN(get_func_arg),		\
 	FN(get_func_ret),		\
 	FN(get_func_arg_cnt),		\
+	FN(copy_from_user_remote),	\
 	/* */
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
diff --git a/tools/testing/selftests/bpf/prog_tests/test_lsm.c b/tools/testing/selftests/bpf/prog_tests/test_lsm.c
index 244c01125126..71014141c675 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_lsm.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_lsm.c
@@ -56,6 +56,7 @@ static int test_lsm(struct lsm *skel)
 	struct bpf_link *link;
 	int buf = 1234;
 	int err;
+	int fd;
 
 	err = lsm__attach(skel);
 	if (!ASSERT_OK(err, "attach"))
@@ -86,6 +87,12 @@ static int test_lsm(struct lsm *skel)
 
 	ASSERT_EQ(skel->bss->copy_test, 3, "copy_test");
 
+	fd = syscall(__NR_open, "/dev/null", syscall(__NR_getpid));
+	if (fd >= 0)
+		syscall(__NR_close, fd);
+
+	ASSERT_EQ(skel->bss->copy_remote_test, 1, "copy_remote_test");
+
 	lsm__detach(skel);
 
 	skel->bss->copy_test = 0;
diff --git a/tools/testing/selftests/bpf/progs/lsm.c b/tools/testing/selftests/bpf/progs/lsm.c
index 33694ef8acfa..469ff988b0e9 100644
--- a/tools/testing/selftests/bpf/progs/lsm.c
+++ b/tools/testing/selftests/bpf/progs/lsm.c
@@ -177,3 +177,21 @@ int BPF_PROG(test_sys_setdomainname, struct pt_regs *regs)
 		copy_test++;
 	return 0;
 }
+
+int copy_remote_test = 0;
+
+SEC("fentry.s/__x64_sys_open")
+int BPF_PROG(test_sys_open, struct pt_regs *regs)
+{
+	void *ptr = (void *)PT_REGS_PARM1(regs);
+	pid_t pid = (pid_t)PT_REGS_PARM2(regs);
+	char path[4];
+	long ret;
+
+	ret = bpf_copy_from_user_remote(&path, sizeof(path), pid, ptr);
+
+	if (ret == sizeof(path) && !__builtin_memcmp("/dev", path, 4))
+		copy_remote_test++;
+
+	return 0;
+}
-- 
2.30.2

Re: [PATCH bpf-next 1/1] bpf: Add bpf_copy_from_user_remote to read a process VM given its PID.

[Alexei Starovoitov]

On Tue, Jan 18, 2022 at 2:57 AM Gabriele N. Tornetta
<phoenix1987@gmail.com> wrote:
>
> Add a new BPF helper to read the VM of a process identified by PID.
> Whilst PIDs are ambiguous without a namespace, many traditional
> observability tools, like profilers and debuggers, accept a PID to
> attach to a running process. The new helper proposed by this patch
> is aimed at providing the capability of reading a remote process VM
> to similar tools.

So how exactly is it going to be used with a pid provided by a tool?

I'm guessing if bpf prog attaches to some syscall it will filter out
all events that don't match the pid.
Then when current_pid == user_provided_pid it will read memory.
In such case the prog can use bpf_get_current_task_btf()
and Kenny's bpf_access_process_vm(), right?

Re: [PATCH bpf-next 1/1] bpf: Add bpf_copy_from_user_remote to read a process VM given its PID.

[Gabriele]

On Wed, 19 Jan 2022 at 21:44, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> So how exactly is it going to be used with a pid provided by a tool?
>
> I'm guessing if bpf prog attaches to some syscall it will filter out
> all events that don't match the pid.
> Then when current_pid == user_provided_pid it will read memory.
> In such case the prog can use bpf_get_current_task_btf()
> and Kenny's bpf_access_process_vm(), right?

Hi Alexei

If I understand your suggestion correctly, one would call
bpf_get_current_pid_tgid to get the pid and match it to the one passed
by the user-space part of the BPF program, and then get the current
task to pass to Kenny's helper (but wouldn't the existing
bpf_copy_from_user be enough in this case?).

I've had some further thoughts about my current and future use cases
and I think they can be summarised in two scenarios:

Scenario 1: A pid is passed to the user-space part of a BPF program
and used to filter out as suggested above in kernel-space. Then
Kenny's helper is enough.

Scenario 2: A pid != current_pid is derived in the kernel part of BPF
from somewhere in user-space and extra information needs to be
retrieved from the remote process "represented" by this pid.

I would expect many observability tools to fall within Scenario 1.
However, a debugger might be an example of a tool that falls under
Scenario 2 too. E.g. a function that takes a pid as an argument is
traced and one wants to collect information from the VM of the process
"identified" by it. Then the filtering out described above does not
apply and we need either a helper like the one I'm proposing, or to
expose find_get_task_by_vpid to BPF. More concretely, we might be
tracing a native binary application that refers to a runtime like
Python by pid and we might want to be able to return, e.g., the number
of sub-interpreters or Python threads that are currently running, from
events triggered for the native binary.

Cheers,
Gab

Re: [PATCH bpf-next 1/1] bpf: Add bpf_copy_from_user_remote to read a process VM given its PID.

[Alexei Starovoitov]

On Thu, Jan 20, 2022 at 8:56 AM Gabriele <phoenix1987@gmail.com> wrote:
>
> On Wed, 19 Jan 2022 at 21:44, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > So how exactly is it going to be used with a pid provided by a tool?
> >
> > I'm guessing if bpf prog attaches to some syscall it will filter out
> > all events that don't match the pid.
> > Then when current_pid == user_provided_pid it will read memory.
> > In such case the prog can use bpf_get_current_task_btf()
> > and Kenny's bpf_access_process_vm(), right?
>
> Hi Alexei
>
> If I understand your suggestion correctly, one would call
> bpf_get_current_pid_tgid to get the pid and match it to the one passed
> by the user-space part of the BPF program, and then get the current
> task to pass to Kenny's helper (but wouldn't the existing
> bpf_copy_from_user be enough in this case?).
>
> I've had some further thoughts about my current and future use cases
> and I think they can be summarised in two scenarios:
>
> Scenario 1: A pid is passed to the user-space part of a BPF program
> and used to filter out as suggested above in kernel-space. Then
> Kenny's helper is enough.

Right. The existing bpf_copy_from_user will do here.

> Scenario 2: A pid != current_pid is derived in the kernel part of BPF
> from somewhere in user-space and extra information needs to be
> retrieved from the remote process "represented" by this pid.
>
> I would expect many observability tools to fall within Scenario 1.
> However, a debugger might be an example of a tool that falls under
> Scenario 2 too. E.g. a function that takes a pid as an argument is
> traced and one wants to collect information from the VM of the process
> "identified" by it. Then the filtering out described above does not
> apply and we need either a helper like the one I'm proposing, or to
> expose find_get_task_by_vpid to BPF. More concretely, we might be
> tracing a native binary application that refers to a runtime like
> Python by pid and we might want to be able to return, e.g., the number
> of sub-interpreters or Python threads that are currently running, from
> events triggered for the native binary.

How would bpf prog know the pid of the python interpreter?
Then how would it know the pids of the threads?
I'm not against exposing find_get_task_by_vpid(), but
we need to understand the real usage first.
If we do end up exposing find_get_task_by_vpid(), it's probably
best to do via refcnt-ed kfunc approach (unstable helpers).
For example: https://lore.kernel.org/all/20220114163953.1455836-7-memxor@gmail.com/

Re: [PATCH bpf-next 1/1] bpf: Add bpf_copy_from_user_remote to read a process VM given its PID.

[Gabriele]

On Fri, 21 Jan 2022 at 02:09, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> How would bpf prog know the pid of the python interpreter?
> Then how would it know the pids of the threads?
> I'm not against exposing find_get_task_by_vpid(), but
> we need to understand the real usage first.
> If we do end up exposing find_get_task_by_vpid(), it's probably
> best to do via refcnt-ed kfunc approach (unstable helpers).
> For example: https://lore.kernel.org/all/20220114163953.1455836-7-memxor@gmail.com/

This is a simple but somewhat unrealistic example but hopefully it
will give the idea. Suppose we are tracing sys_kill on entry and that
we have an application that uses it to check if a process exists by
sending the 0 signal to its PID. During the handling of this event, we
might want to read a certain area of the VM (which we would have
identified a priori from user-space) of the process identified by the
PID passed to the syscall.

Re: [PATCH bpf-next 1/1] bpf: Add bpf_copy_from_user_remote to read a process VM given its PID.

[Alexei Starovoitov]

On Sun, Jan 23, 2022 at 2:47 AM Gabriele <phoenix1987@gmail.com> wrote:
>
> On Fri, 21 Jan 2022 at 02:09, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > How would bpf prog know the pid of the python interpreter?
> > Then how would it know the pids of the threads?
> > I'm not against exposing find_get_task_by_vpid(), but
> > we need to understand the real usage first.
> > If we do end up exposing find_get_task_by_vpid(), it's probably
> > best to do via refcnt-ed kfunc approach (unstable helpers).
> > For example: https://lore.kernel.org/all/20220114163953.1455836-7-memxor@gmail.com/
>
> This is a simple but somewhat unrealistic example but hopefully it
> will give the idea. Suppose we are tracing sys_kill on entry and that
> we have an application that uses it to check if a process exists by
> sending the 0 signal to its PID. During the handling of this event, we
> might want to read a certain area of the VM (which we would have
> identified a priori from user-space) of the process identified by the
> PID passed to the syscall.

Fair enough. Pls prepare a patch to make find_get_task_by_vpid
into refcnted unstable helper for certain prog types.
Probably sleepable only.