Messages in this thread Patch in this message |  | | From | Dmitry Vyukov <> | Date | Tue, 17 Dec 2019 09:41:53 +0100 | Subject | Re: [PATCH] kconfig: Add kernel config option for fuzz testing. |
| |
On Mon, Dec 16, 2019 at 11:01 AM Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > > While syzkaller is finding many bugs, sometimes syzkaller examines > stupid operations. But disabling operations using kernel config option > is problematic because "kernel config option excludes whole module when > there is still room for examining all but specific operation" and > "the list of kernel config options becomes too complicated to maintain > because such list changes over time". Thus, this patch introduces a > kernel config option which allows disabling only specific operations. > This kernel config option should be enabled only when building kernels > for fuzz testing. > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Cc: Dmitry Vyukov <dvyukov@google.com> > --- > drivers/char/mem.c | 6 +++--- > drivers/tty/serial/8250/8250_port.c | 7 +++++-- > drivers/tty/vt/keyboard.c | 3 +++ > fs/ioctl.c | 5 +++++ > include/linux/uaccess.h | 12 ++++++++++++ > kernel/printk/printk.c | 8 ++++++++ > lib/Kconfig.debug | 10 ++++++++++ > lib/usercopy.c | 27 +++++++++++++++++++++++++++ > 8 files changed, 73 insertions(+), 5 deletions(-) > > diff --git a/drivers/char/mem.c b/drivers/char/mem.c > index 43dd0891ca1e..63aff3ae7c2b 100644 > --- a/drivers/char/mem.c > +++ b/drivers/char/mem.c > @@ -246,7 +246,7 @@ static ssize_t write_mem(struct file *file, const char __user *buf, > return -EFAULT; > } > > - copied = copy_from_user(ptr, buf, sz); > + copied = copy_from_user_to_any(ptr, buf, sz);
FWIW we disable /dev/{mem,kmem} in the config now: https://github.com/google/syzkaller/blob/master/dashboard/config/bits-syzbot.config#L169 And this looks like a reasonable thing to do for any fuzzing.
> unxlate_dev_mem_ptr(p, ptr); > if (copied) { > written += sz - copied; > @@ -550,7 +550,7 @@ static ssize_t do_write_kmem(unsigned long p, const char __user *buf, > if (!virt_addr_valid(ptr)) > return -ENXIO; > > - copied = copy_from_user(ptr, buf, sz); > + copied = copy_from_user_to_any(ptr, buf, sz); > if (copied) { > written += sz - copied; > if (written) > @@ -604,7 +604,7 @@ static ssize_t write_kmem(struct file *file, const char __user *buf, > err = -ENXIO; > break; > } > - n = copy_from_user(kbuf, buf, sz); > + n = copy_from_user_to_any(kbuf, buf, sz); > if (n) { > err = -EFAULT; > break; > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c > index 90655910b0c7..367b92ad598b 100644 > --- a/drivers/tty/serial/8250/8250_port.c > +++ b/drivers/tty/serial/8250/8250_port.c > @@ -519,11 +519,14 @@ serial_port_out_sync(struct uart_port *p, int offset, int value) > case UPIO_MEM32: > case UPIO_MEM32BE: > case UPIO_AU: > - p->serial_out(p, offset, value); > + /* Writing to random kernel address causes crash. */ > + if (!IS_ENABLED(CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING)) > + p->serial_out(p, offset, value);
Does this do the same as LOCKDOWN_TIOCSSERIAL? How is it different?
> p->serial_in(p, UART_LCR); /* safe, no side-effects */ > break; > default: > - p->serial_out(p, offset, value); > + if (!IS_ENABLED(CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING)) > + p->serial_out(p, offset, value); > } > } > > diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c > index 15d33fa0c925..367820b8ff59 100644 > --- a/drivers/tty/vt/keyboard.c > +++ b/drivers/tty/vt/keyboard.c > @@ -633,6 +633,9 @@ 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 */ > + /* Repeating SysRq-t forever causes RCU stalls. */ > + if (IS_ENABLED(CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING)) > + return; > fn_handler[value](vc); > } > > diff --git a/fs/ioctl.c b/fs/ioctl.c > index 2f5e4e5b97e1..f879aa94b118 100644 > --- a/fs/ioctl.c > +++ b/fs/ioctl.c > @@ -601,6 +601,11 @@ static int ioctl_fsfreeze(struct file *filp) > if (sb->s_op->freeze_fs == NULL && sb->s_op->freeze_super == NULL) > return -EOPNOTSUPP; > > +#ifdef CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING > + /* Freezing filesystems causes hung tasks. */ > + return -EBUSY; > +#endif > + > /* Freeze */ > if (sb->s_op->freeze_super) > return sb->s_op->freeze_super(sb); > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h > index 67f016010aad..6e5ddd0fdcce 100644 > --- a/include/linux/uaccess.h > +++ b/include/linux/uaccess.h > @@ -387,4 +387,16 @@ void __noreturn usercopy_abort(const char *name, const char *detail, > unsigned long len); > #endif > > +#ifdef CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING > +unsigned long __must_check copy_from_user_to_any(void *to, > + const void __user *from, > + unsigned long n); > +#else > +static __always_inline unsigned long __must_check > +copy_from_user_to_any(void *to, const void __user *from, unsigned long n) > +{ > + return copy_from_user(to, from, n); > +} > +#endif > + > #endif /* __LINUX_UACCESS_H__ */ > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index 1ef6f75d92f1..9a2f95a78fef 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -1198,6 +1198,14 @@ MODULE_PARM_DESC(ignore_loglevel, > > static bool suppress_message_printing(int level) > { > +#ifdef CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING > + /* > + * Changing console_loglevel causes "no output". But ignoring > + * console_loglevel is easier than preventing change of > + * console_loglevel. > + */ > + return (level >= CONSOLE_LOGLEVEL_DEFAULT && !ignore_loglevel); > +#endif > return (level >= console_loglevel && !ignore_loglevel); > } > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index d1842fe756d5..f9836cc23942 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -2184,4 +2184,14 @@ config HYPERV_TESTING > help > Select this option to enable Hyper-V vmbus testing. > > +config KERNEL_BUILT_FOR_FUZZ_TESTING > + bool "Build kernel for fuzz testing" > + default n > + help > + Say N unless you are building kernels for fuzz testing. > + Saying Y here disables several things that legitimately cause > + damage under a fuzzer workload (e.g. copying to arbitrary > + user-specified kernel address, changing console loglevel, > + freezing filesystems). > + > endmenu # Kernel hacking > diff --git a/lib/usercopy.c b/lib/usercopy.c > index cbb4d9ec00f2..f7d5d243a63d 100644 > --- a/lib/usercopy.c > +++ b/lib/usercopy.c > @@ -86,3 +86,30 @@ int check_zeroed_user(const void __user *from, size_t size) > return -EFAULT; > } > EXPORT_SYMBOL(check_zeroed_user); > + > +#ifdef CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING > +/* > + * Since copying to arbitrary user-specified kernel address will crash > + * the kernel trivially, do not copy to user-specified kernel address > + * if the kernel was build for fuzz testing. > + */ > +unsigned long __must_check copy_from_user_to_any(void *to, > + const void __user *from, > + unsigned long n) > +{ > + static char dummybuf[PAGE_SIZE]; > + unsigned long ret = 0; > + > + while (n) { > + unsigned long size = min(n, sizeof(dummybuf)); > + > + ret = copy_from_user(dummybuf, from, size); > + if (ret) > + break; > + from += size; > + n -= size; > + } > + return ret; > +} > +EXPORT_SYMBOL(copy_from_user_to_any); > +#endif > -- > 2.18.1 >
|  |