[PATCH v3] Add kernel config option for tweaking kernel behavior.

[PATCH v3] Add kernel config option for tweaking kernel behavior.

[Tetsuo Handa]

Existing kernel config options are defined based on "whether you want to
enable this module/feature or not". And such granularity is sometimes
too rough-grained for fuzzing tools which try to find bugs inside each
module/feature.

While syzkaller (one of fuzzing tools) is finding many bugs, sometimes
syzkaller examines stupid operations. Some examples of such operations
are: changing console loglevel which in turn makes it impossible to get
kernel messages when a crash happens, freezing filesystems which in turn
causes khungtaskd to needlessly complain, programmatically sending
Ctrl-Alt-Del which in turn causes the system to needlessly reboot.
Currently we prevent syzkaller from examining stupid operations by
blacklisting syscall arguments and/or disabling whole functionality
using existing kernel config options. But such approach is difficult to
maintain and needlessly prevents fuzzers from testing kernel code. [1]

We want fuzzers to test as much coverage as possible while we want
fuzzers not to try stupid operations. To achieve this goal, we want
cooperation from kernel side, and build-time branching (i.e. kernel
config options) will be the simplest and the most reliable.

Therefore, this patch introduces a kernel config option which allows
selecting fine-grained kernel config options for tweaking kernel's
behavior. Each fine-grained kernel config option will be added by future
patches. For ease of management, grouping kernel config options for
allowing e.g. syzkaller to select all fine-grained kernel config options
which e.g. syzkaller wants would be added by future patches.

[1] https://lkml.kernel.org/r/CACT4Y+a6KExbggs4mg8pvoD554PcDqQNW4sM15X-tc=YONCzYw@mail.gmail.com

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Dmitry Vyukov <dvyukov@google.com>
---
 lib/Kconfig.debug | 11 +++++++++++
 1 file changed, 11 insertions(+)

