Re: [PATCH] x86, vsyscall: Fix build warning in vsyscall_64.c

[Linus Torvalds]

On Wed, Jun 15, 2011 at 12:25 AM, Ingo Molnar <mingo@xxxxxxx> wrote:
>
> No, that BUG() is a "cannot happen on a correct kernel" so it has no
> ABI impact - but it might trigger if the execution environment is
> violated:

Well, in genreal, I would seriously suggest that people not use
BUG_ON() as liberally as they tend to be used.

There are *very* few reasons to have a BUG_ON(), and they are all "I
cannot possibly continue from this state".

Anything else should have a WARN_ON() or just return an error, or (if
it's a security issue) just kill the process.

Some kernel developers seem to use BUG_ON() as a "I can't see how this
could ever trigger, so let's kill the machine if it does", and that
really is very wrong.

If you are aware that something should never trigger, I'd suggest you
either say "ok, I'm _sure_ that this cannot trigger" and just remove
the BUG_ON(), or you should ask yourself "are we better off killing
the machine than just returning an error".

In this case, for example, if

>  - leave the warning as-is. Whoever builds with CONFIG_BUG=n will
>   have to live with the consequences of the 'impossible' happening
>   and will have to accept the more unpredictable kernel behavior
>   that *will* trigger in various parts of the kernel if BUG() is
>   turned into a NOP. If any of the above 'impossible' failure modes
>   triggers then having more undefined behavior in form of an
>   uninitialized variable will be the least of their worry.

If it's that impossible, I don't see why you have the BUG_ON() in the
first place.

I also don't see why the code isn't just written to be so strict that
the lack of BUG_ON just wouldn't matter. I think that code that
"depends" on a BUG_ON() for correctness (ie assuming that the whole
thing never returns) is buggy by definition. Please just don't write
code like that.

So what's the reason for just not initializing that 'ret' variable to
-EFAULT, and leaving it at that? And/or just removing all the BUG_ON's
entirely? Do they actually _help_ anything?

                               Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/