Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault

From: Steven Rostedt <rostedt@goodmis.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>,
	Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	stable <stable@vger.kernel.org>,
	Changbin Du <changbin.du@gmail.com>, Jann Horn <jannh@google.com>,
	Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@kernel.org>,
	Masami Hiramatsu <mhiramat@kernel.org>
Subject: Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
Date: Tue, 19 Feb 2019 14:03:30 -0500	[thread overview]
Message-ID: <20190219140330.5dd9e876@gandalf.local.home> (raw)
In-Reply-To: <CAHk-=wgTuK3kAduP-gr10vykT1uG=B2VpdffvmyBuTQ1UxPpMg@mail.gmail.com>

On Tue, 19 Feb 2019 10:43:36 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, Feb 19, 2019 at 8:18 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> >  
> > > So it would be good to not just say "user or kernel", but actually say
> > > what *kind* of kernel access it expects.  
> >
> > Note, kprobes are a different kind of beast. I've used kprobes to probe
> > userspace information as well as kernel. Heck, I could see someone
> > even using kprobes to probe IO memory to check if a device is doing
> > what they expect it's doing.  
> 
> Note that even if that is the case, you _need_ to special "user vs
> kernel" information.
> 
> Because the exact same address might exist in both.
> 
> Right now I think that only happens on sparc32, but vendors used to
> have that issue on x86-32 too (if they had the 4G:4G patches).

Hmm, I didn't realize that Linux allowed the same address to be
different depending on being in kernel or user space (learn something
new everyday). I guess it makes sense, and even easier with the switch
of cr3 from user to kernel. And I even knew of 4G:4G, but never used
it, nor put too much thought about its implementation.

> 
> > Basically, a kprobe is mostly used for debugging what's happening in a
> > live kernel, to read any address.  
> 
> My point is that "any address" is not sufficient to begin with. You
> need "kernel or user".
> 
> Having a flag for what _kind_ of kernel address is ok might then be
> required for other cases if they might not be ok with following page
> tables to IO space..
> 

Good point. Looks like we should add a new flag for kprobe
trace parameters, that tell kprobes if the address is expected to be
user or kernel. That would be good regardless of the duplicate
meanings, as we could use copy_from_user without touching KERNEL_DS, if
the probe argument specifically states "this is user space". For
example, when probing do_sys_open, and you want to read what path string
was passed into the kernel.

Masami, thoughts?

-- Steve