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

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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