linux-kernel - Re: [PATCH v2] kdb: Avoid array subscript warnings on non-SMP builds

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] [day] [month] [year] [list]

Date:   Mon, 21 Oct 2019 10:01:34 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Daniel Thompson <daniel.thompson@...aro.org>
Cc:     Jason Wessel <jason.wessel@...driver.com>,
        kgdb-bugreport@...ts.sourceforge.net,
        LKML <linux-kernel@...r.kernel.org>,
        Patch Tracking <patches@...aro.org>,
        kbuild test robot <lkp@...el.com>
Subject: Re: [PATCH v2] kdb: Avoid array subscript warnings on non-SMP builds

Hi,

On Mon, Oct 21, 2019 at 3:11 AM Daniel Thompson
<daniel.thompson@...aro.org> wrote:
>
> Recent versions of gcc (reported on gcc-7.4) issue array subscript
> warnings for builds where SMP is not enabled.
>
> kernel/debug/debug_core.c: In function 'kdb_dump_stack_on_cpu':
> kernel/debug/debug_core.c:452:17: warning: array subscript is outside array
> +bounds [-Warray-bounds]
>      if (!(kgdb_info[cpu].exception_state & DCPU_IS_SLAVE)) {
>            ~~~~~~~~~^~~~~
>    kernel/debug/debug_core.c:469:33: warning: array subscript is outside array
> +bounds [-Warray-bounds]
>      kgdb_info[cpu].exception_state |= DCPU_WANT_BT;
>    kernel/debug/debug_core.c:470:18: warning: array subscript is outside array
> +bounds [-Warray-bounds]
>      while (kgdb_info[cpu].exception_state & DCPU_WANT_BT)
>
> There is no bug here but there is scope to improve the code
> generation for non-SMP systems (whilst also silencing the warning).
>
> Reported-by: kbuild test robot <lkp@...el.com>
> Fixes: 2277b492582d ("kdb: Fix stack crawling on 'running' CPUs that aren't the master")
> Signed-off-by: Daniel Thompson <daniel.thompson@...aro.org>
> ---
>
> Notes:
>     Changes in v2:
>
>      - Moved the IS_ENABLED(CONFIG_SMP) test to the first (slightly easier
>        to read the code, improves code generation a little)
>      - Sent out as a proper patch e-mail ;-)
>
>  kernel/debug/debug_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
> index 70e86b4b4932..2b7c9b67931d 100644
> --- a/kernel/debug/debug_core.c
> +++ b/kernel/debug/debug_core.c
> @@ -444,7 +444,7 @@ int dbg_remove_all_break(void)
>  #ifdef CONFIG_KGDB_KDB
>  void kdb_dump_stack_on_cpu(int cpu)
>  {
> -       if (cpu == raw_smp_processor_id()) {
> +       if (cpu == raw_smp_processor_id() || !IS_ENABLED(CONFIG_SMP)) {

At first I thought maybe your code would be less efficient than:

  if (!IS_ENABLED(CONFIG_SMP) || cpu == raw_smp_processor_id())

...since the compiler would be still be required to "call"
raw_smp_processor_id() in the non-SMP case.  ...but then I realized
that seems to be a macro in the non-SMP case and just resolves to 0.
...so while the compiler would still be "required" to execute the
first part of the if test, it should be able to realize that it's a
no-op.

Reviewed-by: Douglas Anderson <dianders@...omium.org>

-Doug




>                 dump_stack();
>                 return;
>         }
> --
> 2.21.0
>