All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	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>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
Date: Mon, 25 Feb 2019 11:40:25 +0900	[thread overview]
Message-ID: <20190225114025.902c9031075e2f1fc55369a3@kernel.org> (raw)
In-Reply-To: <CAHk-=wjBD+MQfWP2Q4Zp8FP4HR4MdPACw1=iMbDZANdiBwXDAQ@mail.gmail.com>

On Sun, 24 Feb 2019 09:26:45 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Sun, Feb 24, 2019 at 7:18 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > On Sat, 23 Feb 2019 20:38:03 -0800
> > Andy Lutomirski <luto@kernel.org> wrote:
> > >
> > > Can we just get rid of this might_sleep()?  access_ok() doesn't sleep
> > > as far as I know.
> >
> > Hmm, which might_sleep() would you pointed? What I talked was a
> > WARN_ON_ONCE(!in_task()) in access_ok() on x86 (only!), and in_task() just
> > checks preempt_count.
> 
> So the in_task() check does kind of make sense. Using "access_ok()"
> outside of task context is certainly an odd thing, for several
> reasons. The main one being simply that outside of task context, the
> whole "which task" question is open, and you don't know if the task is
> the active one, and so it's not clear if whatever task you interrupt
> might have done "set_fs()" or not.

Ah I got it. Usual case access_ok() in IRQ handler is strange.

> 
> So PeterZ isn't wrong:
> 
> > I guess PeterZ assumed that access_ok() is used only with user space access
> > APIs (e.g. copy_from_user) which can cause page-fault and locks mm (and might
> > sleep :)), but now we are trying to use access_ok() with new functions which
> > disables page-fault and just return -EFAULT.
> 
> .. but in this case, if we do it all *within* code that saves and
> restores the user access flag with get_fs/set_fs, access_ok() would be
> ok and it doesn't have the above issue.
> 
> So access_ok() in _general_ is absolutely not safe to do from
> interrupts, but within the context of probing user memory from a
> tracing event it just happens to be ok.

Hmm, but user can specify user-memory access from the tracing event
which is located in interrupt handler. So I understand that it is safe
only if we correctly setup access flag with get_fs/set_fs, is that
correct?

> It would be lovely to have a special macro for this, and keep the
> warning for the general case, but because this is a "every
> architecture needs to build their own" it's probably too painful.

Agreed.

