On Thu, Feb 2, 2023 at 2:55 PM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > 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/ Yes, it is based on bpf-next/master. This is my first patch here and I am still trying to figure out a few logistics getting the patch correct. I figured out that my patch has some formatting issue that's not obvious and causing the patch apply to fail. I was "handcrafting" the patch email previously until I found out that this could all be done by tools. Please checkout the latest version of the patch. It applied successfully. https://patchwork.kernel.org/project/netdevbpf/patch/20230203080210.2459384-1-hao.xiang@xxxxxxxxxxxxx/ > > > > 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
