| Message ID | 20181215234947.18225-1-daniel@iogearbox.net |
|---|---|
| State | Accepted, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf-next] bpf: remove useless version check for prog load | expand |
2018-12-16 00:49 UTC+0100 ~ Daniel Borkmann <daniel@iogearbox.net> > Existing libraries and tracing frameworks work around this kernel > version check by automatically deriving the kernel version from > uname(3) or similar such that the user does not need to do it > manually; these workarounds also make the version check useless > at the same time. > > Moreover, most other BPF tracing types enabling bpf_probe_read()-like > functionality have /not/ adapted this check, and in general these > days it is well understood anyway that all the tracing programs are > not stable with regards to future kernels as kernel internal data > structures are subject to change from release to release. > > Back at last netconf we discussed [0] and agreed to remove this > check from bpf_prog_load() and instead document it here in the uapi > header that there is no such guarantee for stable API for these > programs. > > [0] http://vger.kernel.org/netconf2018_files/DanielBorkmann_netconf2018.pdf > > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> > Acked-by: Alexei Starovoitov <ast@kernel.org> For what it's worth, Acked-by: Quentin Monnet <quentin.monnet@netronome.com> Note: samples and selftests setting a kern_version can probably wait before being updated (if they get updated at all), but you might want to change now the remark in Documentation/bpf/bpf_design_QA.rst about kern_version being checked.
On 12/17/2018 10:39 AM, Quentin Monnet wrote:
[...]
> Note: samples and selftests setting a kern_version can probably wait before being updated (if they get updated at all), but you might want to change now the remark in Documentation/bpf/bpf_design_QA.rst about kern_version being checked.
Makes sense, I have couple of other doc updates, so I'll spin this in
there as well.
Thanks,
Daniel
2018-12-17 10:57 UTC+0100 ~ Daniel Borkmann <daniel@iogearbox.net> > On 12/17/2018 10:39 AM, Quentin Monnet wrote: > [...] >> Note: samples and selftests setting a kern_version can probably wait before being updated (if they get updated at all), but you might want to change now the remark in Documentation/bpf/bpf_design_QA.rst about kern_version being checked. > > Makes sense, I have couple of other doc updates, so I'll spin this in > there as well. Sounds good, thanks!
On Mon, Dec 17, 2018 at 09:39:47AM +0000, Quentin Monnet wrote: > 2018-12-16 00:49 UTC+0100 ~ Daniel Borkmann <daniel@iogearbox.net> > > Existing libraries and tracing frameworks work around this kernel > > version check by automatically deriving the kernel version from > > uname(3) or similar such that the user does not need to do it > > manually; these workarounds also make the version check useless > > at the same time. > > > > Moreover, most other BPF tracing types enabling bpf_probe_read()-like > > functionality have /not/ adapted this check, and in general these > > days it is well understood anyway that all the tracing programs are > > not stable with regards to future kernels as kernel internal data > > structures are subject to change from release to release. > > > > Back at last netconf we discussed [0] and agreed to remove this > > check from bpf_prog_load() and instead document it here in the uapi > > header that there is no such guarantee for stable API for these > > programs. > > > > [0] http://vger.kernel.org/netconf2018_files/DanielBorkmann_netconf2018.pdf > > > > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> > > Acked-by: Alexei Starovoitov <ast@kernel.org> > > For what it's worth, > > Acked-by: Quentin Monnet <quentin.monnet@netronome.com> Applied, Thanks
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index e7d57e89..07ce10c 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -133,6 +133,14 @@ enum bpf_map_type { BPF_MAP_TYPE_STACK, }; +/* Note that tracing related programs such as + * BPF_PROG_TYPE_{KPROBE,TRACEPOINT,PERF_EVENT,RAW_TRACEPOINT} + * are not subject to a stable API since kernel internal data + * structures can change from release to release and may + * therefore break existing tracing BPF programs. Tracing BPF + * programs correspond to /a/ specific kernel which is to be + * analyzed, and not /a/ specific kernel /and/ all future ones. + */ enum bpf_prog_type { BPF_PROG_TYPE_UNSPEC, BPF_PROG_TYPE_SOCKET_FILTER, @@ -343,7 +351,7 @@ union bpf_attr { __u32 log_level; /* verbosity level of verifier */ __u32 log_size; /* size of user buffer */ __aligned_u64 log_buf; /* user supplied buffer */ - __u32 kern_version; /* checked when prog_type=kprobe */ + __u32 kern_version; /* not used */ __u32 prog_flags; char prog_name[BPF_OBJ_NAME_LEN]; __u32 prog_ifindex; /* ifindex of netdev to prep for */ diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 6ae062f..5db3106 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -1473,11 +1473,6 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr) if (attr->insn_cnt == 0 || attr->insn_cnt > BPF_MAXINSNS) return -E2BIG; - - if (type == BPF_PROG_TYPE_KPROBE && - attr->kern_version != LINUX_VERSION_CODE) - return -EINVAL; - if (type != BPF_PROG_TYPE_SOCKET_FILTER && type != BPF_PROG_TYPE_CGROUP_SKB && !capable(CAP_SYS_ADMIN)) diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index e7d57e89..07ce10c 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -133,6 +133,14 @@ enum bpf_map_type { BPF_MAP_TYPE_STACK, }; +/* Note that tracing related programs such as + * BPF_PROG_TYPE_{KPROBE,TRACEPOINT,PERF_EVENT,RAW_TRACEPOINT} + * are not subject to a stable API since kernel internal data + * structures can change from release to release and may + * therefore break existing tracing BPF programs. Tracing BPF + * programs correspond to /a/ specific kernel which is to be + * analyzed, and not /a/ specific kernel /and/ all future ones. + */ enum bpf_prog_type { BPF_PROG_TYPE_UNSPEC, BPF_PROG_TYPE_SOCKET_FILTER, @@ -343,7 +351,7 @@ union bpf_attr { __u32 log_level; /* verbosity level of verifier */ __u32 log_size; /* size of user buffer */ __aligned_u64 log_buf; /* user supplied buffer */ - __u32 kern_version; /* checked when prog_type=kprobe */ + __u32 kern_version; /* not used */ __u32 prog_flags; char prog_name[BPF_OBJ_NAME_LEN]; __u32 prog_ifindex; /* ifindex of netdev to prep for */