On 12/09/2023 11:51, Zachary Leaf wrote:
[...] +#define bpf_uattr_compat_ptr(DEST, SRC, FIELD) \
- ((DEST)->FIELD = (__kernel_aligned_uintptr_t)compat_ptr((SRC)->FIELD))
Nit: better use lowercase arguments here to be consistent with the surrounding macros.
Also since this isn't tied to struct bpf_attr in any way, it might be better to call it something like bpf_compat_ptr_feld(). It's not actually specific to BPF either but I'm not sure it makes sense to have such a uapi-specific helper in stddef.h, so I'd stick to the middle-ground approach.
[...] @@ -3200,7 +3200,7 @@ static int bpf_raw_tp_link_fill_link_info(const struct bpf_link *link, { struct bpf_raw_tp_link *raw_tp_link = container_of(link, struct bpf_raw_tp_link, link);
- char __user *ubuf = u64_to_user_ptr(info->raw_tracepoint.tp_name);
- char __user *ubuf = (void __user *)info->raw_tracepoint.tp_name;
Nit: s/void/char/
[...] +#define bpfptr_copy(dest, src, size) \
- (in_compat64_syscall() ? copy_from_bpfptr(dest, src, size) \
: copy_from_bpfptr_with_ptr(dest, src, size))
Nit: empty line before a function.
I'm not a big fan of this sort of local macro but I have to admit it makes the function more readable. It's not the only case where we want a conditional *_with_ptr, I'll think about adding a generic helper. Let's keep this one for now.
Kevin
static int copy_bpf_attr_from_user(union bpf_attr *attr, int cmd, bpfptr_t uattr, unsigned int *size) {
[...]