| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACzwLxg7_HPxbjLT1v+dHG=V0wAcUJYZvqdcdLBD9xhLgNUmqQ@mail.gmail.com>
Date: Mon, 23 Sep 2024 11:09:33 +0500
From: Sabyrzhan Tasbolatov <snovitoll@...il.com>
To: andreyknvl@...il.com
Cc: akpm@...ux-foundation.org, bp@...en8.de, brauner@...nel.org,
dave.hansen@...ux.intel.com, dhowells@...hat.com, dvyukov@...gle.com,
glider@...gle.com, hpa@...or.com, kasan-dev@...glegroups.com,
linux-kernel@...r.kernel.org, linux-mm@...ck.org, mingo@...hat.com,
ryabinin.a.a@...il.com, tglx@...utronix.de, vincenzo.frascino@....com,
x86@...nel.org
Subject: Re: [PATCH v5] mm: x86: instrument __get/__put_kernel_nofault
On Sun, Sep 22, 2024 at 7:57 PM Sabyrzhan Tasbolatov
<snovitoll@...il.com> wrote:
> arch/x86/include/asm/uaccess.h | 3 +++
> mm/kasan/kasan_test.c | 23 +++++++++++++++++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> index 3a7755c1a441..e8e5185dd65c 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -620,6 +620,7 @@ do { \
>
> #ifdef CONFIG_CC_HAS_ASM_GOTO_OUTPUT
> #define __get_kernel_nofault(dst, src, type, err_label) \
> + instrument_memcpy_before(dst, src, sizeof(type)); \
> __get_user_size(*((type *)(dst)), (__force type __user *)(src), \
> sizeof(type), err_label)
> #else // !CONFIG_CC_HAS_ASM_GOTO_OUTPUT
> @@ -627,6 +628,7 @@ do { \
> do { \
> int __kr_err; \
> \
> + instrument_memcpy_before(dst, src, sizeof(type)); \
> __get_user_size(*((type *)(dst)), (__force type __user *)(src), \
> sizeof(type), __kr_err); \
> if (unlikely(__kr_err)) \
> @@ -635,6 +637,7 @@ do { \
> #endif // CONFIG_CC_HAS_ASM_GOTO_OUTPUT
>
> #define __put_kernel_nofault(dst, src, type, err_label) \
> + instrument_write(dst, sizeof(type)); \
> __put_user_size(*((type *)(src)), (__force type __user *)(dst), \
> sizeof(type), err_label)
>
Instead of adding KASAN, KCSAN checks per arch macro,
here is the alternative, generic way with a wrapper.
I've tested it on x86_64 only, going to test on arm64
with KASAN_SW_TAGS, KASAN_HW_TAGS if I can do it in qemu,
and form a new patch for all arch
and this PATCH v5 for x86 only can be abandoned.
Please let me know if this wrapper is good enough,
I will see in kasan_test.c how I should use SW/HW_TAG, probably,
they should be a separate test with
KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_SW_TAGS);
---
include/linux/uaccess.h | 8 ++++++++
mm/kasan/kasan_test.c | 21 +++++++++++++++++++++
mm/maccess.c | 4 ++--
3 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index d8e4105a2f21..1b5c23868f97 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -422,6 +422,14 @@ do { \
} while (0)
#endif
+#define __get_kernel_nofault_wrapper(dst, src, type, err_label) \
+ instrument_memcpy_before(dst, src, sizeof(type)); \
+ __get_kernel_nofault(dst, src, type, err_label); \
+
+#define __put_kernel_nofault_wrapper(dst, src, type, err_label) \
+ instrument_write(dst, sizeof(type)); \
+ __put_kernel_nofault(dst, src, type, err_label); \
+
/**
* get_kernel_nofault(): safely attempt to read from a location
* @val: read into this variable
diff --git a/mm/kasan/kasan_test.c b/mm/kasan/kasan_test.c
index 567d33b493e2..ae05c8858c07 100644
--- a/mm/kasan/kasan_test.c
+++ b/mm/kasan/kasan_test.c
@@ -1944,6 +1944,26 @@ static void match_all_mem_tag(struct kunit *test)
kfree(ptr);
}
+static void copy_from_to_kernel_nofault_oob(struct kunit *test)
+{
+ char *ptr;
+ char buf[128];
+ size_t size = sizeof(buf);
+
+ ptr = kmalloc(size - KASAN_GRANULE_SIZE, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
+
+ KUNIT_EXPECT_KASAN_FAIL(test,
+ copy_from_kernel_nofault(&buf[0], ptr, size));
+ KUNIT_EXPECT_KASAN_FAIL(test,
+ copy_from_kernel_nofault(ptr, &buf[0], size));
+ KUNIT_EXPECT_KASAN_FAIL(test,
+ copy_to_kernel_nofault(&buf[0], ptr, size));
+ KUNIT_EXPECT_KASAN_FAIL(test,
+ copy_to_kernel_nofault(ptr, &buf[0], size));
+ kfree(ptr);
+}
+
static struct kunit_case kasan_kunit_test_cases[] = {
KUNIT_CASE(kmalloc_oob_right),
KUNIT_CASE(kmalloc_oob_left),
@@ -2017,6 +2037,7 @@ static struct kunit_case kasan_kunit_test_cases[] = {
KUNIT_CASE(match_all_not_assigned),
KUNIT_CASE(match_all_ptr_tag),
KUNIT_CASE(match_all_mem_tag),
+ KUNIT_CASE(copy_from_to_kernel_nofault_oob),
{}
};
diff --git a/mm/maccess.c b/mm/maccess.c
index 518a25667323..a3533a0d0677 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -15,7 +15,7 @@ bool __weak copy_from_kernel_nofault_allowed(const
void *unsafe_src,
#define copy_from_kernel_nofault_loop(dst, src, len, type, err_label) \
while (len >= sizeof(type)) { \
- __get_kernel_nofault(dst, src, type, err_label); \
+ __get_kernel_nofault_wrapper(dst, src, type, err_label);\
dst += sizeof(type); \
src += sizeof(type); \
len -= sizeof(type); \
@@ -49,7 +49,7 @@ EXPORT_SYMBOL_GPL(copy_from_kernel_nofault);
#define copy_to_kernel_nofault_loop(dst, src, len, type, err_label) \
while (len >= sizeof(type)) { \
- __put_kernel_nofault(dst, src, type, err_label); \
+ __put_kernel_nofault_wrapper(dst, src, type, err_label);\
dst += sizeof(type); \
src += sizeof(type); \
len -= sizeof(type); \
--
Powered by blists - more mailing lists