On Wed, Feb 1, 2023 at 4:09 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)
>
> However, the /proc/version_signature file doesn't exist 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>
> Signed-off-by: Ho-Ren (Jack) Chuang <horenchuang@xxxxxxxxxxxxx>
> ---
> tools/lib/bpf/libbpf.c | 92 +++++++++++++++++++++++++++++++++++-------
libbpf.c is already huge, let's move all this
get_kernel_version()-related code to libbpf_probes.c where it is also
used?
> 1 file changed, 78 insertions(+), 14 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index eed5cec6f510..bc022d0cd71f 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -870,21 +870,20 @@ bpf_object__add_programs(struct bpf_object *obj,
> Elf_Data *sec_data,
> return 0;
> }
>
[...]
> +
> +#define VERSION_PREFIX_DEBIAN "Debian "
Not sure #define really helps here, just use "Debian " directly, as
this is a very localized use of this constant
> +
> +/* 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;
> + size_t len;
> + char *version;
> + char *find;
> +
> + version = info->version;
> +
> + find = strstr(version, VERSION_PREFIX_DEBIAN);
> + if (!find) {
nit: find reads confusingly, "found" would be more meaningful, but we
often use short "p" (for position), please use that instead
> + /* This is not a Debian kernel. */
> return 0;
> + }
> +
> + len = strlen(version);
> + find += strlen(VERSION_PREFIX_DEBIAN);
> + if (find - version >= len)
> + return 0;
instead of these strlen() manipulations, can we do just
char *p = strstr(info->version, "Debian ");
if (!p)
return 0;
if (sscanf(p, "Debian %u.%u.%u", ...
?
> +
> + if (sscanf(find, "%u.%u.%u", &major, &minor, &patch) != 3)
> + return 0;
> +
> + return KERNEL_VERSION(major, minor, patch);
> +}
> +
[...]
