Proposal: bpf_copy_from_user_remote

[Gabriele <phoenix1987@gmail.com>]

Hi there

As I have mentioned in another thread
(https://lore.kernel.org/bpf/CAGnuNNt7va4u78rvPmusYnhXAuy5e9aRhEeO6HDqYUsH979QLQ@mail.gmail.com/T/)
I started looking into adding a BPF wrapper around process_vm_readv to
allow BPF programs to read the userspace VM of a remote process.

Given my unfamiliarity with the code, I hope you would indulge me in
asking for feedback on my current approach to see whether it makes
sense or not. I have tried to model my change on top of the patch that
introduced bpf_copy_from_user
(https://lwn.net/ml/netdev/20200630043343.53195-4-alexei.starovoitov@gmail.com/)
and this is what I have got so far.

Cheers,
Gab

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index e7a163a3146b..05060a709609 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2150,6 +2150,7 @@ extern const struct bpf_func_proto
bpf_skc_to_tcp_request_sock_proto;
 extern const struct bpf_func_proto bpf_skc_to_udp6_sock_proto;
 extern const struct bpf_func_proto bpf_skc_to_unix_sock_proto;
 extern const struct bpf_func_proto bpf_copy_from_user_proto;
+extern const struct bpf_func_proto bpf_copy_from_user_remote_proto;
 extern const struct bpf_func_proto bpf_snprintf_btf_proto;
 extern const struct bpf_func_proto bpf_snprintf_proto;
 extern const struct bpf_func_proto bpf_per_cpu_ptr_proto;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index a7e4a9e7d807..8af855d62a5f 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3391,5 +3391,8 @@ static inline int seal_check_future_write(int
seals, struct vm_area_struct *vma)
  return 0;
 }

+extern ssize_t process_vm_read(pid_t pid, void * dst, ssize_t size,
+ const void __user * user_ptr, unsigned long flags);
+
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index ba5af15e25f5..436c703f3a13 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -4938,6 +4938,14 @@ union bpf_attr {
  * **-ENOENT** if symbol is not found.
  *
  * **-EPERM** if caller does not have permission to obtain kernel address.
+ *
+ * long bpf_copy_from_user_remote(void *dst, u32 size, const void
*user_ptr, pid_t pid)
+ * 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
+ * **process_vm_readv**\ ().
+ * Return
+ * 0 on success, or a negative error in case of failure.
  */
 #define __BPF_FUNC_MAPPER(FN) \
  FN(unspec), \
@@ -5120,6 +5128,7 @@ union bpf_attr {
  FN(trace_vprintk), \
  FN(skc_to_unix_sock), \
  FN(kallsyms_lookup_name), \
+ 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 649f07623df6..0343870a8c03 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -15,6 +15,7 @@
 #include <linux/pid_namespace.h>
 #include <linux/proc_ns.h>
 #include <linux/security.h>
+#include <linux/mm.h>

 #include "../../lib/kstrtox.h"

@@ -656,6 +657,37 @@ 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,
+    const void __user *, user_ptr, pid_t, pid)
+{
+ int ret;
+ struct iovec local, remote;
+
+ local.iov_base = dst;
+ remote.iov_base = (void *) user_ptr;
+
+ local.iov_len = remote.iov_len = size;
+
+ ret = process_vm_read(pid, dst, size, user_ptr, 0);
+
+ if (unlikely(ret)) {
+ memset(dst, 0, size);
+ ret = -EFAULT;
+ }
+
+ return ret;
+}
+
+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 ae9755037b7e..b9f27b0b62f9 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1208,6 +1208,8 @@ bpf_tracing_func_proto(enum bpf_func_id func_id,
const struct bpf_prog *prog)
  return &bpf_get_branch_snapshot_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/mm/process_vm_access.c b/mm/process_vm_access.c
index 4bcc11958089..90097267b567 100644
--- a/mm/process_vm_access.c
+++ b/mm/process_vm_access.c
@@ -302,3 +302,12 @@ SYSCALL_DEFINE6(process_vm_writev, pid_t, pid,
 {
  return process_vm_rw(pid, lvec, liovcnt, rvec, riovcnt, flags, 1);
 }
+
+ssize_t process_vm_read(pid_t pid, void * dst, ssize_t size,
+ const void __user * user_ptr, unsigned long flags)
+{
+    struct iovec lvec = {.iov_base = dst, .iov_len = size};
+    struct iovec rvec = {.iov_base = (void *) user_ptr, .iov_len = size};
+
+ return process_vm_rw(pid, &lvec, 1, &rvec, 1, flags, 0);
+}
\ No newline at end of file

-- 
"Egli è scritto in lingua matematica, e i caratteri son triangoli,
cerchi, ed altre figure
geometriche, senza i quali mezzi è impossibile a intenderne umanamente parola;
senza questi è un aggirarsi vanamente per un oscuro laberinto."

-- G. Galilei, Il saggiatore.