On 12/09/2023 11:51, Zachary Leaf wrote:
The purpose of the bpf_copy_from_user_with_task eBPF helper is to copy user data from the provided task. Since __user *ptrs are only valid for the current task/process, what is actually being passed in here is an address. access_process_vm is then used to access that address given the task specified by the task_struct.
Additionally, this helper is called from an eBPF program, which does not support capabilities. It is therefore unable to pass in a capability as a __user ptr in the first place.
(in PCuABI)
The use case of this helper is to read arbitrary user memory, as used by security or monitoring eBPF programs. There is therefore no requirement to check user capabilities or uaccess here. Loading of eBPF applications
It might be worth clarifying that access_process_vm() is not really uaccess in the usual sense, as uaccess is about accessing current. We simply never check such accesses at the moment.
using this helper type is already strictly limited to privileged/root users.
Change the inbound type from __user * to ptraddr_t and remove the TODO since no capability checks are required here.
Signed-off-by: Zachary Leaf zachary.leaf@arm.com
kernel/bpf/helpers.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 1362d7736a93..b4f3c2079484 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -670,7 +670,7 @@ const struct bpf_func_proto bpf_copy_from_user_proto = { }; BPF_CALL_5(bpf_copy_from_user_task, void *, dst, u32, size,
const void __user *, user_ptr, struct task_struct *, tsk, u64, flags)
const ptraddr_t, addr, struct task_struct *, tsk, u64, flags)
There's no point in passing const arguments - pointers to const are a different story. Same comment in next patch.
Kevin
{ int ret; @@ -681,8 +681,7 @@ BPF_CALL_5(bpf_copy_from_user_task, void *, dst, u32, size, if (unlikely(!size)) return 0;
- /* TODO [PCuABI] - capability checks for uaccess */
- ret = access_process_vm(tsk, user_ptr_addr(user_ptr), dst, size, 0);
- ret = access_process_vm(tsk, addr, dst, size, 0); if (ret == size) return 0;