linux-kernel - Re: [GIT pull] smp/hotplug updates for 4.13

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]

Message-ID: <CA+55aFxeEnmx-8R1FLwRqamvMZuUqkHto8tadVBae+3WnLYbgA@mail.gmail.com>
Date:   Sun, 9 Jul 2017 11:15:03 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Ingo Molnar <mingo@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [GIT pull] smp/hotplug updates for 4.13

On Sun, Jul 9, 2017 at 2:07 AM, Thomas Gleixner <tglx@...utronix.de> wrote:
>
> +       /* Wait for the CPU to reach CPUHP_AP_ONLINE_IDLE */
>         wait_for_completion(&st->done);
> +       BUG_ON(!cpu_online(cpu));

I realize this isn't actually a new BUG_ON(), just a moved one, but
can we *please* just agree to get rid of these machine-killing
idiocies?

The fact that it would be a bug if this triggers doesn't excuse it one
whit. BUG_ON() is simply bad.

Either it's a "this cannot happen", in which case it should be
deleted, or it's a "if this ever happens, we really should notice it",
in which case it should probably be a

   if (WARN_ON)ONCE(..))
      return -EBUSY;

or something.

That routine very much has an existing error path for failure to bring
up a CPU, dammit!

THERE IS NO EXCUSE FOR KILLING THE MACHINE THERE!

If it really was something core where there is no possible way to
continue, and continuing would imply that the end result will kill
your pets and everbody you love, then yes, use BUG_ON().

But if you already have code to handle the failure case, HELL NO!

I've pulled this thing (exactly because it's not a *new* BUG_ON()),
but I really think that people should look at the code they move. And
if it's a BUG_ON() or similar braindamage, they should look very
closely at what alternatives there are.

               Linus