linux-kernel - Re: [PATCH v3 12/12] kdump: Use vmlinux_build

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 for Android: free password hash cracker in your pocket

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

Message-ID: <161786020531.3790633.14618419586085886962@swboyd.mtv.corp.google.com>
Date:   Wed, 07 Apr 2021 22:36:45 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     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>,
        Dave Young <dyoung@...hat.com>, Baoquan He <bhe@...hat.com>,
        Vivek Goyal <vgoyal@...hat.com>, kexec@...ts.infradead.org
Subject: Re: [PATCH v3 12/12] kdump: Use vmlinux_build_id to simplify

Quoting Petr Mladek (2021-04-07 10:03:28)
> On Tue 2021-03-30 20:05:20, Stephen Boyd wrote:
> > We can use the vmlinux_build_id array here now instead of open coding
> > it. This mostly consolidates code.
> > 
> > Cc: Jiri Olsa <jolsa@...nel.org>
> > Cc: Alexei Starovoitov <ast@...nel.org>
> > Cc: Jessica Yu <jeyu@...nel.org>
> > Cc: Evan Green <evgreen@...omium.org>
> > Cc: Hsin-Yi Wang <hsinyi@...omium.org>
> > Cc: Dave Young <dyoung@...hat.com>
> > Cc: Baoquan He <bhe@...hat.com>
> > Cc: Vivek Goyal <vgoyal@...hat.com>
> > Cc: <kexec@...ts.infradead.org>
> > Signed-off-by: Stephen Boyd <swboyd@...omium.org>
> > ---
> >  include/linux/crash_core.h |  6 +-----
> >  kernel/crash_core.c        | 41 ++------------------------------------
> >  2 files changed, 3 insertions(+), 44 deletions(-)
> > 
> > diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> > index 206bde8308b2..fb8ab99bb2ee 100644
> > --- a/include/linux/crash_core.h
> > +++ b/include/linux/crash_core.h
> > @@ -39,7 +39,7 @@ phys_addr_t paddr_vmcoreinfo_note(void);
> >  #define VMCOREINFO_OSRELEASE(value) \
> >       vmcoreinfo_append_str("OSRELEASE=%s\n", value)
> >  #define VMCOREINFO_BUILD_ID(value) \
> > -     vmcoreinfo_append_str("BUILD-ID=%s\n", value)
> > +     vmcoreinfo_append_str("BUILD-ID=%20phN\n", value)
> 
> Please, add also build check that BUILD_ID_MAX == 20.
> 

I added a BUILD_BUG_ON() in kernel/crash_core.c. I tried static_assert()
here but got mixed ISO errors from gcc-10, although it feels like it
should work.

In file included from ./arch/arm64/include/asm/cmpxchg.h:10,
                 from ./arch/arm64/include/asm/atomic.h:16,
                 from ./include/linux/atomic.h:7,
                 from ./include/linux/mm_types_task.h:13,
                 from ./include/linux/mm_types.h:5,
                 from ./include/linux/buildid.h:5,
                 from kernel/crash_core.c:7:
kernel/crash_core.c: In function 'crash_save_vmcoreinfo_init':
./include/linux/build_bug.h:78:41: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                         ^~~~~~~~~~~~~~
./include/linux/build_bug.h:77:34: note: in expansion of macro '__static_assert'
   77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
      |                                  ^~~~~~~~~~~~~~~
./include/linux/crash_core.h:42:2: note: in expansion of macro 'static_assert'
   42 |  static_assert(ARRAY_SIZE(value) == BUILD_ID_SIZE_MAX); \
      |  ^~~~~~~~~~~~~
kernel/crash_core.c:401:2: note: in expansion of macro 'VMCOREINFO_BUILD_ID'
  401 |  VMCOREINFO_BUILD_ID(vmlinux_build_id);

> 
> The function add_build_id_vmcoreinfo() is used in
> crash_save_vmcoreinfo_init() in this context:
> 
> 
>         VMCOREINFO_OSRELEASE(init_uts_ns.name.release);
>         add_build_id_vmcoreinfo();
>         VMCOREINFO_PAGESIZE(PAGE_SIZE);
> 
>         VMCOREINFO_SYMBOL(init_uts_ns);
>         VMCOREINFO_OFFSET(uts_namespace, name);
>         VMCOREINFO_SYMBOL(node_online_map);
> 
> The function is not longer need. VMCOREINFO_BUILD_ID()
> can be used directly:
> 
>         VMCOREINFO_OSRELEASE(init_uts_ns.name.release);
>         VMCOREINFO_BUILD_ID(vmlinux_build_id);
>         VMCOREINFO_PAGESIZE(PAGE_SIZE);
> 
>         VMCOREINFO_SYMBOL(init_uts_ns);
>         VMCOREINFO_OFFSET(uts_namespace, name);
>         VMCOREINFO_SYMBOL(node_online_map);
> 
> 

Thanks. Makes sense. I've rolled that in.