Hi there As I have mentioned in another thread (https://lore.kernel.org/bpf/CAGnuNNt7va4u78rvPmusYnhXAuy5e9aRhEeO6HDqYUsH979QLQ@xxxxxxxxxxxxxx/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@xxxxxxxxx/) 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.
