All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jann Horn <jannh@google.com>
To: Masami Hiramatsu <mhiramat@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	kernel test robot <lkp@intel.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Shuah Khan <shuah@kernel.org>,
	Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Changbin Du <changbin.du@gmail.com>,
	Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@kernel.org>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Nadav Amit <namit@vmware.com>,
	Joel Fernandes <joel@joelfernandes.org>,
	yhs@fb.com, lkp@01.org
Subject: Re: [uaccess] 780464aed0: WARNING:at_arch/x86/include/asm/uaccess.h:#strnlen_user/0x
Date: Mon, 4 Mar 2019 16:58:05 +0100	[thread overview]
Message-ID: <CAG48ez2a_dcLFpuEL3M-_KbTEmLq6dn+bXtGmL2MhQrYuj6tLA@mail.gmail.com> (raw)
In-Reply-To: <20190305001609.10f950fd4f8089e1ae07c9f1@kernel.org>

On Mon, Mar 4, 2019 at 4:16 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Hello,
>
> On Mon, 4 Mar 2019 18:06:10 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> > On Sun, 3 Mar 2019 18:37:59 -0800
> > Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >
> > > On Sun, Mar 3, 2019 at 5:14 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > >
> > > > I think it comes from WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS)) in
> > > > user_access_ok(). The call trace shows that strndup_user might be called
> > > > from kernel daemon context.
> > >
> > > Ahh, yes.
> > >
> > > We've had this before. We've gotten rid of the actual "use system
> > > calls", but we still have some of the init sequence in particular just
> > > calling the wrappers instead.
> >
> > Are those safe if we are in init sequence?
> >
> > >
> > > And yes, ksys_mount() takes __user pointers.
> > >
> > > It would be a lot better to use "do_mount()", which is the interface
> > > that takes actual "char *" pointers.
> >
> > Unfortunately, it still takes a __user pointer.
> >
> > long do_mount(const char *dev_name, const char __user *dir_name,
> >                 const char *type_page, unsigned long flags, void *data_page)
> >
> > So what we need is
> >
> > long do_mount(const char *dev_name, struct path *dir_path,
> >                 const char *type_page, unsigned long flags, void *data_page)
> >
> > or introduce kern_do_mount()?
>
> Would this work?
>
> ( BTW, what is this code for? It seems totally insane termination.
>   at least this should be done in copy_mount_options().
>         if (data_page)
>                 ((char *)data_page)[PAGE_SIZE - 1] = 0;
> )
>
> =======
> devtmpfs: Avoid passing kernel memory to __user parameter
>
> From: Masami Hiramatsu <mhiramat@kernel.org>
>
> Since ksys_mount(), ksys_chdir() and ksys_chroot() takes
> __user parameters, devtmpfsd must not pass the kernel memory
> to them. On some arch, it is not possible to pass th kernel
> memory to them, since the memory address spaces can be
> different.

strncpy_from_user() also uses user_access_begin(), and that thing's
used from all the VFS syscalls via getname(). And those are used all
over the place in init/ - for example:
init/do_mounts.c uses ksys_open() and ksys_chroot()
init/do_mounts_initrd.c uses ksys_open(), ksys_chdir(), ksys_mount(),
ksys_mkdir() and ksys_unlink()
init/do_mounts_md.c uses ksys_open()
and so on.
(identify_ramdisk_image() even uses ksys_lseek() and ksys_read()
instead of just using kernel_read()...)

WARNING: multiple messages have this Message-ID (diff)
From: Jann Horn <jannh@google.com>
To: lkp@lists.01.org
Subject: Re: [uaccess] 780464aed0: WARNING:at_arch/x86/include/asm/uaccess.h:#strnlen_user/0x
Date: Mon, 04 Mar 2019 16:58:05 +0100	[thread overview]
Message-ID: <CAG48ez2a_dcLFpuEL3M-_KbTEmLq6dn+bXtGmL2MhQrYuj6tLA@mail.gmail.com> (raw)
In-Reply-To: <20190305001609.10f950fd4f8089e1ae07c9f1@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 2602 bytes --]

On Mon, Mar 4, 2019 at 4:16 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Hello,
>
> On Mon, 4 Mar 2019 18:06:10 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> > On Sun, 3 Mar 2019 18:37:59 -0800
> > Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >
> > > On Sun, Mar 3, 2019 at 5:14 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > >
> > > > I think it comes from WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS)) in
> > > > user_access_ok(). The call trace shows that strndup_user might be called
> > > > from kernel daemon context.
> > >
> > > Ahh, yes.
> > >
> > > We've had this before. We've gotten rid of the actual "use system
> > > calls", but we still have some of the init sequence in particular just
> > > calling the wrappers instead.
> >
> > Are those safe if we are in init sequence?
> >
> > >
> > > And yes, ksys_mount() takes __user pointers.
> > >
> > > It would be a lot better to use "do_mount()", which is the interface
> > > that takes actual "char *" pointers.
> >
> > Unfortunately, it still takes a __user pointer.
> >
> > long do_mount(const char *dev_name, const char __user *dir_name,
> >                 const char *type_page, unsigned long flags, void *data_page)
> >
> > So what we need is
> >
> > long do_mount(const char *dev_name, struct path *dir_path,
> >                 const char *type_page, unsigned long flags, void *data_page)
> >
> > or introduce kern_do_mount()?
>
> Would this work?
>
> ( BTW, what is this code for? It seems totally insane termination.
>   at least this should be done in copy_mount_options().
>         if (data_page)
>                 ((char *)data_page)[PAGE_SIZE - 1] = 0;
> )
>
> =======
> devtmpfs: Avoid passing kernel memory to __user parameter
>
> From: Masami Hiramatsu <mhiramat@kernel.org>
>
> Since ksys_mount(), ksys_chdir() and ksys_chroot() takes
> __user parameters, devtmpfsd must not pass the kernel memory
> to them. On some arch, it is not possible to pass th kernel
> memory to them, since the memory address spaces can be
> different.

