From: Florian Lehner <dev@der-flo.net>
To: bpf@vger.kernel.org
Cc: x86@kernel.org, davem@davemloft.net, ast@kernel.org,
daniel@iogearbox.net, andrii@kernel.org, peterz@infradead.org,
keescook@chromium.org, tglx@linutronix.de, hsinweih@uci.edu,
rostedt@goodmis.org, vegard.nossum@oracle.com,
gregkh@linuxfoundation.org, alan.maguire@oracle.com,
dylany@meta.com, riel@surriel.com,
Florian Lehner <dev@der-flo.net>
Subject: [v2 bpf-next 1/2] mm: Fix copy_from_user_nofault().
Date: Mon, 10 Apr 2023 19:43:44 +0200 [thread overview]
Message-ID: <20230410174345.4376-2-dev@der-flo.net> (raw)
In-Reply-To: <20230410174345.4376-1-dev@der-flo.net>
From: Alexei Starovoitov <ast@kernel.org>
There are several issues with copy_from_user_nofault():
- access_ok() is designed for user context only and for that reason
it has WARN_ON_IN_IRQ() which triggers when bpf, kprobe, eprobe
and perf on ppc are calling it from irq.
- it's missing nmi_uaccess_okay() which is a nop on all architectures
except x86 where it's required.
The comment in arch/x86/mm/tlb.c explains the details why it's necessary.
Calling copy_from_user_nofault() from bpf, [ke]probe without this check is not safe.
- __copy_from_user_inatomic() under CONFIG_HARDENED_USERCOPY is calling
check_object_size()->__check_object_size()->check_heap_object()->find_vmap_area()->spin_lock()
which is not safe to do from bpf, [ke]probe and perf due to potential deadlock.
Fix all three issues. At the end the copy_from_user_nofault() becomes
equivalent to copy_from_user_nmi() from safety point of view with
a difference in the return value.
Reported-by: Hsin-Wei Hung <hsinweih@uci.edu>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Florian Lehner <dev@der-flo.net>
Tested-by: Hsin-Wei Hung <hsinweih@uci.edu>
Tested-by: Florian Lehner <dev@der-flo.net>
---
mm/maccess.c | 16 +++++++++++-----
mm/usercopy.c | 2 +-
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/mm/maccess.c b/mm/maccess.c
index 074f6b086671..518a25667323 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -5,6 +5,7 @@
#include <linux/export.h>
#include <linux/mm.h>
#include <linux/uaccess.h>
+#include <asm/tlb.h>
bool __weak copy_from_kernel_nofault_allowed(const void *unsafe_src,
size_t size)
@@ -113,11 +114,16 @@ long strncpy_from_kernel_nofault(char *dst, const void *unsafe_addr, long count)
long copy_from_user_nofault(void *dst, const void __user *src, size_t size)
{
long ret = -EFAULT;
- if (access_ok(src, size)) {
- pagefault_disable();
- ret = __copy_from_user_inatomic(dst, src, size);
- pagefault_enable();
- }
+
+ if (!__access_ok(src, size))
+ return ret;
+
+ if (!nmi_uaccess_okay())
+ return ret;
+
+ pagefault_disable();
+ ret = __copy_from_user_inatomic(dst, src, size);
+ pagefault_enable();
if (ret)
return -EFAULT;
diff --git a/mm/usercopy.c b/mm/usercopy.c
index 4c3164beacec..83c164aba6e0 100644
--- a/mm/usercopy.c
+++ b/mm/usercopy.c
@@ -173,7 +173,7 @@ static inline void check_heap_object(const void *ptr, unsigned long n,
return;
}
- if (is_vmalloc_addr(ptr)) {
+ if (is_vmalloc_addr(ptr) && !pagefault_disabled()) {
struct vmap_area *area = find_vmap_area(addr);
if (!area)
--
2.39.2
next prev parent reply other threads:[~2023-04-10 17:44 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-10 17:43 [v2 bpf-next 0/2] Fix copy_from_user_nofault() Florian Lehner
2023-04-10 17:43 ` Florian Lehner [this message]
2023-04-10 17:43 ` [v2 bpf-next 2/2] perf: Fix arch_perf_out_copy_user() Florian Lehner
2023-04-13 0:43 ` Alexei Starovoitov
2023-04-13 0:50 ` [v2 bpf-next 0/2] Fix copy_from_user_nofault() patchwork-bot+netdevbpf
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=20230410174345.4376-2-dev@der-flo.net \
--to=dev@der-flo.net \
--cc=alan.maguire@oracle.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=dylany@meta.com \
--cc=gregkh@linuxfoundation.org \
--cc=hsinweih@uci.edu \
--cc=keescook@chromium.org \
--cc=peterz@infradead.org \
--cc=riel@surriel.com \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=vegard.nossum@oracle.com \
--cc=x86@kernel.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.