Skip to content

Commit e4137f0

Browse files
novitollakpm00
authored andcommitted
mm, kasan, kmsan: instrument copy_from/to_kernel_nofault
Instrument copy_from_kernel_nofault() with KMSAN for uninitialized kernel memory check and copy_to_kernel_nofault() with KASAN, KCSAN to detect the memory corruption. syzbot reported that bpf_probe_read_kernel() kernel helper triggered KASAN report via kasan_check_range() which is not the expected behaviour as copy_from_kernel_nofault() is meant to be a non-faulting helper. Solution is, suggested by Marco Elver, to replace KASAN, KCSAN check in copy_from_kernel_nofault() with KMSAN detection of copying uninitilaized kernel memory. In copy_to_kernel_nofault() we can retain instrument_write() explicitly for the memory corruption instrumentation. copy_to_kernel_nofault() is tested on x86_64 and arm64 with CONFIG_KASAN_SW_TAGS. On arm64 with CONFIG_KASAN_HW_TAGS, kunit test currently fails. Need more clarification on it. [akpm@linux-foundation.org: fix comment layout, per checkpatch Link: https://lore.kernel.org/linux-mm/CANpmjNMAVFzqnCZhEity9cjiqQ9CVN1X7qeeeAp_6yKjwKo8iw@mail.gmail.com/ Link: https://lkml.kernel.org/r/20241011035310.2982017-1-snovitoll@gmail.com Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@gmail.com> Reviewed-by: Marco Elver <elver@google.com> Reported-by: syzbot+61123a5daeb9f7454599@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=61123a5daeb9f7454599 Reported-by: Andrey Konovalov <andreyknvl@gmail.com> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=210505 Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com> [KASAN] Tested-by: Andrey Konovalov <andreyknvl@gmail.com> [KASAN] Cc: Alexander Potapenko <glider@google.com> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com> Cc: Dmitry Vyukov <dvyukov@google.com> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
1 parent 908378a commit e4137f0

File tree

3 files changed

+61
-2
lines changed

3 files changed

+61
-2
lines changed

mm/kasan/kasan_test_c.c

+36
Original file line numberDiff line numberDiff line change
@@ -1928,6 +1928,41 @@ static void rust_uaf(struct kunit *test)
19281928
KUNIT_EXPECT_KASAN_FAIL(test, kasan_test_rust_uaf());
19291929
}
19301930

1931+
static void copy_to_kernel_nofault_oob(struct kunit *test)
1932+
{
1933+
char *ptr;
1934+
char buf[128];
1935+
size_t size = sizeof(buf);
1936+
1937+
/*
1938+
* This test currently fails with the HW_TAGS mode. The reason is
1939+
* unknown and needs to be investigated.
1940+
*/
1941+
KASAN_TEST_NEEDS_CONFIG_OFF(test, CONFIG_KASAN_HW_TAGS);
1942+
1943+
ptr = kmalloc(size - KASAN_GRANULE_SIZE, GFP_KERNEL);
1944+
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
1945+
OPTIMIZER_HIDE_VAR(ptr);
1946+
1947+
/*
1948+
* We test copy_to_kernel_nofault() to detect corrupted memory that is
1949+
* being written into the kernel. In contrast,
1950+
* copy_from_kernel_nofault() is primarily used in kernel helper
1951+
* functions where the source address might be random or uninitialized.
1952+
* Applying KASAN instrumentation to copy_from_kernel_nofault() could
1953+
* lead to false positives. By focusing KASAN checks only on
1954+
* copy_to_kernel_nofault(), we ensure that only valid memory is
1955+
* written to the kernel, minimizing the risk of kernel corruption
1956+
* while avoiding false positives in the reverse case.
1957+
*/
1958+
KUNIT_EXPECT_KASAN_FAIL(test,
1959+
copy_to_kernel_nofault(&buf[0], ptr, size));
1960+
KUNIT_EXPECT_KASAN_FAIL(test,
1961+
copy_to_kernel_nofault(ptr, &buf[0], size));
1962+
1963+
kfree(ptr);
1964+
}
1965+
19311966
static struct kunit_case kasan_kunit_test_cases[] = {
19321967
KUNIT_CASE(kmalloc_oob_right),
19331968
KUNIT_CASE(kmalloc_oob_left),
@@ -2000,6 +2035,7 @@ static struct kunit_case kasan_kunit_test_cases[] = {
20002035
KUNIT_CASE(match_all_not_assigned),
20012036
KUNIT_CASE(match_all_ptr_tag),
20022037
KUNIT_CASE(match_all_mem_tag),
2038+
KUNIT_CASE(copy_to_kernel_nofault_oob),
20032039
KUNIT_CASE(rust_uaf),
20042040
{}
20052041
};

mm/kmsan/kmsan_test.c

+17
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,22 @@ static void test_unpoison_memory(struct kunit *test)
640640
KUNIT_EXPECT_TRUE(test, report_matches(&expect));
641641
}
642642

643+
static void test_copy_from_kernel_nofault(struct kunit *test)
644+
{
645+
long ret;
646+
char buf[4], src[4];
647+
size_t size = sizeof(buf);
648+
649+
EXPECTATION_UNINIT_VALUE_FN(expect, "copy_from_kernel_nofault");
650+
kunit_info(
651+
test,
652+
"testing copy_from_kernel_nofault with uninitialized memory\n");
653+
654+
ret = copy_from_kernel_nofault((char *)&buf[0], (char *)&src[0], size);
655+
USE(ret);
656+
KUNIT_EXPECT_TRUE(test, report_matches(&expect));
657+
}
658+
643659
static struct kunit_case kmsan_test_cases[] = {
644660
KUNIT_CASE(test_uninit_kmalloc),
645661
KUNIT_CASE(test_init_kmalloc),
@@ -664,6 +680,7 @@ static struct kunit_case kmsan_test_cases[] = {
664680
KUNIT_CASE(test_long_origin_chain),
665681
KUNIT_CASE(test_stackdepot_roundtrip),
666682
KUNIT_CASE(test_unpoison_memory),
683+
KUNIT_CASE(test_copy_from_kernel_nofault),
667684
{},
668685
};
669686

mm/maccess.c

+8-2
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,14 @@ bool __weak copy_from_kernel_nofault_allowed(const void *unsafe_src,
1313
return true;
1414
}
1515

16+
/*
17+
* The below only uses kmsan_check_memory() to ensure uninitialized kernel
18+
* memory isn't leaked.
19+
*/
1620
#define copy_from_kernel_nofault_loop(dst, src, len, type, err_label) \
1721
while (len >= sizeof(type)) { \
18-
__get_kernel_nofault(dst, src, type, err_label); \
22+
__get_kernel_nofault(dst, src, type, err_label); \
23+
kmsan_check_memory(src, sizeof(type)); \
1924
dst += sizeof(type); \
2025
src += sizeof(type); \
2126
len -= sizeof(type); \
@@ -49,7 +54,8 @@ EXPORT_SYMBOL_GPL(copy_from_kernel_nofault);
4954

5055
#define copy_to_kernel_nofault_loop(dst, src, len, type, err_label) \
5156
while (len >= sizeof(type)) { \
52-
__put_kernel_nofault(dst, src, type, err_label); \
57+
__put_kernel_nofault(dst, src, type, err_label); \
58+
instrument_write(dst, sizeof(type)); \
5359
dst += sizeof(type); \
5460
src += sizeof(type); \
5561
len -= sizeof(type); \

0 commit comments

Comments
 (0)