Re: [PATCH v2] twist: allow converting pr_devel()/pr_debug() into snprintf()
11 views
Skip to first unread message
Tetsuo Handa
May 29, 2020, 9:27:35 AM5/29/20
to Dmitry Vyukov, syzkaller, Linus Torvalds, Petr Mladek, Andrew Morton, Linux Kernel Mailing List, Ondrej Mosnacek, Sergey Senozhatsky, Steven Rostedt
Hello, Dmitry.
Linus is asking me to avoid build-time switching based on kernel config options,
and is suggesting me to use boot-time switching based on boot-config file feature
(which is available since 5.6). I have several concerns about use of boot-config file
feature in syzkaller.
(1) To use boot-config file, syzkaller will need to add "bootconfig" option
to the kernel command line. This will be doable by patching
https://github.com/google/syzkaller/tree/master/dashboard/config/ *.cmdline
files.
(2) The boot-config file is embedded into initramfs file. Since syzkaller builds
kernels with almost-allyesconfig, booting syzkaller kernels do not require
initramfs for loading kernel modules needed for mounting the root partition.
In fact, according to "unexpected kernel reboot" report which contains boot messages,
I can't find "Unpacking initramfs..." message. It seems that syzkaller kernels do not
use initramfs file.
Is it possible for syzkaller to enforce performing steps for creating an initramfs,
embedding the boot-config file
( https://www.kernel.org/doc/html/latest/admin-guide/bootconfig.html#boot-kernel-with-a-boot-config),
and loading that initramfs whenever booting the syzkaller kernels?
By the way, I do worry that people forget to perform these steps when they do
their tests without asking syzbot...
(3) Since syzkaller keeps track of "kernel tree", "commit of the kernel tree", and
"commit of the syzkaller tree" in order to guarantee reproducibility, it would be
possible to identify the "your-config" file used for tools/bootconfig/bootconfig
command. But since "#syz test" command currently accepts only "kernel tree" and
"commit of the kernel tree" arguments, we might fail to use intended "your-config"
file when doing reproducibility test. Can syzbot solve this concern?
(4) Of course, "your-config" file would not change so frequently, but "#syz test" command
relies on external file in "syzkaller tree" makes it impossible to try different
configuration when I have to ask syzbot to test. (Since I don't have hardware which
syzbot is reporting problems, I have to ask syzbot when I can't reproduce the problem
in my environment.)
https://syzkaller.appspot.com/text?tag=Patch&x=135f254a100000 is an example of
need to enforce CONFIG_DEBUG_INFO_BTF=n in order to workaround build failure during
"#syz test" command. If we bring "which twist options should be enabled" to an external
boot-config file, I can't ask syzbot to try different twist options (except directly
patching the kernel source which handles "which twist options should be enabled").
Can syzbot solve this concern?
(5) Anything else?
Linus is asking me to avoid build-time switching based on kernel config options,
and is suggesting me to use boot-time switching based on boot-config file feature
(which is available since 5.6). I have several concerns about use of boot-config file
feature in syzkaller.
(1) To use boot-config file, syzkaller will need to add "bootconfig" option
to the kernel command line. This will be doable by patching
https://github.com/google/syzkaller/tree/master/dashboard/config/ *.cmdline
files.
(2) The boot-config file is embedded into initramfs file. Since syzkaller builds
kernels with almost-allyesconfig, booting syzkaller kernels do not require
initramfs for loading kernel modules needed for mounting the root partition.
In fact, according to "unexpected kernel reboot" report which contains boot messages,
I can't find "Unpacking initramfs..." message. It seems that syzkaller kernels do not
use initramfs file.
Is it possible for syzkaller to enforce performing steps for creating an initramfs,
embedding the boot-config file
( https://www.kernel.org/doc/html/latest/admin-guide/bootconfig.html#boot-kernel-with-a-boot-config),
and loading that initramfs whenever booting the syzkaller kernels?
By the way, I do worry that people forget to perform these steps when they do
their tests without asking syzbot...
(3) Since syzkaller keeps track of "kernel tree", "commit of the kernel tree", and
"commit of the syzkaller tree" in order to guarantee reproducibility, it would be
possible to identify the "your-config" file used for tools/bootconfig/bootconfig
command. But since "#syz test" command currently accepts only "kernel tree" and
"commit of the kernel tree" arguments, we might fail to use intended "your-config"
file when doing reproducibility test. Can syzbot solve this concern?
(4) Of course, "your-config" file would not change so frequently, but "#syz test" command
relies on external file in "syzkaller tree" makes it impossible to try different
configuration when I have to ask syzbot to test. (Since I don't have hardware which
syzbot is reporting problems, I have to ask syzbot when I can't reproduce the problem
in my environment.)
https://syzkaller.appspot.com/text?tag=Patch&x=135f254a100000 is an example of
need to enforce CONFIG_DEBUG_INFO_BTF=n in order to workaround build failure during
"#syz test" command. If we bring "which twist options should be enabled" to an external
boot-config file, I can't ask syzbot to try different twist options (except directly
patching the kernel source which handles "which twist options should be enabled").
Can syzbot solve this concern?
(5) Anything else?
Tetsuo Handa
Jun 3, 2020, 7:04:10 AM6/3/20
to Dmitry Vyukov, syzkaller, Linus Torvalds, Petr Mladek, Andrew Morton, Linux Kernel Mailing List, Ondrej Mosnacek, Sergey Senozhatsky, Steven Rostedt
On 2020/05/29 22:26, Tetsuo Handa wrote:
> By the way, I do worry that people forget to perform these steps when they do
> their tests without asking syzbot...
Here is a draft of boot-time switching. Since kconfig can handle string variable up to
> By the way, I do worry that people forget to perform these steps when they do
> their tests without asking syzbot...
2048 characters, we could hold the content of the "your-config" file inside .config file
in order to avoid relying on external file in "syzkaller tree". But since only one kconfig
option is used, basically the way to temporarily include/exclude specific options (under
automated testing by syzbot) seems to remain directly patching apply_twist_flags(), for
https://github.com/google/syzkaller/blob/master/dashboard/config/util.sh will automatically
overwrite CONFIG_DEFAULT_TWIST_FLAGS settings. If each twist flag were using independent
kconfig option, the way to temporarily include/exclude specific options will become directly
patching Kconfig file.
drivers/tty/vt/keyboard.c | 2 ++
include/linux/kernel.h | 8 ++++++++
init/main.c | 30 ++++++++++++++++++++++++++++++
kernel/reboot.c | 36 ++++++++++++++++++++++++++++++++++++
lib/Kconfig.debug | 5 +++++
5 files changed, 81 insertions(+)
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index 568b2171f335..ae0b7cd69249 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -637,6 +637,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 (twist_flags.disable_kbd_k_spec_handler)
+ return;
fn_handler[value](vc);
}
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 82d91547d122..78fdbb4f17b1 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -1038,4 +1038,12 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
/* OTHER_WRITABLE? Generally considered a bad idea. */ \
BUILD_BUG_ON_ZERO((perms) & 2) + \
(perms))
+
+/* Flags for twisting kernel behavior. */
+struct twist_flags {
+ bool disable_kbd_k_spec_handler;
+ bool disable_reboot_request;
+};
+extern struct twist_flags twist_flags;
+
#endif
diff --git a/init/main.c b/init/main.c
index 0ead83e86b5a..15eecd253b61 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1531,3 +1531,33 @@ static noinline void __init kernel_init_freeable(void)
integrity_load_keys();
}
+
+/* Flags for twisting kernel behavior. */
+struct twist_flags twist_flags __ro_after_init;
+EXPORT_SYMBOL(twist_flags);
+static __initdata char default_twist_flags[] __initdata = CONFIG_DEFAULT_TWIST_FLAGS;
+static __initdata char *chosen_twist_flags = default_twist_flags;
+
+static int __init overwrite_twist_flags(char *str)
+{
+ chosen_twist_flags = str;
+ return 1;
+}
+__setup("twist_flags=", overwrite_twist_flags);
+
+static int __init apply_twist_flags(void)
+{
+ char *flags = chosen_twist_flags;
+ char *name;
+
+ while ((name = strsep(&flags, ",")) != NULL) {
+ if (!strcmp(name, "kbd-disable-hotkeys"))
+ twist_flags.disable_kbd_k_spec_handler = true;
+ else if (!strcmp(name, "disable-reboot-request"))
+ twist_flags.disable_reboot_request = true;
+ else
+ printk(KERN_INFO "Ignoring unknown twist option '%s'.\n", name);
+ }
+ return 0;
+}
+late_initcall(apply_twist_flags);
diff --git a/kernel/reboot.c b/kernel/reboot.c
index 491f1347bf43..63cec97a9e59 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -63,6 +63,8 @@ EXPORT_SYMBOL_GPL(pm_power_off_prepare);
*/
void emergency_restart(void)
{
+ if (twist_flags.disable_reboot_request)
+ panic("reboot request is disabled");
kmsg_dump(KMSG_DUMP_EMERG);
machine_emergency_restart();
}
@@ -243,6 +245,8 @@ void migrate_to_reboot_cpu(void)
*/
void kernel_restart(char *cmd)
{
+ if (twist_flags.disable_reboot_request)
+ panic("reboot request is disabled");
kernel_restart_prepare(cmd);
migrate_to_reboot_cpu();
syscore_shutdown();
@@ -270,6 +274,8 @@ static void kernel_shutdown_prepare(enum system_states state)
*/
void kernel_halt(void)
{
+ if (twist_flags.disable_reboot_request)
+ panic("reboot request is disabled");
kernel_shutdown_prepare(SYSTEM_HALT);
migrate_to_reboot_cpu();
syscore_shutdown();
@@ -286,6 +292,8 @@ EXPORT_SYMBOL_GPL(kernel_halt);
*/
void kernel_power_off(void)
{
+ if (twist_flags.disable_reboot_request)
+ panic("reboot request is disabled");
kernel_shutdown_prepare(SYSTEM_POWER_OFF);
if (pm_power_off_prepare)
pm_power_off_prepare();
@@ -344,6 +352,10 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
mutex_lock(&system_transition_mutex);
switch (cmd) {
case LINUX_REBOOT_CMD_RESTART:
+ if (twist_flags.disable_reboot_request) {
+ ret = -EPERM;
+ break;
+ }
kernel_restart(NULL);
break;
@@ -356,11 +368,19 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
break;
case LINUX_REBOOT_CMD_HALT:
+ if (twist_flags.disable_reboot_request) {
+ ret = -EPERM;
+ break;
+ }
kernel_halt();
do_exit(0);
panic("cannot halt");
case LINUX_REBOOT_CMD_POWER_OFF:
+ if (twist_flags.disable_reboot_request) {
+ ret = -EPERM;
+ break;
+ }
kernel_power_off();
do_exit(0);
break;
@@ -373,17 +393,29 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd,
}
buffer[sizeof(buffer) - 1] = '\0';
+ if (twist_flags.disable_reboot_request) {
+ ret = -EPERM;
+ break;
+ }
kernel_restart(buffer);
break;
#ifdef CONFIG_KEXEC_CORE
case LINUX_REBOOT_CMD_KEXEC:
+ if (twist_flags.disable_reboot_request) {
+ ret = -EPERM;
+ break;
+ }
ret = kernel_kexec();
break;
#endif
#ifdef CONFIG_HIBERNATION
case LINUX_REBOOT_CMD_SW_SUSPEND:
+ if (twist_flags.disable_reboot_request) {
+ ret = -EPERM;
+ break;
+ }
ret = hibernate();
break;
#endif
@@ -493,6 +525,8 @@ static DECLARE_WORK(poweroff_work, poweroff_work_func);
*/
void orderly_poweroff(bool force)
{
+ if (twist_flags.disable_reboot_request)
+ panic("reboot request is disabled");
if (force) /* do not override the pending "true" */
poweroff_force = true;
schedule_work(&poweroff_work);
@@ -514,6 +548,8 @@ static DECLARE_WORK(reboot_work, reboot_work_func);
*/
void orderly_reboot(void)
{
+ if (twist_flags.disable_reboot_request)
+ panic("reboot request is disabled");
schedule_work(&reboot_work);
}
EXPORT_SYMBOL_GPL(orderly_reboot);
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 498d344ea53a..41cfabc74ad7 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2338,4 +2338,9 @@ config HYPERV_TESTING
endmenu # "Kernel Testing and Coverage"
+menuconfig DEFAULT_TWIST_FLAGS
+ string "Default twist options (DANGEROUS)"
+ help
+ Don't specify anything unless you know what you are doing.
+
endmenu # Kernel hacking
Petr Mladek
Jun 3, 2020, 8:44:08 AM6/3/20
to Tetsuo Handa, Dmitry Vyukov, syzkaller, Linus Torvalds, Andrew Morton, Linux Kernel Mailing List, Ondrej Mosnacek, Sergey Senozhatsky, Steven Rostedt
On Wed 2020-06-03 20:03:28, Tetsuo Handa wrote:
> On 2020/05/29 22:26, Tetsuo Handa wrote:
> > By the way, I do worry that people forget to perform these steps when they do
> > their tests without asking syzbot...
>
> Here is a draft of boot-time switching. Since kconfig can handle string variable up to
> 2048 characters, we could hold the content of the "your-config" file inside .config file
> in order to avoid relying on external file in "syzkaller tree". But since only one kconfig
> option is used, basically the way to temporarily include/exclude specific options (under
> automated testing by syzbot) seems to remain directly patching apply_twist_flags(), for
> https://github.com/google/syzkaller/blob/master/dashboard/config/util.sh will automatically
> overwrite CONFIG_DEFAULT_TWIST_FLAGS settings. If each twist flag were using independent
> kconfig option, the way to temporarily include/exclude specific options will become directly
> patching Kconfig file.
>
> On 2020/05/29 22:26, Tetsuo Handa wrote:
> > By the way, I do worry that people forget to perform these steps when they do
> > their tests without asking syzbot...
>
> Here is a draft of boot-time switching. Since kconfig can handle string variable up to
> 2048 characters, we could hold the content of the "your-config" file inside .config file
> in order to avoid relying on external file in "syzkaller tree". But since only one kconfig
> option is used, basically the way to temporarily include/exclude specific options (under
> automated testing by syzbot) seems to remain directly patching apply_twist_flags(), for
> https://github.com/google/syzkaller/blob/master/dashboard/config/util.sh will automatically
> overwrite CONFIG_DEFAULT_TWIST_FLAGS settings. If each twist flag were using independent
> kconfig option, the way to temporarily include/exclude specific options will become directly
> patching Kconfig file.
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 82d91547d122..78fdbb4f17b1 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -1038,4 +1038,12 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
> /* OTHER_WRITABLE? Generally considered a bad idea. */ \
> BUILD_BUG_ON_ZERO((perms) & 2) + \
> (perms))
> +
> +/* Flags for twisting kernel behavior. */
> +struct twist_flags {
> + bool disable_kbd_k_spec_handler;
> + bool disable_reboot_request;
> +};
> +extern struct twist_flags twist_flags;
Why all these options have to be in a single structure?
> index 82d91547d122..78fdbb4f17b1 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -1038,4 +1038,12 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
> /* OTHER_WRITABLE? Generally considered a bad idea. */ \
> BUILD_BUG_ON_ZERO((perms) & 2) + \
> (perms))
> +
> +/* Flags for twisting kernel behavior. */
> +struct twist_flags {
> + bool disable_kbd_k_spec_handler;
> + bool disable_reboot_request;
> +};
> +extern struct twist_flags twist_flags;
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 498d344ea53a..41cfabc74ad7 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2338,4 +2338,9 @@ config HYPERV_TESTING
>
> endmenu # "Kernel Testing and Coverage"
>
> +menuconfig DEFAULT_TWIST_FLAGS
> + string "Default twist options (DANGEROUS)"
> + help
> + Don't specify anything unless you know what you are doing.
> +
> endmenu # Kernel hacking
I think that the only way to get this upstream is to:
+ Add separate boot options that might theoretically be used also
by other people.
+ Use boot parameters and not build configuration.
+ Avoid the meaningless word "twist" !!!
Best Regards,
Petr
Tetsuo Handa
Jun 3, 2020, 9:35:41 AM6/3/20
to Petr Mladek, Dmitry Vyukov, syzkaller, Linus Torvalds, Andrew Morton, Linux Kernel Mailing List, Ondrej Mosnacek, Sergey Senozhatsky, Steven Rostedt, Greg Kroah-Hartman
Do we really want to expose so many options individually?
(If these options were build-time configuration, we won't need
this structure at all.)
>
>
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 498d344ea53a..41cfabc74ad7 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -2338,4 +2338,9 @@ config HYPERV_TESTING
>>
>> endmenu # "Kernel Testing and Coverage"
>>
>> +menuconfig DEFAULT_TWIST_FLAGS
>> + string "Default twist options (DANGEROUS)"
>> + help
>> + Don't specify anything unless you know what you are doing.
>> +
>> endmenu # Kernel hacking
>
> Why such a crazy build configure option?
>
> I think that the only way to get this upstream is to:
>
> + Add separate boot options that might theoretically be used also
> by other people.
I am afraid that many of them could not be normal options. They change or
break some behavior that is necessary by seriously used system.
these options are meant for helping fuzzers to find bugs while protecting
the kernel from legitimate-but-stupid requests from fuzzers. Other people
can include projects other than syzbot, but basically only useful for
debugging projects. (And making these options boot-time configuration
increases garbage/overhead for non-debugging usage.)
>
> + Use boot parameters and not build configuration.
files breaks reproducibility; people can fail to specify intended options.
Saving intended options into the .config file is the most robust/reliable
approach.
>
> + Avoid the meaningless word "twist" !!!
https://lkml.kernel.org/r/41a49d42-7119-62b9...@i-love.sakura.ne.jp
>
>
> Best Regards,
> Petr
>
Petr Mladek
Jun 4, 2020, 6:21:55 AM6/4/20
to Tetsuo Handa, Dmitry Vyukov, syzkaller, Linus Torvalds, Andrew Morton, Linux Kernel Mailing List, Ondrej Mosnacek, Sergey Senozhatsky, Steven Rostedt, Greg Kroah-Hartman
You should start with them.
Anyway, it still sounds to me like another lockdown mode. I think that
the relation with lockdown has already been discussed. But I do not
remember if it was refused.
> > + Use boot parameters and not build configuration.
>
> That sounds like a very tight restriction for syzbot. Relying on external
> files breaks reproducibility; people can fail to specify intended options.
> Saving intended options into the .config file is the most robust/reliable
> approach.
options is:
+ Create widely useful behavior switches when possible instead of
crazy hacks all over the kernel code.
+ The changes will modify the existing behavior. Build
configuration works only when the kernel will be used
for well defined purpose.
Boot option is better for (distribution) kernels that are used by
many different users. The person that builds the kernel does not
know what behavior would different users prefer.
You might argue that syzbot is a well defined user. But this goes
back to the first motivation to created general purpose options
when possible.
> > + Avoid the meaningless word "twist" !!!
>
> Then, what do you suggest? I think that we need some keyword for grouping.
> https://lkml.kernel.org/r/41a49d42-7119-62b9...@i-love.sakura.ne.jp
acceptable for the affected subsystem. You could always group them
later.
Anyway, the word "twist" is meaning less. It inspires people to create
hacks that have undefined effects.
IMHO, lockdown would be better but it is already used. Anyway, I
suggest start with independent options for the begining.
Best Regards,
Petr
PS: This is most likely my last reply in this thread. I feel that I am
just repeating all the already mentioned ideas just by other words.
Dmitry Vyukov
Jun 8, 2020, 3:48:17 AM6/8/20
to Tetsuo Handa, syzkaller, Linus Torvalds, Petr Mladek, Andrew Morton, Linux Kernel Mailing List, Ondrej Mosnacek, Sergey Senozhatsky, Steven Rostedt
On Fri, May 29, 2020 at 3:27 PM Tetsuo Handa
<penguin...@i-love.sakura.ne.jp> wrote:
>
> Hello, Dmitry.
>
> Linus is asking me to avoid build-time switching based on kernel config options,
> and is suggesting me to use boot-time switching based on boot-config file feature
> (which is available since 5.6). I have several concerns about use of boot-config file
> feature in syzkaller.
>
> (1) To use boot-config file, syzkaller will need to add "bootconfig" option
> to the kernel command line. This will be doable by patching
> https://github.com/google/syzkaller/tree/master/dashboard/config/ *.cmdline
> files.
Hello Tetsuo,
<penguin...@i-love.sakura.ne.jp> wrote:
>
> Hello, Dmitry.
>
> Linus is asking me to avoid build-time switching based on kernel config options,
> and is suggesting me to use boot-time switching based on boot-config file feature
> (which is available since 5.6). I have several concerns about use of boot-config file
> feature in syzkaller.
>
> (1) To use boot-config file, syzkaller will need to add "bootconfig" option
> to the kernel command line. This will be doable by patching
> https://github.com/google/syzkaller/tree/master/dashboard/config/ *.cmdline
> files.
Yes, command line arguments are easily changeable. Please send pull
requests to syzkaller, if you want to change something.
> (2) The boot-config file is embedded into initramfs file. Since syzkaller builds
> kernels with almost-allyesconfig, booting syzkaller kernels do not require
> initramfs for loading kernel modules needed for mounting the root partition.
> In fact, according to "unexpected kernel reboot" report which contains boot messages,
> I can't find "Unpacking initramfs..." message. It seems that syzkaller kernels do not
> use initramfs file.
>
> Is it possible for syzkaller to enforce performing steps for creating an initramfs,
> embedding the boot-config file
> ( https://www.kernel.org/doc/html/latest/admin-guide/bootconfig.html#boot-kernel-with-a-boot-config),
> and loading that initramfs whenever booting the syzkaller kernels?
> By the way, I do worry that people forget to perform these steps when they do
> their tests without asking syzbot...
syzkaller itself does not enforce/require any kernel configuration,
hardware nor use or not use of initramfs. In fact, qemu VM type
supports initramfs today. Or syzkaller can work with bare machines
where all setup is up to the user.
syzbot is just one deployment of syzkaller with a particular
configuration/hardware.
If this feature is useful for any linux kernel fuzzing, then we need
to have a plan for all users and setups.
And, yes, an additional context is kernel developers reproducing bugs.
Not all of them may be happy about any additional steps, nor will
follow them.
Answering your question, syzkaller can do some sanity checking of the
provided machine/kernel and reject working with it. If you tell me
what exactly needs to be checked, I can think where this code should
go.
However, again, I am not sure if one is using, say, Android phones and
they don't envision use of initramfs, then what?
For syzbot, the build happens here:
https://github.com/google/syzkaller/blob/7751efd04aebb07bc82b5c0e8eeaca07be1ae112/pkg/build/linux.go#L72
I don't know if initramfs is supported with GCE machines and what
exactly is required.
> (3) Since syzkaller keeps track of "kernel tree", "commit of the kernel tree", and
> "commit of the syzkaller tree" in order to guarantee reproducibility, it would be
> possible to identify the "your-config" file used for tools/bootconfig/bootconfig
> command. But since "#syz test" command currently accepts only "kernel tree" and
> "commit of the kernel tree" arguments, we might fail to use intended "your-config"
> file when doing reproducibility test. Can syzbot solve this concern?
> (4) Of course, "your-config" file would not change so frequently, but "#syz test" command
> relies on external file in "syzkaller tree" makes it impossible to try different
> configuration when I have to ask syzbot to test. (Since I don't have hardware which
> syzbot is reporting problems, I have to ask syzbot when I can't reproduce the problem
> in my environment.)
>
> https://syzkaller.appspot.com/text?tag=Patch&x=135f254a100000 is an example of
> need to enforce CONFIG_DEBUG_INFO_BTF=n in order to workaround build failure during
> "#syz test" command. If we bring "which twist options should be enabled" to an external
> boot-config file, I can't ask syzbot to try different twist options (except directly
> patching the kernel source which handles "which twist options should be enabled").
> Can syzbot solve this concern?
solved during boot, right? So what is the relation?
> (5) Anything else?
Reading:
https://www.kernel.org/doc/html/latest/admin-guide/bootconfig.html#boot-kernel-with-a-boot-config
It seems that boot config is just a more complex way to provide
command line arguments. syzbot already supports command line
arguments, and it looks much simpler and no additional work required.
Why do we want to use boot config?
Next quarter we will be additionally busy with interns, so I can't
promise any time availability for syzbot improvements. But pull
requests are welcome.
Tetsuo Handa
Jun 8, 2020, 6:31:08 AM6/8/20
to Dmitry Vyukov, syzkaller, Linus Torvalds, Petr Mladek, Andrew Morton, Linux Kernel Mailing List, Ondrej Mosnacek, Sergey Senozhatsky, Steven Rostedt
On 2020/06/08 16:48, Dmitry Vyukov wrote:
>> (5) Anything else?
>
> Reading:
> https://www.kernel.org/doc/html/latest/admin-guide/bootconfig.html#boot-kernel-with-a-boot-config
> It seems that boot config is just a more complex way to provide
> command line arguments. syzbot already supports command line
> arguments, and it looks much simpler and no additional work required.
> Why do we want to use boot config?
Since the max length a bootloader can accept for kernel command line arguments is finite (e.g.
>> (5) Anything else?
>
> Reading:
> https://www.kernel.org/doc/html/latest/admin-guide/bootconfig.html#boot-kernel-with-a-boot-config
> It seems that boot config is just a more complex way to provide
> command line arguments. syzbot already supports command line
> arguments, and it looks much simpler and no additional work required.
> Why do we want to use boot config?
https://bugzilla.redhat.com/show_bug.cgi?id=1239170 ), we can't specify as many arguments as
we want. Since Linus is expecting to specify independently upon boot, length for the kernel
command line arguments might exceed that limit. The boot config is a method for allowing longer
kernel command line arguments, at the cost of mandating use of the initramfs file.
>> (2) The boot-config file is embedded into initramfs file. Since syzkaller builds
>> kernels with almost-allyesconfig, booting syzkaller kernels do not require
>> initramfs for loading kernel modules needed for mounting the root partition.
>> In fact, according to "unexpected kernel reboot" report which contains boot messages,
>> I can't find "Unpacking initramfs..." message. It seems that syzkaller kernels do not
>> use initramfs file.
>>
>> Is it possible for syzkaller to enforce performing steps for creating an initramfs,
>> embedding the boot-config file
>> ( https://www.kernel.org/doc/html/latest/admin-guide/bootconfig.html#boot-kernel-with-a-boot-config),
>> and loading that initramfs whenever booting the syzkaller kernels?
>> By the way, I do worry that people forget to perform these steps when they do
>> their tests without asking syzbot...
>
> I think we have some confusion between syzkaller and syzbot here.
> syzkaller itself does not enforce/require any kernel configuration,
> hardware nor use or not use of initramfs. In fact, qemu VM type
> supports initramfs today. Or syzkaller can work with bare machines
> where all setup is up to the user.
> syzbot is just one deployment of syzkaller with a particular
> configuration/hardware.
>
> If this feature is useful for any linux kernel fuzzing, then we need
> to have a plan for all users and setups.
environment/purpose only. Assuming that this feature is useful for any linux kernel fuzzing,
we need to care about whether we can specify longer kernel command line arguments on every
environment in order to support boot-time switching.
>
> And, yes, an additional context is kernel developers reproducing bugs.
> Not all of them may be happy about any additional steps, nor will
> follow them.
Not mandate specifying appropriate twist options to the kernel command line
arguments can prevent automated/manual testings from finding/reproducing bugs.
>
> Answering your question, syzkaller can do some sanity checking of the
> provided machine/kernel and reject working with it. If you tell me
> what exactly needs to be checked, I can think where this code should
> go.
> However, again, I am not sure if one is using, say, Android phones and
> they don't envision use of initramfs, then what?
twist options to the kernel command line (due to length limitation).
>
> For syzbot, the build happens here:
> https://github.com/google/syzkaller/blob/7751efd04aebb07bc82b5c0e8eeaca07be1ae112/pkg/build/linux.go#L72
> I don't know if initramfs is supported with GCE machines and what
> exactly is required.
>
>> (4) Of course, "your-config" file would not change so frequently, but "#syz test" command
>> relies on external file in "syzkaller tree" makes it impossible to try different
>> configuration when I have to ask syzbot to test. (Since I don't have hardware which
>> syzbot is reporting problems, I have to ask syzbot when I can't reproduce the problem
>> in my environment.)
>>
>> https://syzkaller.appspot.com/text?tag=Patch&x=135f254a100000 is an example of
>> need to enforce CONFIG_DEBUG_INFO_BTF=n in order to workaround build failure during
>> "#syz test" command. If we bring "which twist options should be enabled" to an external
>> boot-config file, I can't ask syzbot to try different twist options (except directly
>> patching the kernel source which handles "which twist options should be enabled").
>> Can syzbot solve this concern?
>
> The CONFIG_DEBUG_INFO_BTF relates to build config. This can't be
> solved during boot, right? So what is the relation?
compensate for lack of ability to temporarily change the kernel command line arguments
(when it is difficult to temporarily change the kernel command line arguments).
Andrey Konovalov
Jun 8, 2020, 7:31:26 AM6/8/20
to Dmitry Vyukov, Tetsuo Handa, syzkaller, Linus Torvalds, Petr Mladek, Andrew Morton, Linux Kernel Mailing List, Ondrej Mosnacek, Sergey Senozhatsky, Steven Rostedt
>
> > (4) Of course, "your-config" file would not change so frequently, but "#syz test" command
> > relies on external file in "syzkaller tree" makes it impossible to try different
> > configuration when I have to ask syzbot to test. (Since I don't have hardware which
> > syzbot is reporting problems, I have to ask syzbot when I can't reproduce the problem
> > in my environment.)
> >
> > https://syzkaller.appspot.com/text?tag=Patch&x=135f254a100000 is an example of
> > need to enforce CONFIG_DEBUG_INFO_BTF=n in order to workaround build failure during
> > "#syz test" command. If we bring "which twist options should be enabled" to an external
> > boot-config file, I can't ask syzbot to try different twist options (except directly
> > patching the kernel source which handles "which twist options should be enabled").
> > Can syzbot solve this concern?
>
> The CONFIG_DEBUG_INFO_BTF relates to build config. This can't be
> solved during boot, right? So what is the relation?
>
> > (5) Anything else?
>
> Reading:
> https://www.kernel.org/doc/html/latest/admin-guide/bootconfig.html#boot-kernel-with-a-boot-config
> It seems that boot config is just a more complex way to provide
> command line arguments. syzbot already supports command line
> arguments, and it looks much simpler and no additional work required.
> Why do we want to use boot config?
>
> Next quarter we will be additionally busy with interns, so I can't
> promise any time availability for syzbot improvements. But pull
> requests are welcome.
>
> You received this message because you are subscribed to the Google Groups "syzkaller" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller/CACT4Y%2BZ58Z8u1g8SBy-i1WxLMrdmXvggsLFAhfbLc8D%3DuffPyA%40mail.gmail.com.
Reply all
Reply to author
Forward
0 new messages