On Wed, Feb 1, 2023 at 10:33 PM Hao Xiang . <hao.xiang@xxxxxxxxxxxxx> wrote: > > In a previous commit, Ubuntu kernel code version is correctly set > by retrieving the information from /proc/version_signature. > > commit<5b3d72987701d51bf31823b39db49d10970f5c2d> > (libbpf: Improve LINUX_VERSION_CODE detection) > > The /proc/version_signature file doesn't present in at least the > older versions of Debian distributions (eg, Debian 9, 10). The Debian > kernel has a similar issue where the release information from uname() > syscall doesn't give the kernel code version that matches what the > kernel actually expects. Below is an example content from Debian 10. > > release: 4.19.0-23-amd64 > version: #1 SMP Debian 4.19.269-1 (2022-12-20) x86_64 > > Debian reports incorrect kernel version in utsname::release returned > by uname() syscall, which in older kernels (Debian 9, 10) leads to > kprobe BPF programs failing to load due to the version check mismatch. > > Fortunately, the correct kernel code version presents in the > utsname::version returned by uname() syscall in Debian kernels. This > change adds another get kernel version function to handle Debian in > addition to the previously added get kernel version function to handle > Ubuntu. Some minor refactoring work is also done to make the code more > readable. > > Signed-off-by: Hao Xiang <hao.xiang@xxxxxxxxxxxxx> > --- Are you basing your patches on top of bpf-next/master? BPF CI claims that your patch has merge conflicts ([0]) [0] https://patchwork.kernel.org/project/netdevbpf/patch/CAAYibXgCncBUj8m03iGvOgq8dt2evNHFh0BO1-EnAjtkf+c5yg@xxxxxxxxxxxxxx/ > tools/lib/bpf/libbpf.c | 37 -------------- > tools/lib/bpf/libbpf_probes.c | 93 +++++++++++++++++++++++++++++++++++ > 2 files changed, 93 insertions(+), 37 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index eed5cec6f510..4191a78b2815 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -34,7 +34,6 @@ > #include <linux/limits.h> > #include <linux/perf_event.h> > #include <linux/ring_buffer.h> > -#include <linux/version.h> > #include <sys/epoll.h> > #include <sys/ioctl.h> > #include <sys/mman.h> > @@ -870,42 +869,6 @@ bpf_object__add_programs(struct bpf_object *obj, > Elf_Data *sec_data, > return 0; > } > > -__u32 get_kernel_version(void) > -{ > - /* On Ubuntu LINUX_VERSION_CODE doesn't correspond to info.release, > - * but Ubuntu provides /proc/version_signature file, as described at > - * https://ubuntu.com/kernel, with an example contents below, which we > - * can use to get a proper LINUX_VERSION_CODE. > - * > - * Ubuntu 5.4.0-12.15-generic 5.4.8 > - * > - * In the above, 5.4.8 is what kernel is actually expecting, while > - * uname() call will return 5.4.0 in info.release. > - */ > - const char *ubuntu_kver_file = "/proc/version_signature"; > - __u32 major, minor, patch; > - struct utsname info; > - > - if (faccessat(AT_FDCWD, ubuntu_kver_file, R_OK, AT_EACCESS) == 0) { > - FILE *f; > - > - f = fopen(ubuntu_kver_file, "r"); > - if (f) { > - if (fscanf(f, "%*s %*s %d.%d.%d\n", &major, > &minor, &patch) == 3) { > - fclose(f); > - return KERNEL_VERSION(major, minor, patch); > - } > - fclose(f); > - } > - /* something went wrong, fall back to uname() approach */ > - } > - > - uname(&info); > - if (sscanf(info.release, "%u.%u.%u", &major, &minor, &patch) != 3) > - return 0; > - return KERNEL_VERSION(major, minor, patch); > -} > - > static const struct btf_member * > find_member_by_offset(const struct btf_type *t, __u32 bit_offset) > { > diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c > index b44fcbb4b42e..8d2a2bc5eec9 100644 > --- a/tools/lib/bpf/libbpf_probes.c > +++ b/tools/lib/bpf/libbpf_probes.c > @@ -12,11 +12,104 @@ > #include <linux/btf.h> > #include <linux/filter.h> > #include <linux/kernel.h> > +#include <linux/version.h> > > #include "bpf.h" > #include "libbpf.h" > #include "libbpf_internal.h" > > +/* On Ubuntu LINUX_VERSION_CODE doesn't correspond to info.release, > + * but Ubuntu provides /proc/version_signature file, as described at > + * https://ubuntu.com/kernel, with an example contents below, which we > + * can use to get a proper LINUX_VERSION_CODE. > + * > + * Ubuntu 5.4.0-12.15-generic 5.4.8 > + * > + * In the above, 5.4.8 is what kernel is actually expecting, while > + * uname() call will return 5.4.0 in info.release. > + */ > +static __u32 get_ubuntu_kernel_version(void) > +{ > + const char *ubuntu_kver_file = "/proc/version_signature"; > + __u32 major, minor, patch; > + > + if (faccessat(AT_FDCWD, ubuntu_kver_file, R_OK, AT_EACCESS) == 0) { nit: invert condition and exit early > + FILE *f; > + > + f = fopen(ubuntu_kver_file, "r"); > + if (f) { same, invert and return > + if (fscanf(f, "%*s %*s %d.%d.%d\n", &major, > &minor, &patch) == 3) { let's also use %u just like you do in the new code > + fclose(f); > + return KERNEL_VERSION(major, minor, patch); > + } > + fclose(f); > + } > + /* something went wrong, fall back to uname() approach */ > + } > + > + return 0; > +} > + > +/* On Debian LINUX_VERSION_CODE doesn't correspond to info.release. > + * Instead, it is provided in info.version. An example content of > + * Debian 10 looks like the below. > + * > + * utsname::release 4.19.0-22-amd64 > + * utsname::version #1 SMP Debian 4.19.260-1 (2022-09-29) > + * > + * In the above, 4.19.260 is what kernel is actually expecting, while > + * uname() call will return 4.19.0 in info.release. > + */ > +static __u32 get_debian_kernel_version(struct utsname *info) > +{ > + __u32 major, minor, patch; > + char *version; > + char *p; > + > + version = info->version; > + > + p = strstr(version, "Debian "); > + if (!p) { > + /* This is not a Debian kernel. */ > + return 0; > + } > + > + if (sscanf(p, "Debian %u.%u.%u", &major, &minor, &patch) != 3) > + return 0; > + > + return KERNEL_VERSION(major, minor, patch); > +} > + > +static __u32 get_general_kernel_version(struct utsname *info) > +{ > + __u32 major, minor, patch; > + > + if (sscanf(info->release, "%u.%u.%u", &major, &minor, &patch) != 3) > + return 0; > + > + return KERNEL_VERSION(major, minor, patch); > +} just put this at the bottom of get_kernel_version, not worth having a separate function for generic case, IMO > + > +__u32 get_kernel_version(void) > +{ > + __u32 version; > + struct utsname info; > + > + /* Check if this is an Ubuntu kernel. */ > + version = get_ubuntu_kernel_version(); > + if (version != 0) > + return version; > + > + uname(&info); > + > + /* Check if this is a Debian kernel. */ > + version = get_debian_kernel_version(&info); > + if (version != 0) > + return version; > + > + return get_general_kernel_version(&info); > +} > + > static int probe_prog_load(enum bpf_prog_type prog_type, > const struct bpf_insn *insns, size_t insns_cnt, > char *log_buf, size_t log_buf_sz) > -- > 2.30.2