Changes since v2 ( https://lkml.kernel.org/r/20200307135822.3894-1-penguin-kernel@I-love.SAKURA.ne.jp ):
  Reduce the role of this kernel config option from "enable everything
  which would be useful for fuzz testing" to "simply serve as a gate for
  hiding individual kernel config option", for we should use individual
  kernel config option for tweaking individual kernel behavior.

Changes since v1 ( https://lkml.kernel.org/r/20191216095955.9886-1-penguin-kernel@I-love.SAKURA.ne.jp ):
  Drop users of this kernel config option.
  Update patch description.

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 50c1f5f08e6f..a7c3ebc21428 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2223,4 +2223,15 @@ config HYPERV_TESTING
 
 endmenu # "Kernel Testing and Coverage"
 
+menuconfig TWEAK_KERNEL_BEHAVIOR
+	bool "Tweak kernel behavior"
+	help
+	  Saying Y here allows modifying kernel behavior via kernel
+	  config options which will become visible by selecting this
+	  config option.
+
+if TWEAK_KERNEL_BEHAVIOR
+
+endif # TWEAK_KERNEL_BEHAVIOR
+
 endmenu # Kernel hacking
-- 
2.18.2

Re: [PATCH v3] Add kernel config option for tweaking kernel behavior.

[Greg Kroah-Hartman]

On Mon, Apr 13, 2020 at 03:33:17PM +0900, Tetsuo Handa wrote:
> Existing kernel config options are defined based on "whether you want to
> enable this module/feature or not". And such granularity is sometimes
> too rough-grained for fuzzing tools which try to find bugs inside each
> module/feature.
> 
> While syzkaller (one of fuzzing tools) is finding many bugs, sometimes
> syzkaller examines stupid operations. Some examples of such operations
> are: changing console loglevel which in turn makes it impossible to get
> kernel messages when a crash happens, freezing filesystems which in turn
> causes khungtaskd to needlessly complain, programmatically sending
> Ctrl-Alt-Del which in turn causes the system to needlessly reboot.
> Currently we prevent syzkaller from examining stupid operations by
> blacklisting syscall arguments and/or disabling whole functionality
> using existing kernel config options. But such approach is difficult to
> maintain and needlessly prevents fuzzers from testing kernel code. [1]
> 
> We want fuzzers to test as much coverage as possible while we want
> fuzzers not to try stupid operations. To achieve this goal, we want
> cooperation from kernel side, and build-time branching (i.e. kernel
> config options) will be the simplest and the most reliable.
> 
> Therefore, this patch introduces a kernel config option which allows
> selecting fine-grained kernel config options for tweaking kernel's
> behavior. Each fine-grained kernel config option will be added by future
> patches. For ease of management, grouping kernel config options for
> allowing e.g. syzkaller to select all fine-grained kernel config options
> which e.g. syzkaller wants would be added by future patches.
> 
> [1] https://lkml.kernel.org/r/CACT4Y+a6KExbggs4mg8pvoD554PcDqQNW4sM15X-tc=YONCzYw@mail.gmail.com
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> ---
>  lib/Kconfig.debug | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> Changes since v2 ( https://lkml.kernel.org/r/20200307135822.3894-1-penguin-kernel@I-love.SAKURA.ne.jp ):
>   Reduce the role of this kernel config option from "enable everything
>   which would be useful for fuzz testing" to "simply serve as a gate for
>   hiding individual kernel config option", for we should use individual
>   kernel config option for tweaking individual kernel behavior.
> 
> Changes since v1 ( https://lkml.kernel.org/r/20191216095955.9886-1-penguin-kernel@I-love.SAKURA.ne.jp ):
>   Drop users of this kernel config option.
>   Update patch description.
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 50c1f5f08e6f..a7c3ebc21428 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2223,4 +2223,15 @@ config HYPERV_TESTING
>  
>  endmenu # "Kernel Testing and Coverage"
>  
> +menuconfig TWEAK_KERNEL_BEHAVIOR
> +	bool "Tweak kernel behavior"
> +	help
> +	  Saying Y here allows modifying kernel behavior via kernel
> +	  config options which will become visible by selecting this
> +	  config option.

This "help" text really only says "say Y here to allow for some config
options".  It doesn't explain what these are for, or why anyone would
select them, or what type of options are here at all.

I don't see how this option, by just looking at it, relates to your goal
of doing things to make fuzzers' lives easier.

thanks,

greg k-h

Re: [PATCH v3] Add kernel config option for tweaking kernel behavior.

[Tetsuo Handa]

On 2020/04/13 17:14, Greg Kroah-Hartman wrote:
>> @@ -2223,4 +2223,15 @@ config HYPERV_TESTING
>>  
>>  endmenu # "Kernel Testing and Coverage"
>>  
>> +menuconfig TWEAK_KERNEL_BEHAVIOR
>> +	bool "Tweak kernel behavior"
>> +	help
>> +	  Saying Y here allows modifying kernel behavior via kernel
>> +	  config options which will become visible by selecting this
>> +	  config option.
> 
> This "help" text really only says "say Y here to allow for some config
> options".  It doesn't explain what these are for, or why anyone would
> select them, or what type of options are here at all.
> 
> I don't see how this option, by just looking at it, relates to your goal
> of doing things to make fuzzers' lives easier.

Well, we could add some more text (like shown below), but this option itself
is neutral. This option is not limiting the target to fuzzers.
Below 3 patches are an example of "a set of fine-grained configs" with
"some umbrella uber-config" approach. Linus, are you OK with this approach?



From 23b585fdd0ed55710285a6d67faffe63cebc098f Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sat, 18 Apr 2020 22:03:04 +0900
Subject: [PATCH 1/3] Add kernel config option for tweaking kernel behavior.

Existing kernel config options are defined based on "whether you want to
enable this module/feature or not". And such granularity is sometimes
too rough-grained for fuzzing tools which try to find bugs inside each
module/feature.

While syzkaller (one of fuzzing tools) is finding many bugs, sometimes
syzkaller examines stupid operations. Some examples of such operations
are: changing console loglevel which in turn makes it impossible to get
kernel messages when a crash happens, freezing filesystems which in turn
causes khungtaskd to needlessly complain, programmatically sending
Ctrl-Alt-Del which in turn causes the system to needlessly reboot.
Currently we prevent syzkaller from examining stupid operations by
blacklisting syscall arguments and/or disabling whole functionality
using existing kernel config options. But such approach is difficult to
maintain and needlessly prevents fuzzers from testing kernel code. [1]

We want fuzzers to test as much coverage as possible while we want
fuzzers not to try stupid operations. To achieve this goal, we want
cooperation from kernel side, and build-time branching (i.e. kernel
config options) will be the simplest and the most reliable.

Therefore, this patch introduces a kernel config option which allows
selecting fine-grained kernel config options for tweaking kernel's
behavior. Each fine-grained kernel config option will be added by future
patches. For ease of management, grouping kernel config options for
allowing e.g. syzkaller to select all fine-grained kernel config options
which e.g. syzkaller wants would be added by future patches.

[1] https://lkml.kernel.org/r/CACT4Y+a6KExbggs4mg8pvoD554PcDqQNW4sM15X-tc=YONCzYw@mail.gmail.com

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 lib/Kconfig.debug |  2 ++
 lib/Kconfig.tewak | 13 +++++++++++++
 2 files changed, 15 insertions(+)
 create mode 100644 lib/Kconfig.tewak

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 21d9c5f6e7ec..e6162595ef9d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2225,4 +2225,6 @@ config HYPERV_TESTING
 
 endmenu # "Kernel Testing and Coverage"
 
+source "lib/Kconfig.tewak"
+
 endmenu # Kernel hacking
diff --git a/lib/Kconfig.tewak b/lib/Kconfig.tewak
new file mode 100644
index 000000000000..a5ce0db67f28
--- /dev/null
+++ b/lib/Kconfig.tewak
@@ -0,0 +1,13 @@
+menuconfig TWEAK_KERNEL_BEHAVIOR
+	bool "Tweak kernel behavior"
+	help
+	  Saying Y here allows modifying kernel behavior via kernel
+	  config options which will become visible by selecting this
+	  config option. Since these kernel config options are intended
+	  for helping e.g. fuzz testing, behavior tweaked by this kernel
+	  option might be unstable. Userspace applications should not
+	  count on this option being selected.
+
+if TWEAK_KERNEL_BEHAVIOR
+
+endif # TWEAK_KERNEL_BEHAVIOR
-- 
2.18.2

From f7f668896e188217b50c50a31c4ad1acb7556b36 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sat, 18 Apr 2020 22:39:38 +0900
Subject: [PATCH 2/3] tweak: Allow disabling k_spec() function in drivers/tty/vt/keyboard.c

syzbot is reporting unexpected kernel reboots [1]. This seems to be
caused by triggering Ctrl-Alt-Del event via k_spec() function in
drivers/tty/vt/keyboard.c file, for the console output includes normal
restart sequence. Therefore, allow disabling only k_spec() function
in order to allow fuzzers to examine the remaining part in that file.

[1] https://syzkaller.appspot.com/bug?id=321861b1588b44d064b779b92293c5d55cfe8430

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 drivers/tty/vt/keyboard.c | 2 ++
 lib/Kconfig.tewak         | 7 +++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index 15d33fa0c925..f08855c4c5ba 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -633,6 +633,8 @@ static void k_spec(struct vc_data *vc, unsigned char value, char up_flag)
 	     kbd->kbdmode == VC_OFF) &&
 	     value != KVAL(K_SAK))
 		return;		/* SAK is allowed even in raw mode */
+	if (IS_ENABLED(CONFIG_TWEAK_DISABLE_KBD_K_SPEC_HANDLER))
+		return;
 	fn_handler[value](vc);
 }
 
diff --git a/lib/Kconfig.tewak b/lib/Kconfig.tewak
index a5ce0db67f28..a1d038bcc2a5 100644
--- a/lib/Kconfig.tewak
+++ b/lib/Kconfig.tewak
@@ -10,4 +10,11 @@ menuconfig TWEAK_KERNEL_BEHAVIOR
 
 if TWEAK_KERNEL_BEHAVIOR
 
+config TWEAK_DISABLE_KBD_K_SPEC_HANDLER
+       bool "Disable k_spec() function in drivers/tty/vt/keyboard.c"
+       help
+	 k_spec() function allows triggering e.g. Ctrl-Alt-Del event.
+	 Such event is annoying for fuzz testing which wants to test
+	 kernel code without rebooting the system.
+
 endif # TWEAK_KERNEL_BEHAVIOR
-- 
2.18.2

From 5cf7f2ef1d470ea980c6b9a21437d37279d13d12 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sat, 18 Apr 2020 23:12:16 +0900
Subject: [PATCH 3/3] tweak: Add option for selecting tweak options for syzkaller's testing

When a kernel developer adds a kernel config option, they will not go
and update configs on all external testing systems. This problem is also
common for "enable all boot tests that can run on this kernel", or
"configure a 'standard' debug build". Currently doing these things
require all of expertise, sacred knowledge, checking all configs
one-by-one as well as checking every new kernel patch and that needs to
be done by everybody doing any kernel testing.

In order to reduce the burden of maintaining kernel config options, this
patch introduces a kernel config option which will select tweak options
when building kernels for syzkaller's testing.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
---
 lib/Kconfig.tewak | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/Kconfig.tewak b/lib/Kconfig.tewak
index a1d038bcc2a5..95020a56bbfc 100644
--- a/lib/Kconfig.tewak
+++ b/lib/Kconfig.tewak
@@ -10,6 +10,12 @@ menuconfig TWEAK_KERNEL_BEHAVIOR
 
 if TWEAK_KERNEL_BEHAVIOR
 
+config TWEAK_FOR_SYZKALLER_TESTING
+       bool "Select all tewak options suitable for syzkaller testing"
+       select TWEAK_DISABLE_KBD_K_SPEC_HANDLER
+       help
+	 Say N unless you are building kernels for syzkaller's testing.
+
 config TWEAK_DISABLE_KBD_K_SPEC_HANDLER
        bool "Disable k_spec() function in drivers/tty/vt/keyboard.c"
        help
-- 
2.18.2

Re: [PATCH v3] Add kernel config option for tweaking kernel behavior.

[Greg Kroah-Hartman]

On Sat, Apr 18, 2020 at 11:28:22PM +0900, Tetsuo Handa wrote:
> On 2020/04/13 17:14, Greg Kroah-Hartman wrote:
> >> @@ -2223,4 +2223,15 @@ config HYPERV_TESTING
> >>  
> >>  endmenu # "Kernel Testing and Coverage"
> >>  
> >> +menuconfig TWEAK_KERNEL_BEHAVIOR
> >> +	bool "Tweak kernel behavior"
> >> +	help
> >> +	  Saying Y here allows modifying kernel behavior via kernel
> >> +	  config options which will become visible by selecting this
> >> +	  config option.
> > 
> > This "help" text really only says "say Y here to allow for some config
> > options".  It doesn't explain what these are for, or why anyone would
> > select them, or what type of options are here at all.
> > 
> > I don't see how this option, by just looking at it, relates to your goal
> > of doing things to make fuzzers' lives easier.
> 
> Well, we could add some more text (like shown below), but this option itself
> is neutral. This option is not limiting the target to fuzzers.
> Below 3 patches are an example of "a set of fine-grained configs" with
> "some umbrella uber-config" approach. Linus, are you OK with this approach?

Note, the word "tweak" is usually used where people want to get the most
performance out of a system, while here you are using it to remove
functionality out of the system.  You might want to pick a different
word, naming is hard :(

Anyway, a comment on your patch:

> >From f7f668896e188217b50c50a31c4ad1acb7556b36 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Sat, 18 Apr 2020 22:39:38 +0900
> Subject: [PATCH 2/3] tweak: Allow disabling k_spec() function in drivers/tty/vt/keyboard.c
> 
> syzbot is reporting unexpected kernel reboots [1]. This seems to be
> caused by triggering Ctrl-Alt-Del event via k_spec() function in
> drivers/tty/vt/keyboard.c file, for the console output includes normal
> restart sequence. Therefore, allow disabling only k_spec() function
> in order to allow fuzzers to examine the remaining part in that file.
> 
> [1] https://syzkaller.appspot.com/bug?id=321861b1588b44d064b779b92293c5d55cfe8430
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
>  drivers/tty/vt/keyboard.c | 2 ++
>  lib/Kconfig.tewak         | 7 +++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
> index 15d33fa0c925..f08855c4c5ba 100644
> --- a/drivers/tty/vt/keyboard.c
> +++ b/drivers/tty/vt/keyboard.c
> @@ -633,6 +633,8 @@ static void k_spec(struct vc_data *vc, unsigned char value, char up_flag)
>  	     kbd->kbdmode == VC_OFF) &&
>  	     value != KVAL(K_SAK))
>  		return;		/* SAK is allowed even in raw mode */
> +	if (IS_ENABLED(CONFIG_TWEAK_DISABLE_KBD_K_SPEC_HANDLER))
> +		return;

