Re: [External] Re: [PATCH] libbpf: correct the macro KERNEL_VERSION for old kernel

[Greg KH]

On Mon, Apr 17, 2023 at 10:44:47PM -0700, 宋锐 wrote:
> > > The introduced header file linux/version.h in libbpf_probes.c may have a
> > > wrong macro KERNEL_VERSION for calculating LINUX_VERSION_CODE in some old
> > > kernel (Debian9, 10). Below is a version info example from Debian 10.
> > >
> > > release: 4.19.0-22-amd64
> > > version: #1 SMP Debian 4.19.260-1 (2022-09-29)
> > >
> > > The macro KERNEL_VERSION is defined to (((a) << 16) + ((b) << 8)) + (c)),
> > > which a, b, and c stand for major, minor and patch version. So in example here,
> > > the major is 4, minor is 19, patch is 260, the LINUX_VERSION(4, 19, 260) which
> > > is 267268 should be matched to LINUX_VERSION_CODE. However, the KERNEL_VERSION_CODE
> > > in linux/version.h is defined to 267263.
> > >
> > > I noticed that the macro KERNEL_VERSION in linux/version.h of some new kernel is
> > > defined to (((a) << 16) + ((b) << 8) + ((c) > 255 ? 255 : (c))). And
> > > KERNEL_VERSION(4, 19, 260) is equal to 267263 which is the right LINUX_VERSION_CODE.
> > >
> > > The mismatched LINUX_VERSION_CODE which will cause failing to load kprobe BPF
> > > programs in the version check of BPF syscall.
> > >
> > > The return value of get_kernel_version in libbpf_probes.c should be matched to
> > > LINUX_VERSION_CODE by correcting the macro KERNEL_VERSION.
> > >
> > > Signed-off-by: songrui.771 <songrui.771@xxxxxxxxxxxxx>
> >
> > This needs to be your name, not your email alias (do you use ".771" as a
> > name to sign things with?)
> 
> Thanks for your reminding. I will change it.
> >
> > > ---
> > >  tools/lib/bpf/libbpf_probes.c | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> > > index 4f3bc968ff8e..5b22a880c7e7 100644
> > > --- a/tools/lib/bpf/libbpf_probes.c
> > > +++ b/tools/lib/bpf/libbpf_probes.c
> > > @@ -18,6 +18,10 @@
> > >  #include "libbpf.h"
> > >  #include "libbpf_internal.h"
> > >
> > > +#ifndef LIBBPF_KERNEL_VERSION
> > > +#define LIBBPF_KERNEL_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + ((c) > 255 ? 255 : (c)))
> > > +#endif
> >
> > What is wrong with using the KERNEL_VERSION() macro, it should be fixed
> > to work properly here, right?  Did we not get this resolved in the
> > main portion of the kernel already?
> 
> The KERNEL_VERSION() macro from linux/version.h is wrong in some old
> kernel(Debian 9, 10) that we would like to support. As you said, the
> problem was resolved in the newer kernel. Here is the difference:

But the kernels you want to "support" all have older kernel versions and
so you do not need the change to the macro as they are not running newer
kernel versions with an increased minor version number.

So on those systems, building will work just fine, if not, then that's a
Debian bug and they should fix it in their kernel packages.

> linux/version.h
> in older kernel: #define KERNEL_VERSION(a, b, c) (((a) << 16) + ((b)
> << 8)) + (c)))
> in newer kernel: #define KERNEL_VERSION(a, b, c) KERNEL_VERSION(a, b,
> c) (((a) << 16) + ((b) << 8) + ((c) > 255 ? 255 : (c)))
> 
> Using the KERNEL_VERSION macro in the older kernel returns the kern
> version  which is  mismatched to the LINUX_VERSION_CODE that will
> cause failing to load the BPF kprobe program.
> 
> In my opinion, it is a more generic solution that corrects the
> KERNEL_VERSION() macro in libbpf to support some old kernel.

The KERNEL_VERSION() macro comes from the kernel you are building
against.  And so that should match that kernel only.

thanks,

greg k-h