strncpy_from_user() also uses user_access_begin(), and that thing's
used from all the VFS syscalls via getname(). And those are used all
over the place in init/ - for example:
init/do_mounts.c uses ksys_open() and ksys_chroot()
init/do_mounts_initrd.c uses ksys_open(), ksys_chdir(), ksys_mount(),
ksys_mkdir() and ksys_unlink()
init/do_mounts_md.c uses ksys_open()
and so on.
(identify_ramdisk_image() even uses ksys_lseek() and ksys_read()
instead of just using kernel_read()...)

  reply	other threads:[~2019-03-04 15:58 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-28 16:02 [PATCH v5 0/6] tracing/probes: uaccess: Add support user-space access Masami Hiramatsu
2019-02-28 16:02 ` [PATCH v5 1/6] uaccess: Add user_access_ok() Masami Hiramatsu
2019-02-28 16:03 ` [PATCH v5 2/6] uaccess: Use user_access_ok() in user_access_begin() Masami Hiramatsu
2019-03-03 17:39   ` [uaccess] 780464aed0: WARNING:at_arch/x86/include/asm/uaccess.h:#strnlen_user/0x kernel test robot
2019-03-03 17:39     ` kernel test robot
2019-03-03 19:53     ` Linus Torvalds
2019-03-03 19:53       ` Linus Torvalds
2019-03-04  1:14       ` Masami Hiramatsu
2019-03-04  1:14         ` Masami Hiramatsu
2019-03-04  2:37         ` Linus Torvalds
2019-03-04  2:37           ` Linus Torvalds
2019-03-04  9:06           ` Masami Hiramatsu
2019-03-04  9:06             ` Masami Hiramatsu
2019-03-04 15:16             ` Masami Hiramatsu
2019-03-04 15:16               ` Masami Hiramatsu
2019-03-04 15:58               ` Jann Horn [this message]
2019-03-04 15:58                 ` Jann Horn
2019-03-04 18:59             ` Linus Torvalds
2019-03-04 18:59               ` Linus Torvalds
2019-03-05  2:36               ` Masami Hiramatsu
2019-03-05  2:36                 ` Masami Hiramatsu
2019-03-05  8:22                 ` Masami Hiramatsu
2019-03-05  8:22                   ` Masami Hiramatsu
2019-03-05  9:01                   ` Masami Hiramatsu
2019-03-05  9:01                     ` Masami Hiramatsu
2019-03-05  9:07                 ` Peter Zijlstra
2019-03-05  9:07                   ` Peter Zijlstra
2019-03-05 13:58                   ` Masami Hiramatsu
2019-03-05 13:58                     ` Masami Hiramatsu
2019-03-05 14:53                     ` Peter Zijlstra
2019-03-05 14:53                       ` Peter Zijlstra
2019-03-05 15:18                       ` Masami Hiramatsu
2019-03-05 15:18                         ` Masami Hiramatsu
2019-03-04  3:20       ` [LKP] " Rong Chen
2019-03-04  3:20         ` Rong Chen
2019-02-28 16:03 ` [PATCH v5 3/6] uaccess: Add non-pagefault user-space read functions Masami Hiramatsu
2019-02-28 22:49   ` Yonghong Song
2019-03-01  2:29     ` Masami Hiramatsu
2019-03-01  6:30       ` Yonghong Song
2019-02-28 16:04 ` [PATCH v5 4/6] tracing/probe: Add ustring type for user-space string Masami Hiramatsu
2019-02-28 16:04 ` [PATCH v5 5/6] tracing/probe: Support user-space dereference Masami Hiramatsu
2019-02-28 16:05 ` [PATCH v5 6/6] selftests/ftrace: Add user-memory access syntax testcase Masami Hiramatsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAG48ez2a_dcLFpuEL3M-_KbTEmLq6dn+bXtGmL2MhQrYuj6tLA@mail.gmail.com \
    --to=jannh@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=alexei.starovoitov@gmail.com \
    --cc=changbin.du@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@01.org \
    --cc=lkp@intel.com \
    --cc=luto@amacapital.net \
    --cc=luto@kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=namit@vmware.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=shuah@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.