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

[Dmitry Vyukov]

On Thu, Aug 22, 2019 at 2:17 PM Tetsuo Handa
<penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On 2019/08/23 2:11, Dmitry Vyukov wrote:
> > On Thu, Aug 22, 2019 at 9:42 AM Greg Kroah-Hartman
> > <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >>>>> By the way, write_mem() worries me whether there is possibility of replacing
> >>>>> kernel code/data with user-defined memory data supplied from userspace.
> >>>>> If write_mem() were by chance replaced with code that does
> >>>>>
> >>>>>    while (1);
> >>>>>
> >>>>> we won't be able to return from write_mem() even if we added fatal_signal_pending() check.
> >>>>> Ditto for replacing local variables with unexpected values...
> >>>>
> >>>> I'm sorry, I don't really understand what you mean here, but I haven't
> >>>> had my morning coffee...  Any hints as to an example?
> >>>
> >>> Probably similar idea: "lockdown: Restrict /dev/{mem,kmem,port} when the kernel is locked down"
> >>>
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/char/mem.c?h=next-20190822&id=9b9d8dda1ed72e9bd560ab0ca93d322a9440510e
> >>>
> >>> Then, syzbot might want to blacklist writing to /dev/mem .
> >>
> >> syzbot should probably blacklist that now, you can do a lot of bad
> >> things writing to that device node :(
> >
> > Agree. It wasn't supposed to reach it, but it figured out how to mount
> > devfs and then open "./mem"  bypassing all checks. Fortunately there
> > is a config to disable /dev/mem, so we are going to turn it off.
> >
>
> 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? That looks
like the right thing to do because we don't want fuzzer to do anything
with these files anyway. 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).


> ----------
>                         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 = 0;
> +#endif
>                         unxlate_dev_mem_ptr(p, ptr);
> ----------