[PATCH] checkpatch: add warning on WARN/WARN_ON use

[Tetsuo Handa]

Commit 9d3e3c705eb3 ("checkpatch: add warning on BUG/BUG_ON use") started
recommending to replace BUG() with WARN() + recovery code because BUG()
crashes the kernel. However, recently syzkaller (one of continuous fuzz
testing systems) is frequently disturbed by inappropriate "BUG:" or
"WARNING:" usage.

There are procedural problems that severely break the value of "continuous
testing" but are not widely known to developers:

Problem 1: syzkaller uses a kernel config which is close to
"make allyesconfig", which means that initialization failures
of built-in modules which happens during the boot phase
prevents syzkaller from switching to the latest kernel.

Problem 2: It can take many days (or even weeks) until a patch which
fixes initialization failures is applied, which means that
syzkaller is unable to test the latest changes until the fix
patch is applied.

To maintain the efficiency of continuous testing systems, developers should
adhere to the following guidelines regarding error reporting:

Rule 1: Do not use WARN()/WARN_ON() etc. for reporting non-fatal or
recoverable problems such as -EINVAL, -EINTR, -ENOMEM.

Some developers use WARN() as a quick method to notify other developers
that they need to update their code (e.g. when an API has changed or a
new sanity check is introduced). For example, [1] has prevented
syzkaller from switching to the latest linux-next kernel for more than
three weeks.

Other developers use WARN() within normal, expected error paths
(e.g., when kthread_run() is interrupted by a fatal signal [2], or
kmalloc() fails due to fault injection [3]).

These transient or recoverable conditions are exactly where
include/asm-generic/bug.h states WARN() should not be used.
For such conditions, use printk() family, optionally followed by
dump_stack().

Rule 2: Do not use printk("WARNING: ...\n") followed by dump_stack().

Since syzkaller treats printk("WARNING: ...\n") followed by
dump_stack() as an indicator of a fatal problem, please do not use
"WARNING:" prefix when replacing WARN() etc. with printk().

Rule 3: Do not use printk("BUG: ...\n").

Some developers use printk("BUG: ...\n"), but that is effectively
equivalent to panic() for syzkaller [4].

Since syzkaller treats printk("BUG: ...\n") as an indicator of a fatal
problem, please do not use "BUG:" prefix when using printk().

Link: https://syzkaller.appspot.com/bug?extid=6245cb95ae707991bffa [1]
Link: https://syzkaller.appspot.com/bug?extid=1ebbc20f223b99446034 [2]
Link: https://syzkaller.appspot.com/bug?extid=369ee6a6e9d0bbf14b37 [3]
Link: https://syzkaller.appspot.com/bug?extid=ded267b328e950a7c0c4 [4]
Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
---
See https://github.com/google/syzkaller/issues/7686 for background.
I want to get this patch reviewed by syzkaller people before sending
to LKML.

include/asm-generic/bug.h | 17 ++++++++---------
scripts/checkpatch.pl | 9 ++++++++-
2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 09e8eccee8ed..f12ae1c003b6 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -82,18 +82,17 @@ struct bug_entry {
#endif

/*
- * WARN(), WARN_ON(), WARN_ON_ONCE(), and so on can be used to report
- * significant kernel issues that need prompt attention if they should ever
- * appear at runtime.
+ * WARN(), WARN_ON(), WARN_ON_ONCE(), and so on should be used only if
+ * developers want to let system administrators choose from "availability"
+ * (i.e. panic_on_warn=0) or "safety / consistency" (i.e. panic_on_warn=1).
*
- * Do not use these macros when checking for invalid external inputs
+ * Never use these macros when reporting recoverable problems
* (e.g. invalid system call arguments, or invalid data coming from
* network/devices), and on transient conditions like ENOMEM or EAGAIN.
- * These macros should be used for recoverable kernel issues only.
- * For invalid external inputs, transient conditions, etc use
- * pr_err[_once/_ratelimited]() followed by dump_stack(), if necessary.
- * Do not include "BUG"/"WARNING" in format strings manually to make these
- * conditions distinguishable from kernel issues.
+ * For such situations, use printk() family (optionally followed by
+ * dump_stack()). Also, do not use "BUG:" or "WARNING:" prefix when using
+ * printk() family, for continuous kernel testing systems might treat
+ * such prefix as an indicator of a fatal problem.
*
* Use the versions with printk format strings to provide better diagnostics.
*/
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2b7a42bbdd94..161cbb5ee835 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4915,7 +4915,14 @@ sub process {
my $msg_level = \&WARN;
$msg_level = \&CHK if ($file);
&{$msg_level}("AVOID_BUG",
- "Do not crash the kernel unless it is absolutely unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of BUG() or variants\n" . $herecurr);
+ "Do not crash the kernel unless it is absolutely unavoidable. Please use printk() family (without \"BUG:\" prefix, and optionally followed by dump_stack()) plus recovery code unless this is an unrecoverable situation.\n" . $herecurr);
+ }
+# do not use WARN() or variants
+ if ($line =~ /\bWARN(?:_ON)?(_ONCE|_RATELIMITED)?\s*\(/) {
+ my $msg_level = \&WARN;
+ $msg_level = \&CHK if ($file);
+ &{$msg_level}("DISCOURAGE_WARN",
+ "Do not disturb continuous kernel testing systems. Please use printk() family (without \"WARNING:\" prefix, and optionally followed by dump_stack()).\n" . $herecurr);
}

# avoid LINUX_VERSION_CODE
--
2.55.0