From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Ben Dooks <ben.dooks@codethink.co.uk>,
syzbot+e74b94fe601ab9552d69@syzkaller.appspotmail.com,
Arnd Bergman <arnd@arndb.de>,
Palmer Dabbelt <palmerdabbelt@google.com>,
Sasha Levin <sashal@kernel.org>,
linux-riscv@lists.infradead.org
Subject: [PATCH AUTOSEL 5.11 20/22] riscv: evaluate put_user() arg before enabling user access
Date: Mon, 5 Apr 2021 12:04:03 -0400 [thread overview]
Message-ID: <20210405160406.268132-20-sashal@kernel.org> (raw)
In-Reply-To: <20210405160406.268132-1-sashal@kernel.org>
From: Ben Dooks <ben.dooks@codethink.co.uk>
[ Upstream commit 285a76bb2cf51b0c74c634f2aaccdb93e1f2a359 ]
The <asm/uaccess.h> header has a problem with put_user(a, ptr) if
the 'a' is not a simple variable, such as a function. This can lead
to the compiler producing code as so:
1: enable_user_access()
2: evaluate 'a' into register 'r'
3: put 'r' to 'ptr'
4: disable_user_acess()
The issue is that 'a' is now being evaluated with the user memory
protections disabled. So we try and force the evaulation by assigning
'x' to __val at the start, and hoping the compiler barriers in
enable_user_access() do the job of ordering step 2 before step 1.
This has shown up in a bug where 'a' sleeps and thus schedules out
and loses the SR_SUM flag. This isn't sufficient to fully fix, but
should reduce the window of opportunity. The first instance of this
we found is in scheudle_tail() where the code does:
$ less -N kernel/sched/core.c
4263 if (current->set_child_tid)
4264 put_user(task_pid_vnr(current), current->set_child_tid);
Here, the task_pid_vnr(current) is called within the block that has
enabled the user memory access. This can be made worse with KASAN
which makes task_pid_vnr() a rather large call with plenty of
opportunity to sleep.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Reported-by: syzbot+e74b94fe601ab9552d69@syzkaller.appspotmail.com
Suggested-by: Arnd Bergman <arnd@arndb.de>
--
Changes since v1:
- fixed formatting and updated the patch description with more info
Changes since v2:
- fixed commenting on __put_user() (schwab@linux-m68k.org)
Change since v3:
- fixed RFC in patch title. Should be ready to merge.
Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/riscv/include/asm/uaccess.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
index 824b2c9da75b..f944062c9d99 100644
--- a/arch/riscv/include/asm/uaccess.h
+++ b/arch/riscv/include/asm/uaccess.h
@@ -306,7 +306,9 @@ do { \
* data types like structures or arrays.
*
* @ptr must have pointer-to-simple-variable type, and @x must be assignable
- * to the result of dereferencing @ptr.
+ * to the result of dereferencing @ptr. The value of @x is copied to avoid
+ * re-ordering where @x is evaluated inside the block that enables user-space
+ * access (thus bypassing user space protection if @x is a function).
*
* Caller must check the pointer with access_ok() before calling this
* function.
@@ -316,12 +318,13 @@ do { \
#define __put_user(x, ptr) \
({ \
__typeof__(*(ptr)) __user *__gu_ptr = (ptr); \
+ __typeof__(*__gu_ptr) __val = (x); \
long __pu_err = 0; \
\
__chk_user_ptr(__gu_ptr); \
\
__enable_user_access(); \
- __put_user_nocheck(x, __gu_ptr, __pu_err); \
+ __put_user_nocheck(__val, __gu_ptr, __pu_err); \
__disable_user_access(); \
\
__pu_err; \
--
2.30.2
next prev parent reply other threads:[~2021-04-05 16:05 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-05 16:03 [PATCH AUTOSEL 5.11 01/22] interconnect: core: fix error return code of icc_link_destroy() Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 02/22] gfs2: Flag a withdraw if init_threads() fails Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 03/22] KVM: arm64: Hide system instruction access to Trace registers Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 04/22] KVM: arm64: Disable guest access to trace filter controls Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 05/22] drm/imx: imx-ldb: fix out of bounds array access warning Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 06/22] gfs2: report "already frozen/thawed" errors Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 07/22] scsi: iscsi: Fix race condition between login and sync thread Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 08/22] ftrace: Check if pages were allocated before calling free_pages() Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 09/22] tools/kvm_stat: Add restart delay Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 10/22] drm/tegra: dc: Don't set PLL clock to 0Hz Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 11/22] gpu: host1x: Use different lock classes for each client Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 12/22] XArray: Fix splitting to non-zero orders Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 13/22] radix tree test suite: Fix compilation Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 14/22] block: only update parent bi_status when bio fail Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 15/22] radix tree test suite: Register the main thread with the RCU library Sasha Levin
2021-04-05 16:03 ` [PATCH AUTOSEL 5.11 16/22] idr test suite: Take RCU read lock in idr_find_test_1 Sasha Levin
2021-04-05 16:04 ` [PATCH AUTOSEL 5.11 17/22] idr test suite: Create anchor before launching throbber Sasha Levin
2021-04-05 16:04 ` [PATCH AUTOSEL 5.11 18/22] null_blk: fix command timeout completion handling Sasha Levin
2021-04-05 16:04 ` [PATCH AUTOSEL 5.11 19/22] io_uring: don't mark S_ISBLK async work as unbounded Sasha Levin
2021-04-05 16:04 ` Sasha Levin [this message]
2021-04-05 16:04 ` [PATCH AUTOSEL 5.11 21/22] riscv,entry: fix misaligned base for excp_vect_table Sasha Levin
2021-04-05 16:04 ` [PATCH AUTOSEL 5.11 22/22] block: don't ignore REQ_NOWAIT for direct IO Sasha Levin
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=20210405160406.268132-20-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=arnd@arndb.de \
--cc=ben.dooks@codethink.co.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmerdabbelt@google.com \
--cc=stable@vger.kernel.org \
--cc=syzbot+e74b94fe601ab9552d69@syzkaller.appspotmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).