linux-kernel - Re: [PATCH v4 05/13] module: Add printk formats to add module build ID to stacktraces

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 13 Apr 2021 15:36:57 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Petr Mladek <pmladek@...e.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, Jiri Olsa <jolsa@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Jessica Yu <jeyu@...nel.org>,
        Evan Green <evgreen@...omium.org>,
        Hsin-Yi Wang <hsinyi@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        linux-doc@...r.kernel.org, Matthew Wilcox <willy@...radead.org>
Subject: Re: [PATCH v4 05/13] module: Add printk formats to add module build ID to stacktraces

Quoting Stephen Boyd (2021-04-13 13:10:05)
> Quoting Petr Mladek (2021-04-13 08:16:20)
> > On Tue 2021-04-13 13:56:31, Andy Shevchenko wrote:
> > > On Mon, Apr 12, 2021 at 12:29:05PM -0700, Stephen Boyd wrote:
> > > > Quoting Andy Shevchenko (2021-04-12 04:58:02)
> > > > > 
> > > > > First of all, why not static_assert() defined near to the actual macro?
> > > > 
> > > > Which macro? BUILD_ID_SIZE_MAX?
> > > 
> > > Yes.
> > > 
> > > > I tried static_assert() and it didn't
> > > > work for me but maybe I missed something.
> > 
> > I guess that you wanted to use it inside macro definition:
> > 
> > #define VMCOREINFO_BUILD_ID(value) \
> >         static_assert(ARRAY_SIZE(value) == BUILD_ID_SIZE_MAX); \
> >         vmcoreinfo_append_str("BUILD-ID=%20phN\n", value)
> > 
> > Instead, you should do it outside the macro:
> > 
> > static_assert(ARRAY_SIZE(value) == BUILD_ID_SIZE_MAX);
> > #define VMCOREINFO_BUILD_ID(value) \
> >         vmcoreinfo_append_str("BUILD-ID=%20phN\n", value)
> 
> In this example "value" is not defined because it's an argument to the
> macro. How can this work?
> 
> From what I can tell static_assert() is for the case that you want to
> assert something at the global scope level. BUILD_BUG_ON() can't be used
> at global scope. I see the usage is usually to assert struct members and
> alignment of those members. In turn, static_assert() can't be used at
> function level scope. Each has a use and in this case I want to assert
> at function level scope to be as close as possible to the place that
> would need to change.
> 

Good news. I can do this to force a basic block and then GCC doesn't complain.

---8<---
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index 2174dab16ba9..de62a722431e 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -38,9 +38,12 @@ phys_addr_t paddr_vmcoreinfo_note(void);

 #define VMCOREINFO_OSRELEASE(value) \
        vmcoreinfo_append_str("OSRELEASE=%s\n", value)
-#define VMCOREINFO_BUILD_ID(value) \
-       BUILD_BUG_ON(ARRAY_SIZE(value) != BUILD_ID_SIZE_MAX); \
-       vmcoreinfo_append_str("BUILD-ID=%20phN\n", value)
+#define VMCOREINFO_BUILD_ID()                                          \
+       ({                                                              \
+               static_assert(sizeof(vmlinux_build_id) == 20);          \
+               vmcoreinfo_append_str("BUILD-ID=%20phN\n", vmlinux_build_id); \
+       })
+
 #define VMCOREINFO_PAGESIZE(value) \
        vmcoreinfo_append_str("PAGESIZE=%ld\n", value)