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

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

[songrui.771]

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@bytedance.com>
---
 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
+
 /* 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
@@ -47,7 +51,7 @@ static __u32 get_ubuntu_kernel_version(void)
 	if (ret != 3)
 		return 0;
 
-	return KERNEL_VERSION(major, minor, patch);
+	return LIBBPF_KERNEL_VERSION(major, minor, patch);
 }
 
 /* On Debian LINUX_VERSION_CODE doesn't correspond to info.release.
@@ -74,7 +78,7 @@ static __u32 get_debian_kernel_version(struct utsname *info)
 	if (sscanf(p, "Debian %u.%u.%u", &major, &minor, &patch) != 3)
 		return 0;
 
-	return KERNEL_VERSION(major, minor, patch);
+	return LIBBPF_KERNEL_VERSION(major, minor, patch);
 }
 
 __u32 get_kernel_version(void)
@@ -97,7 +101,7 @@ __u32 get_kernel_version(void)
 	if (sscanf(info.release, "%u.%u.%u", &major, &minor, &patch) != 3)
 		return 0;
 
-	return KERNEL_VERSION(major, minor, patch);
+	return LIBBPF_KERNEL_VERSION(major, minor, patch);
 }
 
 static int probe_prog_load(enum bpf_prog_type prog_type,
-- 
2.39.2 (Apple Git-143)

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

[Greg KH]

On Mon, Apr 17, 2023 at 04:44:49PM +0800, songrui.771 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@bytedance.com>

This needs to be your name, not your email alias (do you use ".771" as a
name to sign things with?)

> ---
>  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?

thanks,

greg k-h

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

[Greg KH]

On Mon, Apr 17, 2023 at 06:44:34AM -0700, 宋锐 wrote:

<snip>

You responded in html, which is dropped by the mailing lists.  Please
fix your email client up to not do so and we can continue the
conversation.  The kernel Documentation/ directory has a file explaining
how to fix your email client if you have questions about this.

thanks,

greg k-h

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

[宋锐]

> > 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@bytedance.com>
>
> 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:

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.

Hope I make that clear. Thanks.

Jerry Song

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@bytedance.com>
> >
> > 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

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

[宋锐]

> > > > 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@bytedance.com>
> > >
> > > 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 again for your reply. You're absolutely right. This bug exists
on many older kernels(Debian9, 10, CentOS 7). It's not a kernel bug,
but the  kernel release package bug. I will correct the
KERNEL_VERSION() macro in header file linux/kernel.h provided by
kernel package to make things right.

Thanks
Best
Jerry Song