> 
> PeterZ, do you remember the particular use case that triggered that
> commit 7c4788950ba5 ("x86/uaccess, sched/preempt: Verify access_ok()
> context")?
> 
>                     Linus


Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

  reply	other threads:[~2019-02-25  2:40 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-15 17:47 [PATCH 0/2 v2] [GIT PULL (take two)] tracing: Two more fixes Steven Rostedt
2019-02-15 17:47 ` [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault Steven Rostedt
2019-02-15 17:55   ` Linus Torvalds
2019-02-15 22:15     ` Steven Rostedt
2019-02-15 23:49       ` Andy Lutomirski
2019-02-16  0:19         ` Steven Rostedt
2019-02-16  1:32           ` Andy Lutomirski
2019-02-16  2:08             ` Steven Rostedt
2019-02-16  2:14               ` Andy Lutomirski
2019-02-16  2:21                 ` Steven Rostedt
2019-02-18 17:58           ` Linus Torvalds
2019-02-18 18:23             ` Linus Torvalds
2019-02-19 16:18               ` Steven Rostedt
2019-02-19 18:43                 ` Linus Torvalds
2019-02-19 19:03                   ` Steven Rostedt
2019-02-20  8:10                     ` Masami Hiramatsu
2019-02-20 13:57                       ` Jann Horn
2019-02-20 14:47                         ` Steven Rostedt
2019-02-20 15:08                         ` Masami Hiramatsu
2019-02-20 14:49                       ` Steven Rostedt
2019-02-20 16:04                         ` Masami Hiramatsu
2019-02-20 16:42                           ` Steven Rostedt
2019-02-21  7:37                             ` Masami Hiramatsu
2019-02-22  8:27                         ` Masami Hiramatsu
2019-02-22  8:35                           ` Masami Hiramatsu
2019-02-22 17:43                             ` Linus Torvalds
2019-02-22 17:48                               ` Andy Lutomirski
2019-02-22 18:28                                 ` Linus Torvalds
2019-02-22 19:52                                   ` Andy Lutomirski
2019-02-22 19:27                               ` Alexei Starovoitov
2019-02-22 19:30                                 ` Steven Rostedt
2019-02-22 19:34                                   ` Alexei Starovoitov
2019-02-22 19:39                                     ` Steven Rostedt
2019-02-22 19:55                                     ` Andy Lutomirski
2019-02-22 21:43                                       ` Jann Horn
2019-02-22 22:08                                         ` Nadav Amit
2019-02-22 22:17                                           ` Jann Horn
2019-02-22 22:21                                             ` Nadav Amit
2019-02-22 22:39                                               ` Nadav Amit
2019-02-22 23:02                                                 ` Jann Horn
2019-02-22 23:22                                                   ` Nadav Amit
2019-02-22 23:59                                                   ` Andy Lutomirski
2019-02-23  0:03                                                     ` Alexei Starovoitov
2019-02-23  0:15                                                     ` Nadav Amit
2019-02-24 19:35                                                       ` Andy Lutomirski
2019-02-25 13:36                                                     ` Masami Hiramatsu
2019-02-22 21:20                                 ` Linus Torvalds
2019-02-22 21:38                                   ` David Miller
2019-02-22 21:59                                     ` Linus Torvalds
2019-02-22 22:51                                       ` Alexei Starovoitov
2019-02-22 23:11                                         ` Jann Horn
2019-02-22 23:16                                           ` David Miller
2019-02-22 23:16                                         ` Linus Torvalds
2019-02-22 23:56                                           ` Alexei Starovoitov
2019-02-23  0:08                                             ` Linus Torvalds
2019-02-23  2:28                                               ` Alexei Starovoitov
2019-02-23  2:32                                                 ` Linus Torvalds
2019-02-23  3:02                                                 ` Steven Rostedt
2019-02-23  4:51                                             ` Masami Hiramatsu
2019-02-26  3:57                                       ` Christoph Hellwig
2019-02-26 15:24                                 ` Joel Fernandes
2019-02-28 12:29                                   ` Masami Hiramatsu
2019-02-28 15:18                                     ` Joel Fernandes
2019-02-23  3:47                               ` Masami Hiramatsu
2019-02-24  0:44                                 ` Steven Rostedt
2019-02-24  4:38                                   ` Andy Lutomirski
2019-02-24 15:17                                     ` Masami Hiramatsu
2019-02-24 17:26                                       ` Linus Torvalds
2019-02-25  2:40                                         ` Masami Hiramatsu [this message]
2019-02-25  4:49                                           ` Andy Lutomirski
2019-02-25  8:09                                             ` Masami Hiramatsu
2019-02-25 16:40                                               ` Steven Rostedt
2019-02-26  1:35                                                 ` Masami Hiramatsu
2019-02-25  8:33                                         ` Peter Zijlstra
2019-02-25 14:52                                           ` Peter Zijlstra
2019-02-25 16:48                                     ` Kees Cook
2019-02-25 16:58                                       ` Andy Lutomirski
2019-02-25 17:07                                         ` Kees Cook
2019-02-21  7:52   ` Masami Hiramatsu
2019-02-21 14:36     ` Steven Rostedt
2019-02-21 15:58       ` Masami Hiramatsu
2019-02-21 16:16         ` Masami Hiramatsu
2019-02-21 16:32           ` Steven Rostedt
2019-02-23 14:48     ` Masami Hiramatsu
2019-02-15 17:47 ` [PATCH 2/2 v2] tracing: Fix number of entries in trace header Steven Rostedt

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=20190225114025.902c9031075e2f1fc55369a3@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=changbin.du@gmail.com \
    --cc=jannh@google.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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.