Hi Luca,
On 05/06/2023 16:46, Luca Vizzarro wrote:
Whenever a GUP call is made, a user address in the form of a 64-bit integer is used to lookup its corresponding page. When working in PCuABI, this means that the metadata of the user capability gets discarded, hence any access made by the GUP is not checked in hardware.
This commit introduces explicit capability checks whenever a call to the current mm through the GUP functions is made.> Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com
kernel/bpf/helpers.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index a8e76cf06da7..4db910f1758d 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -675,7 +675,9 @@ BPF_CALL_5(bpf_copy_from_user_task, void *, dst, u32, size,
Turns out this one is a bit complicated.
Here's the full signature for reference: BPF_CALL_5(bpf_copy_from_user_task, void *, dst, u32, size, const void __user *, user_ptr, struct task_struct *, tsk, u64, flags)
also see: BPF_CALL_3(bpf_copy_from_user, void *, dst, u32, size, const void __user *, user_ptr)
Having looked into this and talking with Kevin I think what we should be doing here is instead changing these from __user *ptrs into just an address i.e. ptraddr_t.
There's quite a lot going on so I'll try keep it short:
- bpf programs have to be one of a specific type e.g. BPF_PROG_TYPE_SOCKET_FILTER, ...TRACEPOINT etc - bpf programs cannot do whatever they want in the kernel space, there is a limited set of available kernel functions available to them called bpf helper functions - internally bpf programs call directly into the compiled helper functions - each particular program type only has access to a small subset of available helper functions that is relevant to the program type (along with common functions available to all) - bpf_copy_from_user_task and bpf_copy_from_user are helper functions that are only available to tracing prog types i.e. TRACEPOINT, KPROBE, PERF_EVENT, RAW_TP - these specific program types always require privileges to load them from userspace into the kernel and execute them
The purpose of bpf_copy_from_user() is to read memory of whatever process is being traced.
The bpf_copy_from_user_task() helper function allows reading memory from some other thread or processes address space.
Here's an excerpt from the bpf-helpers man page[1]: Read *size* bytes from user space address *user_ptr* in *tsk*'s address space, and stores the data in *dst*
The purpose of these seems to be either debugging[2] or some kind of security function as per this lwn article[3]: One potential use for this ability would be to allow security- related BPF programs to [...] get a better idea of what user space is actually up to."
The main problem here is that there is no way for a bpf program to hold or use capabilities to pass to these helper functions. bpf as an architecture does not support or understand capabilities.
Even if we did somehow extend bpf to allow this, we'd need a way to pass in valid capabilities and I'm not certain how this would work or even if it would make sense to do so here.
Loading these tracing bpf programs already requires root privileges so I think we just have to accept they will be able to access arbitrary user memory.
In any case I can address these issues in the next version of the bpf series so I think this patch can be dropped from here for now.
Thanks,
Zach
1: https://man.archlinux.org/man/bpf-helpers.7.en 2: https://lore.kernel.org/bpf/20220124185403.468466-3-kennyyu@fb.com/ 3: https://lwn.net/Articles/825415/
if (unlikely(!size)) return 0;
- /* TODO [PCuABI] - capability checks for uaccess *
TLDR: no capability check required here.
- if (!check_user_ptr_read(user_ptr, size))
return -EFAULT;- ret = access_process_vm(tsk, user_ptr_addr(user_ptr), dst, size, 0); if (ret == size) return 0;