Re: [PATCH] /dev/mem: Bail out upon SIGKILL when reading memory.

[Dmitry Vyukov]

On Fri, Aug 23, 2019 at 1:17 AM Tetsuo Handa
<penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On 2019/08/23 8:59, Dmitry Vyukov wrote:
> >> Can't we introduce a kernel config which selectively blocks specific actions?
> >> If we don't need to worry about bypassing blacklist checks, we will be able to
> >> enable syz_execute_func() again.
> >
> >
> > We can consider this, but we need some set of good use cases first.
> > For /dev/{mem,kmem} we disable them with config, right?
>
> /dev/{mem,kmem} can be disabled by kernel config options. But
>
> >                                                         That looks
> > like the right thing to do because we don't want fuzzer to do anything
> > with these files anyway.
>
> I don't think so. To examine as corner as possible (e.g. lock dependency),
> I consider that even doing
>
> ----------
> +#ifdef CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING
> +static char dummybuf[PAGE_SIZE];
> +#endif
> ----------
>
> ----------
>                         ptr = xlate_dev_mem_ptr(p);
>                         if (!ptr) {
>                                 if (written)
>                                         break;
>                                 return -EFAULT;
>                         }
> +#ifndef CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING
>                         copied = copy_from_user(ptr, buf, sz);
> +#else
> +                       copied = copy_from_user(dummybuf, buf, min(sizeof(dummybuf), sz));
> +#endif
>                         unxlate_dev_mem_ptr(p, ptr);
> ----------
>
> makes sense, for copy_from_user() might find new lock dependency
> which would otherwise be unnoticed.
>
> >                          So this won't be a good use case for
> > CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING.
> > Fuzzer can also reliably filter out based on syscall numbers of
> > top-level argument values. The potential problem is with (1)
> > pointers/indirect memory and (2) where blacklisting some top-level
> > argument values would backlist too much (e.g. prohibiting 3rd ioctl
> > argument 0 entirely).
>
> I consider that functions that freezes processes/filesystems,
> reboots/shutdowns a system, changes console loglevels can be blocked
> as well. Trying to examine up to last-second conditional branches will
> catch more bugs (e.g. bugs in error recovery paths).

Well, ok, sounds reasonable. If you can take on upstreaming such
config, we will definitely enable it on syzbot.