On 8/11/23 16:05, Stanislav Fomichev wrote:
On 08/10, thinker.li@xxxxxxxxx wrote:From: Kui-Feng Lee <kuifeng@xxxxxxxx> Provide bpf_copy_from_user() and bpf_copy_to_user() to the BPF programs attached to cgroup/{set,get}sockopt. bpf_copy_to_user() is a new kfunc to copy data from an kernel space buffer to a user space buffer. They are only available for sleepable BPF programs. bpf_copy_to_user() is only available to the BPF programs attached to cgroup/getsockopt. Signed-off-by: Kui-Feng Lee <thinker.li@xxxxxxxxx> --- kernel/bpf/cgroup.c | 6 ++++++ kernel/bpf/helpers.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c index 5bf3115b265c..c15a72860d2a 100644 --- a/kernel/bpf/cgroup.c +++ b/kernel/bpf/cgroup.c @@ -2461,6 +2461,12 @@ cg_sockopt_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) #endif case BPF_FUNC_perf_event_output: return &bpf_event_output_data_proto; + + case BPF_FUNC_copy_from_user: + if (prog->aux->sleepable) + return &bpf_copy_from_user_proto; + return NULL;If we just allow copy to/from, I'm not sure I understand how the buffer sharing between sleepable/non-sleepable works. Let's assume I have two progs in the chain: 1. non-sleepable - copies the buffer, does some modifications; since we don't copy the buffer back after every prog run, the modifications stay in the kernel buffer 2. sleepable - runs and just gets the user pointer? does it mean this sleepable program doesn't see the changes from (1)? IOW, do we need some custom sockopt copy_to/from that handle this potential buffer location transparently or am I missing something? Assuming we want to support this at all. If we do, might deserve a selftest.
It is why BPF_SOCKOPT_FLAG_OPTVAL_USER is there. It helps programs to make a right decision. However, I am going to remove bpf_copy_from_user() since we have bpf_so_optval_copy_to() and bpf_so_optval_copy_to_r(). Does it make sense to you?
