Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Greg KH <gregkh@linuxfoundation.org>]

On Sat, Dec 14, 2019 at 07:28:02PM +0900, Tetsuo Handa wrote:
> On 2019/12/14 18:09, Greg KH wrote:
> > On Sat, Dec 14, 2019 at 05:39:02PM +0900, Tetsuo Handa wrote:
> >> On 2019/12/14 16:55, Greg KH wrote:
> >>>>>> That suggestion got no response for two months.
> >>>>>>
> >>>>>>   https://lkml.kernel.org/r/3e4e2b6b-7828-54ab-cf28-db1a396d7e20@i-love.sakura.ne.jp
> >>>>>>
> >>>>>> Unless we add such kernel config option to upstream kernels, it will become
> >>>>>> a whack-a-mole game.
> >>>>>
> >>>>> It will be a whack-a-mole game no matter what.
> >>>>>
> >>>>> Yes, /dev/mem/ makes no sense to fuzz.  Neither does other things (like
> >>>>> serial port memory addresses.)
> >>>>
> >>>> /dev/mem makes sense to fuzz. Ditto for other things.
> >>>
> >>> What?  What are you going to find if you randomly start to write to
> >>> /dev/mem?  How are we supposed to "fix" that?
> >>>
> >>
> >> When did I say "writing to random addresses" ? If you saw my suggestion, you
> >> will find that "fuzzer will be able to test reading from random addresses,
> >> writing to safe addresses (in order to find new lock dependency which would
> >> otherwise be unnoticed)".
> > 
> > I don't remember the suggestion specifically, sorry.  But how would you
> > figure out what those "safe addresses" really are?  They will change on
> > every single platform.
> 
> ----------
> +#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

Ick ick ick, we don't like #ifdefs in .c files for a reason :)

>                         unxlate_dev_mem_ptr(p, ptr);
> ----------
> 
> How dummybuf cannot be "safe address" ?

Sure, that's safe, but don't try to change what I originally said about
writing to random /dev/mem addresses please.

> > And why would this even help anything?  What lock dependency?
> > 
> 
> copy_from_user() can trigger page fault which involves memory allocation.
> And direct reclaim which is performed within memory allocation operation
> is full of subtle dependency bugs. :-(

It's as if you want a memory/platform type to be a "fuzz target", right?
You aren't going to go around and try to put the above crazyness into
each and every spot in the kernel that wants to write to a
user-specified address.

Fuzzing the kernel is great, but remember it's a means to an end, that
of trying to ensure that the kernel has less bugs than before.  Forcing
the kernel to adapt a memory model that is not what it "normally" uses
kind of goes against your main goal here, right?

And have you all really run out of apis that are all now bug free that
you are going after these types of ones?

thanks,

greg k-h