Add vmlinux_build_id() so that callers can print a hex format string
representation of the running kernel's build ID. This will be used in
the kdump and dump_stack code so that developers can easily locate the
vmlinux debug symbols for a crash/stacktrace.
Cc: Jiri Olsa <[email protected]>
Cc: Alexei Starovoitov <[email protected]>
Cc: Jessica Yu <[email protected]>
Cc: Evan Green <[email protected]>
Cc: Hsin-Yi Wang <[email protected]>
Cc: Dave Young <[email protected]>
Cc: Baoquan He <[email protected]>
Cc: Vivek Goyal <[email protected]>
Cc: <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
include/linux/buildid.h | 2 ++
lib/buildid.c | 19 +++++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/include/linux/buildid.h b/include/linux/buildid.h
index ebce93f26d06..2ff6b1b7cc9b 100644
--- a/include/linux/buildid.h
+++ b/include/linux/buildid.h
@@ -10,4 +10,6 @@ int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
__u32 *size);
int build_id_parse_buf(const void *buf, unsigned char *build_id, u32 buf_size);
+const unsigned char *vmlinux_build_id(void);
+
#endif
diff --git a/lib/buildid.c b/lib/buildid.c
index 010ab0674cb9..fa1b6466b4b8 100644
--- a/lib/buildid.c
+++ b/lib/buildid.c
@@ -4,6 +4,7 @@
#include <linux/elf.h>
#include <linux/kernel.h>
#include <linux/pagemap.h>
+#include <linux/string.h>
#define BUILD_ID 3
@@ -171,3 +172,21 @@ int build_id_parse_buf(const void *buf, unsigned char *build_id, u32 buf_size)
{
return parse_build_id_buf(build_id, NULL, buf, buf_size);
}
+
+/**
+ * vmlinux_build_id - Get the running kernel's build ID
+ *
+ * Return: Running kernel's build ID
+ */
+const unsigned char *vmlinux_build_id(void)
+{
+ extern const void __start_notes __weak;
+ extern const void __stop_notes __weak;
+ unsigned int size = &__stop_notes - &__start_notes;
+ static unsigned char vmlinux_build_id[BUILD_ID_SIZE_MAX];
+
+ if (!memchr_inv(vmlinux_build_id, 0, BUILD_ID_SIZE_MAX))
+ build_id_parse_buf(&__start_notes, vmlinux_build_id, size);
+
+ return vmlinux_build_id;
+}
--
https://chromeos.dev
On 24/03/2021 03.04, Stephen Boyd wrote:
> Add vmlinux_build_id() so that callers can print a hex format string
> representation of the running kernel's build ID. This will be used in
> the kdump and dump_stack code so that developers can easily locate the
> vmlinux debug symbols for a crash/stacktrace.
>
> Cc: Jiri Olsa <[email protected]>
> Cc: Alexei Starovoitov <[email protected]>
> Cc: Jessica Yu <[email protected]>
> Cc: Evan Green <[email protected]>
> Cc: Hsin-Yi Wang <[email protected]>
> Cc: Dave Young <[email protected]>
> Cc: Baoquan He <[email protected]>
> Cc: Vivek Goyal <[email protected]>
> Cc: <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> include/linux/buildid.h | 2 ++
> lib/buildid.c | 19 +++++++++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/include/linux/buildid.h b/include/linux/buildid.h
> index ebce93f26d06..2ff6b1b7cc9b 100644
> --- a/include/linux/buildid.h
> +++ b/include/linux/buildid.h
> @@ -10,4 +10,6 @@ int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
> __u32 *size);
> int build_id_parse_buf(const void *buf, unsigned char *build_id, u32 buf_size);
>
> +const unsigned char *vmlinux_build_id(void);
> +
> #endif
> diff --git a/lib/buildid.c b/lib/buildid.c
> index 010ab0674cb9..fa1b6466b4b8 100644
> --- a/lib/buildid.c
> +++ b/lib/buildid.c
> @@ -4,6 +4,7 @@
> #include <linux/elf.h>
> #include <linux/kernel.h>
> #include <linux/pagemap.h>
> +#include <linux/string.h>
>
> #define BUILD_ID 3
>
> @@ -171,3 +172,21 @@ int build_id_parse_buf(const void *buf, unsigned char *build_id, u32 buf_size)
> {
> return parse_build_id_buf(build_id, NULL, buf, buf_size);
> }
> +
> +/**
> + * vmlinux_build_id - Get the running kernel's build ID
> + *
> + * Return: Running kernel's build ID
> + */
> +const unsigned char *vmlinux_build_id(void)
> +{
> + extern const void __start_notes __weak;
> + extern const void __stop_notes __weak;
> + unsigned int size = &__stop_notes - &__start_notes;
> + static unsigned char vmlinux_build_id[BUILD_ID_SIZE_MAX];
> +
> + if (!memchr_inv(vmlinux_build_id, 0, BUILD_ID_SIZE_MAX))
> + build_id_parse_buf(&__start_notes, vmlinux_build_id, size);
> +
> + return vmlinux_build_id;
> +}
>
Hm, is there any reason to do that initialization lazily and thus need
an accessor? If the system is coming down hard, there's a (very very
small) risk that one thread starts finding the build id, is in the
middle of the memcpy, another thread also ends up wanting the vmlinux
build id, sees some non-nul byte, and proceeds to using the partially
written vmlinux_build_id.
Perhaps consider just exposing the vmlinux_build_id[] array itself,
adding a init_vmlinux_build_id() call somewhere early in start_kernel().
It could then also be made __ro_after_init.
In any case, if you decide to keep the current way, please rename the
local variable (just "build_id" is fine) so that it doesn't shadow the
very function it resides in, that's very confusing.
Rasmus
Quoting Rasmus Villemoes (2021-03-24 02:24:27)
> On 24/03/2021 03.04, Stephen Boyd wrote:
> > Add vmlinux_build_id() so that callers can print a hex format string
> > representation of the running kernel's build ID. This will be used in
> > the kdump and dump_stack code so that developers can easily locate the
> > vmlinux debug symbols for a crash/stacktrace.
> >
> > Cc: Jiri Olsa <[email protected]>
> > Cc: Alexei Starovoitov <[email protected]>
> > Cc: Jessica Yu <[email protected]>
> > Cc: Evan Green <[email protected]>
> > Cc: Hsin-Yi Wang <[email protected]>
> > Cc: Dave Young <[email protected]>
> > Cc: Baoquan He <[email protected]>
> > Cc: Vivek Goyal <[email protected]>
> > Cc: <[email protected]>
> > Signed-off-by: Stephen Boyd <[email protected]>
> > ---
> > include/linux/buildid.h | 2 ++
> > lib/buildid.c | 19 +++++++++++++++++++
> > 2 files changed, 21 insertions(+)
> >
> > diff --git a/include/linux/buildid.h b/include/linux/buildid.h
> > index ebce93f26d06..2ff6b1b7cc9b 100644
> > --- a/include/linux/buildid.h
> > +++ b/include/linux/buildid.h
> > @@ -10,4 +10,6 @@ int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
> > __u32 *size);
> > int build_id_parse_buf(const void *buf, unsigned char *build_id, u32 buf_size);
> >
> > +const unsigned char *vmlinux_build_id(void);
> > +
> > #endif
> > diff --git a/lib/buildid.c b/lib/buildid.c
> > index 010ab0674cb9..fa1b6466b4b8 100644
> > --- a/lib/buildid.c
> > +++ b/lib/buildid.c
> > @@ -4,6 +4,7 @@
> > #include <linux/elf.h>
> > #include <linux/kernel.h>
> > #include <linux/pagemap.h>
> > +#include <linux/string.h>
> >
> > #define BUILD_ID 3
> >
> > @@ -171,3 +172,21 @@ int build_id_parse_buf(const void *buf, unsigned char *build_id, u32 buf_size)
> > {
> > return parse_build_id_buf(build_id, NULL, buf, buf_size);
> > }
> > +
> > +/**
> > + * vmlinux_build_id - Get the running kernel's build ID
> > + *
> > + * Return: Running kernel's build ID
> > + */
> > +const unsigned char *vmlinux_build_id(void)
> > +{
> > + extern const void __start_notes __weak;
> > + extern const void __stop_notes __weak;
> > + unsigned int size = &__stop_notes - &__start_notes;
> > + static unsigned char vmlinux_build_id[BUILD_ID_SIZE_MAX];
> > +
> > + if (!memchr_inv(vmlinux_build_id, 0, BUILD_ID_SIZE_MAX))
> > + build_id_parse_buf(&__start_notes, vmlinux_build_id, size);
> > +
> > + return vmlinux_build_id;
> > +}
> >
>
> Hm, is there any reason to do that initialization lazily and thus need
> an accessor? If the system is coming down hard, there's a (very very
> small) risk that one thread starts finding the build id, is in the
> middle of the memcpy, another thread also ends up wanting the vmlinux
> build id, sees some non-nul byte, and proceeds to using the partially
> written vmlinux_build_id.
>
> Perhaps consider just exposing the vmlinux_build_id[] array itself,
> adding a init_vmlinux_build_id() call somewhere early in start_kernel().
>
> It could then also be made __ro_after_init.
>
> In any case, if you decide to keep the current way, please rename the
> local variable (just "build_id" is fine) so that it doesn't shadow the
> very function it resides in, that's very confusing.
>
No particular reason to do it this way. I'll take that approach to
initialize it early in start_kernel() and then expose the array instead.
Thanks!