[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20190222173509.88489b7c5d1bf0e2ec2382ee@kernel.org>
Date: Fri, 22 Feb 2019 17:35:09 +0900
From: Masami Hiramatsu <mhiramat@...nel.org>
To: Masami Hiramatsu <mhiramat@...nel.org>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Andy Lutomirski <luto@...capital.net>,
Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
stable <stable@...r.kernel.org>,
Changbin Du <changbin.du@...il.com>,
Jann Horn <jannh@...gle.com>,
Kees Cook <keescook@...omium.org>,
Andy Lutomirski <luto@...nel.org>
Subject: Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access
kernel memory that can fault
On Fri, 22 Feb 2019 17:27:45 +0900
Masami Hiramatsu <mhiramat@...nel.org> wrote:
> Hi Steve,
>
> On Wed, 20 Feb 2019 09:49:26 -0500
> Steven Rostedt <rostedt@...dmis.org> wrote:
>
> > On Wed, 20 Feb 2019 17:10:19 +0900
> > Masami Hiramatsu <mhiramat@...nel.org> wrote:
> >
> > > Let me ensure what you want. So you want to access a "string" in user-space,
> > > not a data structure? In that case, it is very easy to me. It is enough to
> > > add a "ustring" type to kprobe events. For example, do_sys_opsn's path
> > > variable is one example. That will be +0(+0(%si)):ustring, and fetcher
> > > finally copy the string using strncpy_from_user() instead of
> > > strncpy_from_unsafe(). (*)
> >
> > ustring would be good.
>
> I've tried to implement ustring and u-offsets, but I got some issues.
>
> - access_ok() warns if it is called in IRQ context (kprobes is.)
> - copy_from_user uses access_ok(), so it is not designed for irq handler.
>
> Moreover, if we have different kernel/user address spaces, we have to
> assign target user-pages to kernel vma. Can we do that (doesn't it involve
> mutex locks)?
Or, can we do this?
long __probe_user_read(void *dst, const void *src, size_t size)
{
long ret;
mm_segment_t old_fs = get_fs();
set_fs(USER_DS); /* Only this is changed */
pagefault_disable();
current->kernel_uaccess_faults_ok++;
ret = __copy_from_user_inatomic(dst,
(__force const void __user *)src, size);
current->kernel_uaccess_faults_ok--;
pagefault_enable();
set_fs(old_fs);
return ret ? -EFAULT : 0;
}
--
Masami Hiramatsu <mhiramat@...nel.org>
Powered by blists - more mailing lists