From: Masami Hiramatsu <mhiramat@kernel.org> To: Masami Hiramatsu <mhiramat@kernel.org> Cc: Linus Torvalds <torvalds@linux-foundation.org>, Peter Zijlstra <peterz@infradead.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>, Jann Horn <jannh@google.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: Tue, 5 Mar 2019 18:01:26 +0900 [thread overview] Message-ID: <20190305180126.0f7201d8ec733fdc887135f1@kernel.org> (raw) In-Reply-To: <20190305172241.2eef39c6ca74d09120bbbcad@kernel.org> On Tue, 5 Mar 2019 17:22:41 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > On Tue, 5 Mar 2019 11:36:35 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > On Mon, 4 Mar 2019 10:59:22 -0800 > > Linus Torvalds <torvalds@linux-foundation.org> wrote: > > > > I think the better way to do this is allowing strncpy_from_user() > > if some conditions are match, like > > > > - strncpy_from_user() will be able to copy user memory with set_fs(USER_DS) > > - strncpy_from_user() can copy kernel memory with set_fs(KERNEL_DS) > > - strncpy_from_user() can access unsafe memory in IRQ context if > > pagefault is disabled. > > > > This is almost done, except for CONFIG_DEBUG_ATOMIC_SLEEP=y on x86. > > > > So, what about adding a condition to WARN_ON_IN_IRQ() like below > > instead of introducing user_access_ok() ? OK, here is the patch. Does it work for us? ==== x86/uaccess: Verify access_ok() context strictly From: Masami Hiramatsu <mhiramat@kernel.org> WARN_ON_IN_IRQ() assumes that the access_ok() and following user memory access can sleep. But this assumption is not always correct; when the pagefault is disabled, following memory access will just returns -EFAULT and never sleep. Add pagefault_disabled() check in WARN_ON_ONCE() so that it can ignore the case we call it with disabling pagefault. For this purpose, this modified pagefault_disabled() as an inline function. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- arch/x86/include/asm/uaccess.h | 4 +++- include/linux/uaccess.h | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h index 780f2b4..b98a552 100644 --- a/arch/x86/include/asm/uaccess.h +++ b/arch/x86/include/asm/uaccess.h @@ -70,7 +70,9 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un }) #ifdef CONFIG_DEBUG_ATOMIC_SLEEP -# define WARN_ON_IN_IRQ() WARN_ON_ONCE(!in_task()) +static inline bool pagefault_disabled(void); +# define WARN_ON_IN_IRQ() \ + WARN_ON_ONCE(!in_task() && !pagefault_disabled()) #else # define WARN_ON_IN_IRQ() #endif diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h index 37b226e..ef3032d 100644 --- a/include/linux/uaccess.h +++ b/include/linux/uaccess.h @@ -203,7 +203,10 @@ static inline void pagefault_enable(void) /* * Is the pagefault handler disabled? If so, user access methods will not sleep. */ -#define pagefault_disabled() (current->pagefault_disabled != 0) +static inline bool pagefault_disabled(void) +{ + return current->pagefault_disabled != 0; +} /* * The pagefault handler is in general disabled by pagefault_disable() or
WARNING: multiple messages have this Message-ID (diff)
From: Masami Hiramatsu <mhiramat@kernel.org> To: lkp@lists.01.org Subject: Re: [uaccess] 780464aed0: WARNING:at_arch/x86/include/asm/uaccess.h:#strnlen_user/0x Date: Tue, 05 Mar 2019 18:01:26 +0900 [thread overview] Message-ID: <20190305180126.0f7201d8ec733fdc887135f1@kernel.org> (raw) In-Reply-To: <20190305172241.2eef39c6ca74d09120bbbcad@kernel.org> [-- Attachment #1: Type: text/plain, Size: 2770 bytes --] On Tue, 5 Mar 2019 17:22:41 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > On Tue, 5 Mar 2019 11:36:35 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > On Mon, 4 Mar 2019 10:59:22 -0800 > > Linus Torvalds <torvalds@linux-foundation.org> wrote: > > > > I think the better way to do this is allowing strncpy_from_user() > > if some conditions are match, like > > > > - strncpy_from_user() will be able to copy user memory with set_fs(USER_DS) > > - strncpy_from_user() can copy kernel memory with set_fs(KERNEL_DS) > > - strncpy_from_user() can access unsafe memory in IRQ context if > > pagefault is disabled. > > > > This is almost done, except for CONFIG_DEBUG_ATOMIC_SLEEP=y on x86. > > > > So, what about adding a condition to WARN_ON_IN_IRQ() like below > > instead of introducing user_access_ok() ? OK, here is the patch. Does it work for us? ==== x86/uaccess: Verify access_ok() context strictly From: Masami Hiramatsu <mhiramat@kernel.org> WARN_ON_IN_IRQ() assumes that the access_ok() and following user memory access can sleep. But this assumption is not always correct; when the pagefault is disabled, following memory access will just returns -EFAULT and never sleep. Add pagefault_disabled() check in WARN_ON_ONCE() so that it can ignore the case we call it with disabling pagefault. For this purpose, this modified pagefault_disabled() as an inline function. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- arch/x86/include/asm/uaccess.h | 4 +++- include/linux/uaccess.h | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h index 780f2b4..b98a552 100644 --- a/arch/x86/include/asm/uaccess.h +++ b/arch/x86/include/asm/uaccess.h @@ -70,7 +70,9 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un }) #ifdef CONFIG_DEBUG_ATOMIC_SLEEP -# define WARN_ON_IN_IRQ() WARN_ON_ONCE(!in_task()) +static inline bool pagefault_disabled(void); +# define WARN_ON_IN_IRQ() \ + WARN_ON_ONCE(!in_task() && !pagefault_disabled()) #else # define WARN_ON_IN_IRQ() #endif diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h index 37b226e..ef3032d 100644 --- a/include/linux/uaccess.h +++ b/include/linux/uaccess.h @@ -203,7 +203,10 @@ static inline void pagefault_enable(void) /* * Is the pagefault handler disabled? If so, user access methods will not sleep. */ -#define pagefault_disabled() (current->pagefault_disabled != 0) +static inline bool pagefault_disabled(void) +{ + return current->pagefault_disabled != 0; +} /* * The pagefault handler is in general disabled by pagefault_disable() or
next prev parent reply other threads:[~2019-03-05 9:01 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 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 [this message] 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=20190305180126.0f7201d8ec733fdc887135f1@kernel.org \ --to=mhiramat@kernel.org \ --cc=akpm@linux-foundation.org \ --cc=alexei.starovoitov@gmail.com \ --cc=changbin.du@gmail.com \ --cc=jannh@google.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=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: linkBe 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.