Are you sure this is correct?  It seems like you just cut off a number
of other keyboard function handlers instead of just the ctrl-alt-del
functionality.  Did you really want to do that?

thanks,

greg k-h

Re: [PATCH v3] Add kernel config option for tweaking kernel behavior.

[Tetsuo Handa]

On 2020/04/18 23:39, Greg Kroah-Hartman wrote:
>> Well, we could add some more text (like shown below), but this option itself
>> is neutral. This option is not limiting the target to fuzzers.
>> Below 3 patches are an example of "a set of fine-grained configs" with
>> "some umbrella uber-config" approach. Linus, are you OK with this approach?
> 
> Note, the word "tweak" is usually used where people want to get the most
> performance out of a system, while here you are using it to remove
> functionality out of the system.  You might want to pick a different
> word, naming is hard :(

Then, what about "twist" ?

>> @@ -633,6 +633,8 @@ static void k_spec(struct vc_data *vc, unsigned char value, char up_flag)
>>  	     kbd->kbdmode == VC_OFF) &&
>>  	     value != KVAL(K_SAK))
>>  		return;		/* SAK is allowed even in raw mode */
>> +	if (IS_ENABLED(CONFIG_TWEAK_DISABLE_KBD_K_SPEC_HANDLER))
>> +		return;
> 
> Are you sure this is correct?  It seems like you just cut off a number
> of other keyboard function handlers instead of just the ctrl-alt-del
> functionality.  Did you really want to do that?

