On 12/09/2023 11:51, Zachary Leaf wrote:
[...]
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index f1c8733f76b8..867c51d45a83 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -3,6 +3,7 @@ */ #include <linux/bpf.h> #include <linux/bpf-cgroup.h> +#include <linux/bpf_compat.h> #include <linux/bpf_trace.h> #include <linux/bpf_lirc.h> #include <linux/bpf_verifier.h> @@ -11,6 +12,7 @@ #include <linux/syscalls.h> #include <linux/slab.h> #include <linux/sched/signal.h> +#include <linux/stddef.h>
We can skip this. It's included from <linux/kernel.h> so we can consider it available by default.
[...]
+static int copy_bpf_attr_from_user(union bpf_attr *attr, int cmd,
bpfptr_t uattr, unsigned int *size)+{
- union compat_bpf_attr cattr;
- void *select_attr = in_compat64_syscall() ? &cattr : attr;
Nit: select_attr sounds a bit strange, I guess as in "selected", but that doesn't describe what it is. Maybe target_attr? That would apply to the other conversion functions as well.
- size_t attr_size = in_compat64_syscall() ? sizeof(union compat_bpf_attr)
: sizeof(union bpf_attr);- int err;
- err = bpf_check_uarg_tail_zero(uattr, attr_size, *size);
- if (err)
return err;- *size = min_t(u32, *size, attr_size);
- /* copy attributes from user space, may be less than sizeof(bpf_attr) */
- memset(select_attr, 0, attr_size);
- if (copy_from_bpfptr(select_attr, uattr, *size) != 0)
return -EFAULT;- if (in_compat64_syscall())
convert_compat_bpf_attr(attr, &cattr, cmd);- return 0;
+}
+static int __sys_bpf(int cmd, bpfptr_t uattr, unsigned int size) +{
- union bpf_attr attr;
- int err;
- err = bpf_check_perms(cmd);
- if (err)
return err;- err = copy_bpf_attr_from_user(&attr, cmd, uattr, &size);
- if (err)
return err;- return dispatch_bpf(cmd, &attr, uattr, size);
+}
Let's merge this and dispatch_bpf together. This way, we keep __sys_bpf mostly unchanged, removing the big diff in the switch (replacing &attr with attr). This is partly the reason why I suggested not having a separate compat handler: no need to create a common function, reducing the diff further. We probably don't need to create bpf_check_perms() either.
Kevin
SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size) { return __sys_bpf(cmd, USER_BPFPTR(uattr), size);