On 30/10/2023 13:48, Zachary Leaf wrote:
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index f1c8733f76b8..c870a29c9be8 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -5015,6 +5015,298 @@ static int bpf_prog_bind_map(union bpf_attr *attr) return ret; } +static void convert_compat_bpf_attr(union bpf_attr *dest,
const union compat_bpf_attr *cattr, int cmd)
+{
- struct bpf_prog *prog;
- memset(dest, 0, sizeof(union bpf_attr));
I'm realising, isn't this already done in copy_bpf_attr_from_user()?
[...]
- case BPF_LINK_CREATE:
copy_field(dest, cattr, link_create.prog_fd);
copy_field(dest, cattr, link_create.target_fd);
/* u32 target_ifindex is in a union with u32 target_fd */
copy_field(dest, cattr, link_create.attach_type);
copy_field(dest, cattr, link_create.flags);
prog = bpf_prog_get(cattr->link_create.prog_fd);
if (prog->type == BPF_PROG_TYPE_CGROUP_SKB ||
prog->type == BPF_PROG_TYPE_CGROUP_SOCK ||
prog->type == BPF_PROG_TYPE_CGROUP_SOCK_ADDR ||
prog->type == BPF_PROG_TYPE_SOCK_OPS ||
prog->type == BPF_PROG_TYPE_CGROUP_DEVICE ||
prog->type == BPF_PROG_TYPE_CGROUP_SYSCTL ||
prog->type == BPF_PROG_TYPE_CGROUP_SOCKOPT)
break;
if (prog->type == BPF_PROG_TYPE_EXT) {
copy_field(dest, cattr, link_create.tracing.target_btf_id);
copy_field(dest, cattr, link_create.tracing.cookie);
break;
}
if (prog->type == BPF_PROG_TYPE_LSM ||
prog->type == BPF_PROG_TYPE_TRACING) {
if (prog->expected_attach_type == BPF_TRACE_ITER) {
/* iter_info is a user pointer to union
* bpf_iter_link_info however since this union
* contains no pointers, the size/offsets are
* the same for compat64/purecap; hence no
* conversion needed
*/
copy_field(dest, cattr, link_create.iter_info);
copy_field(dest, cattr, link_create.iter_info_len);
break;
} else if (prog->expected_attach_type == BPF_TRACE_RAW_TP
|| prog->expected_attach_type == BPF_LSM_CGROUP) {
/* only uses common fields above */
break;
} else {
copy_field(dest, cattr, link_create.target_btf_id);
copy_field(dest, cattr, link_create.tracing.cookie);
break;
}
}
if (prog->type == BPF_PROG_TYPE_FLOW_DISSECTOR ||
prog->type == BPF_PROG_TYPE_SK_LOOKUP ||
prog->type == BPF_PROG_TYPE_XDP)
break;
/* bpf_cookie is used in bpf_perf_link_attach() */
if (prog->type == BPF_PROG_TYPE_PERF_EVENT ||
prog->type == BPF_PROG_TYPE_TRACEPOINT ||
(prog->type == BPF_PROG_TYPE_KPROBE &&
cattr->link_create.attach_type == BPF_PERF_EVENT)) {
copy_field(dest, cattr, link_create.perf_event.bpf_cookie);
break;
}
/* kprobe_multi is used in bpf_kprobe_multi_link_attach() */
if (prog->type == BPF_PROG_TYPE_KPROBE &&
cattr->link_create.attach_type != BPF_PERF_EVENT) {
copy_field(dest, cattr, link_create.kprobe_multi.flags);
copy_field(dest, cattr, link_create.kprobe_multi.cnt);
copy_field(dest, cattr, link_create.kprobe_multi.syms);
copy_field(dest, cattr, link_create.kprobe_multi.addrs);
copy_field(dest, cattr, link_create.kprobe_multi.cookies);
break;
}
if (prog->type == BPF_PROG_TYPE_NETFILTER) {
copy_field(dest, cattr, link_create.netfilter.pf);
copy_field(dest, cattr, link_create.netfilter.hooknum);
copy_field(dest, cattr, link_create.netfilter.priority);
copy_field(dest, cattr, link_create.netfilter.flags);
break;
}
I don't feel that this approach is really maintainable. In the meantime, a new special case has already been added [1].
Two cases involve pointers. It seems to me that we can identify them based on attach_type alone: BPF_TRACE_ITER for iter_info, and BPF_TRACE_KPROBE_MULTI for kprobe_multi. There are conditions for a call with such attach_type values to be valid, but that doesn't matter - what does is that such attach_type values unambiguously mean that these fields are active.
In all other cases, I think we could just use some memcpy() like in convert_compat_link_info_out() in patch 8. This way, we only need to update this code if operations involving pointers are added, which should be pretty infrequent.
Kevin
[1] https://lore.kernel.org/all/20230323032405.3735486-4-kuifeng@meta.com/