Maybe someday we update this filter more fine-grained. But I think this
granularity is what syzkaller wants for now. The ctrl-alt-del ( k_spec()
=> fn_boot_it() => ctrl_alt_del()) is simply one of them. There was RCU
stall report caused by repeating SysRq-t from this path ( k_spec() =>
fn_show_state() => show_state()) due to too much printk() calls.

Re: [PATCH v3] Add kernel config option for tweaking kernel behavior.

[Linus Torvalds]

On Sun, Apr 12, 2020 at 11:34 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> Existing kernel config options are defined based on "whether you want to
> enable this module/feature or not". And such granularity is sometimes
> too rough-grained for fuzzing tools which try to find bugs inside each
> module/feature.

I still detest making this a hardcoded build-time config option.

A kernel parameter that sets a flag seems much simpler. More
importantly, having it be something sanely named, and something you
can set independently some other way, would allow a regular kernel to
then run a fuzzer as root.

Some kind of "not even root" flag, which might be per-process and not
possible to clear once set (so that your _normal_ system binaries
could still do the root-only stuff, but then you could start a fuzzing
process with that flag set, knowing that the fuzzing process - and
it's children - are not able to do things).

Honestly, in a perfect world, it has nothing at all to do with
fuzzing, and people could even have some local rules like "anybody who
came in through ssh from the network will also have this flag set".

                 Linus

Re: [PATCH v3] Add kernel config option for tweaking kernel behavior.

[Tetsuo Handa]

On 2020/04/14 3:13, Linus Torvalds wrote:
> On Sun, Apr 12, 2020 at 11:34 PM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
>>
>> Existing kernel config options are defined based on "whether you want to
>> enable this module/feature or not". And such granularity is sometimes
>> too rough-grained for fuzzing tools which try to find bugs inside each
>> module/feature.
> 
> I still detest making this a hardcoded build-time config option.
> 
> A kernel parameter that sets a flag seems much simpler. More
> importantly, having it be something sanely named, and something you
> can set independently some other way, would allow a regular kernel to
> then run a fuzzer as root.

That approach involves declaration of user-visible behavioral ABI which
will become impossible to change or remove if we once published.

Also, syzkaller does not want to depend on userspace being rich. There are
userspace tools (e.g. SystemTap) that can tweak kernel's behavior. If we could
use SystemTap, we could do some of tweaks using SystemTap. Likewise, use of
runtime flags which can be set from userspace involves modification of userspace
programs, which is a sort of dependency syzkaller does not want to maintain.

The advantage of build-time tweaking is that we don't need to maintain behavioral
ABI. We can change what tweak is doing over time (e.g. narrowing down what the
build-time tweaking guards, improving how to get more useful information).

> 
> Some kind of "not even root" flag, which might be per-process and not
> possible to clear once set (so that your _normal_ system binaries
> could still do the root-only stuff, but then you could start a fuzzing
> process with that flag set, knowing that the fuzzing process - and
> it's children - are not able to do things).

I don't think per-process flags will work. Not every action is taken inside
process context who issued a syscall. Some action might be taken in interrupt
context or in kthread context. Since we have no means to propagate per-process
flags, we will need to give up more than we have to give up.

> 
> Honestly, in a perfect world, it has nothing at all to do with
> fuzzing, and people could even have some local rules like "anybody who
> came in through ssh from the network will also have this flag set".

I think that such restriction should be implemented as LSM modules.

Re: [PATCH v3] Add kernel config option for tweaking kernel behavior.

[Tetsuo Handa]

On 2020/04/14 17:02, Tetsuo Handa wrote:
>> Some kind of "not even root" flag, which might be per-process and not
>> possible to clear once set (so that your _normal_ system binaries
>> could still do the root-only stuff, but then you could start a fuzzing
>> process with that flag set, knowing that the fuzzing process - and
>> it's children - are not able to do things).
> 
> I don't think per-process flags will work. Not every action is taken inside
> process context who issued a syscall. Some action might be taken in interrupt
> context or in kthread context. Since we have no means to propagate per-process
> flags, we will need to give up more than we have to give up.

An example from https://syzkaller.appspot.com/bug?id=fc28634f4815322260d0735ad0ed14f767b558b6 (currently the third top crasher in "open" table).

------------[ cut here ]------------
WARNING: CPU: 1 PID: 3668 at net/core/dev.c:8776 rollback_registered_many+0x1ed/0xec0 net/core/dev.c:8776
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 3668 Comm: kworker/1:6 Not tainted 5.6.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xef/0x16e lib/dump_stack.c:118
 panic+0x2aa/0x6e1 kernel/panic.c:221
 __warn.cold+0x2f/0x30 kernel/panic.c:582
 report_bug+0x27b/0x2f0 lib/bug.c:195
 fixup_bug arch/x86/kernel/traps.c:174 [inline]
 fixup_bug arch/x86/kernel/traps.c:169 [inline]
 do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
 do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
 invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:rollback_registered_many+0x1ed/0xec0 net/core/dev.c:8776
Code: 05 00 00 31 ff 44 89 fe e8 10 ac 78 fc 45 84 ff 0f 85 49 ff ff ff e8 d2 aa 78 fc 0f 1f 44 00 00 e8 c8 aa 78 fc e8 c3 aa 78 fc <0f> 0b 48 89 ef e8 59 2e 24 fd 31 ff 89 c5 89 c6 e8 de ab 78 fc 40
RSP: 0018:ffff8881d4b7f640 EFLAGS: 00010216
RAX: 0000000000040000 RBX: ffff8881d1620000 RCX: ffffc9000d3f6000
RDX: 0000000000003912 RSI: ffffffff84c6ad5d RDI: 0000000000000001
RBP: ffff8881d1620068 R08: ffff8881c74d9880 R09: fffffbfff0f876c5
R10: fffffbfff0f876c4 R11: ffffffff87c3b627 R12: ffff8881d4b7f788
R13: dffffc0000000000 R14: ffff8881d4b7f720 R15: 0000000000000000
 rollback_registered+0xf2/0x1c0 net/core/dev.c:8855
 unregister_netdevice_queue net/core/dev.c:9947 [inline]
 unregister_netdevice_queue+0x1d7/0x2b0 net/core/dev.c:9940
 unregister_netdevice include/linux/netdevice.h:2715 [inline]
 unregister_netdev+0x18/0x20 net/core/dev.c:9988
 r871xu_dev_remove+0xe2/0x215 drivers/staging/rtl8712/usb_intf.c:604
 usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:436
 __device_release_driver drivers/base/dd.c:1137 [inline]
 device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 usb_disable_device+0x23d/0x790 drivers/usb/core/message.c:1238
 usb_disconnect+0x293/0x900 drivers/usb/core/hub.c:2211
 hub_port_connect drivers/usb/core/hub.c:5046 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5335 [inline]
 port_event drivers/usb/core/hub.c:5481 [inline]
 hub_event+0x1a1d/0x4300 drivers/usb/core/hub.c:5563
 process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
 process_scheduled_works kernel/workqueue.c:2328 [inline]
 worker_thread+0x7ab/0xe20 kernel/workqueue.c:2414
 kthread+0x318/0x420 kernel/kthread.c:255
 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

8767         list_for_each_entry_safe(dev, tmp, head, unreg_list) {
8768                 /* Some devices call without registering
8769                  * for initialization unwind. Remove those
8770                  * devices and proceed with the remaining.
8771                  */
8772                 if (dev->reg_state == NETREG_UNINITIALIZED) {
8773                         pr_debug("unregister_netdevice: device %s/%p never was registered\n",
8774                                  dev->name, dev);
8775 
8776                         WARN_ON(1);
8777                         list_del(&dev->unreg_list);
8778                         continue;
8779                 }
8780                 dev->dismantle = true;
8781                 BUG_ON(dev->reg_state != NETREG_REGISTERED);
8782         }

I can't judge whether this WARN_ON() makes sense outside of syzkaller.
But I'd like to suppress this WARN_ON() under testing by syzkaller so that
syzkaller can spend resource for seeking for different bugs (assuming that
suppressing this WARN_ON() is harmless). To suppress this WARN_ON(),
per-process flags (e.g. current->dont_hit_warn_on_at_rollback_registered_many )
cannot work because this is a Workqueue context. And user-visible global flags
(e.g. sysctl_dont_hit_warn_on_at_rollback_registered_many ) is too much.

An approach for TOMOYO to solve this kind of problem is to guard using
a kernel config option for fuzz testing.

  commit e80b18599a39a625 ("tomoyo: Add a kernel config option for fuzzing testing.")
  commit 4ad98ac46490d5f8 ("tomoyo: Don't emit WARNING: string while fuzzing testing.")

This is not something that should publish permanently user-visible flags.

Re: [PATCH v3] Add kernel config option for tweaking kernel behavior.

[Linus Torvalds]

On Tue, Apr 14, 2020 at 5:06 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> I can't judge whether this WARN_ON() makes sense outside of syzkaller.
> But I'd like to suppress this WARN_ON() under testing by syzkaller so that
> syzkaller can spend resource for seeking for different bugs (assuming that
> suppressing this WARN_ON() is harmless).

That's crazy talk.

Either that WARN_ON() is valid, or it's not. It doesn't matter if it's
syzcaller, and we would never change the behavior of it depending on a
flag - compile-time or dynamic.

There's no way we'd accept that as some "#ifndef CONFIG_FUZZER" thing either.

If that WARN_ON() is a problem, then the people behind it should be
appraised of it, and it should probably be removed. I'm assuming it
was some kind of "I don't think this can happen, so if it does, I want
to see how it happened" WARN_ON.

                Linus

Re: [PATCH v3] Add kernel config option for tweaking kernel behavior.

[Steven Rostedt]

On Tue, 14 Apr 2020 18:14:51 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> If that WARN_ON() is a problem, then the people behind it should be
> appraised of it, and it should probably be removed. I'm assuming it
> was some kind of "I don't think this can happen, so if it does, I want
> to see how it happened" WARN_ON.

What I do (and feel everyone should too), is I only add a WARN_ON()
when I check something that I believe *can't* happen. If the WARN_ON()
triggers, it either means that there's a bug in the code (and needs a
fix), or the code design changed, in which case the WARN_ON() should be
either removed, or that code updated to handle the new change.

In any case, a WARN_ON() should always be investigated when hit.

The only time I've had issues with people is when I have some hardware
(i915) that triggers a WARN_ON() and I'm told that my hardware is buggy
(or I need a firmware update). In which case, I just manually remove
the WARN_ON() because the machine that triggers it is a test machine I
don't have the time to waste updating firmware on.

-- Steve

Re: [PATCH v3] Add kernel config option for tweaking kernel behavior.

[Tetsuo Handa]

On 2020/04/14 3:13, Linus Torvalds wrote:
> On Sun, Apr 12, 2020 at 11:34 PM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
>>
>> Existing kernel config options are defined based on "whether you want to
>> enable this module/feature or not". And such granularity is sometimes
>> too rough-grained for fuzzing tools which try to find bugs inside each
>> module/feature.
> 
> I still detest making this a hardcoded build-time config option.

But I think that starting from build-time config option is the better.

> 
> A kernel parameter that sets a flag seems much simpler. More
> importantly, having it be something sanely named, and something you
> can set independently some other way, would allow a regular kernel to
> then run a fuzzer as root.

We can't beforehand determine how many kernel parameters will be needed.

I don't think the number of kernel parameters is "one". You said "I'd *much*
rather see some way to just lock down certain things individually.". Also,
different fuzzing mechanisms may want different set of things (e.g.
current->do_not_emit_warning_string for syzkaller described bottom).

But will the number of kernel parameters be "a few" ? "a dozen" ?
"some dozen" ? "some hundreds" ? "one thousand" ?

Of course, I'm not expecting "one thousand". But I can't say how many kernel
parameters will be needed. The number of kernel parameters can be determined
only after we identify what "things" we need to tweak. The first step is to
identify which operation should be tweaked, and it will take *very long time*.
I don't think that it is realistic to update userspace programs to support that
flag whenever a new flag was added (with sane name and explanation for users
who are not interested in running fuzzers on their kernels) in the kernel side.

> 
> Some kind of "not even root" flag, which might be per-process and not
> possible to clear once set (so that your _normal_ system binaries
> could still do the root-only stuff, but then you could start a fuzzing
> process with that flag set, knowing that the fuzzing process - and
> it's children - are not able to do things).

I don't think that fuzzing mechanisms is clever enough to distinguish which
process (fuzz tests or _normal_ system binaries) is causing the unwanted
result (e.g. shutting down the system). If administrator's proper operation
(e.g. freezing a filesystem for maintenance purpose) caused a warning (e.g.
khungtaskd complains that file write operation cannot be completed), fuzzing
mechanisms (e.g. monitoring kernel messages printed to consoles) will consider
it as "crash". Since who did "dangerous operation" is irrelevant, who can do
"dangerous operation" (e.g. use of per-process flags) is irrelevant. Doing

--- a/security/tomoyo/util.c
+++ b/security/tomoyo/util.c
@@ -1078,10 +1078,9 @@ bool tomoyo_domain_quota_is_ok(struct tomoyo_request_info *r)
                domain->flags[TOMOYO_DIF_QUOTA_WARNED] = true;
                /* r->granted = false; */
                tomoyo_write_log(r, "%s", tomoyo_dif[TOMOYO_DIF_QUOTA_WARNED]);
-#ifndef CONFIG_SECURITY_TOMOYO_INSECURE_BUILTIN_SETTING
-               pr_warn("WARNING: Domain '%s' has too many ACLs to hold. Stopped learning mode.\n",
-                       domain->domainname->name);
-#endif
+               if (current->do_not_emit_warning_string)
+                       pr_warn("WARNING: Domain '%s' has too many ACLs to hold. Stopped learning mode.\n",
+                               domain->domainname->name);
        }
        return false;
 }

is pointless for syzkaller; syzkaller will treat as "crash" as soon as _normal_
system binaries hit this path. Use of build-time config option is simpler.