Skip to content

Commit 386788a

Browse files
Eric Biggersgregkh
authored andcommitted
kmsan: fix out-of-bounds access to shadow memory
commit 85e1ff6 upstream. Running sha224_kunit on a KMSAN-enabled kernel results in a crash in kmsan_internal_set_shadow_origin(): BUG: unable to handle page fault for address: ffffbc3840291000 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 1810067 P4D 1810067 PUD 192d067 PMD 3c17067 PTE 0 Oops: 0000 [#1] SMP NOPTI CPU: 0 UID: 0 PID: 81 Comm: kunit_try_catch Tainted: G N 6.17.0-rc3 torvalds#10 PREEMPT(voluntary) Tainted: [N]=TEST Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014 RIP: 0010:kmsan_internal_set_shadow_origin+0x91/0x100 [...] Call Trace: <TASK> __msan_memset+0xee/0x1a0 sha224_final+0x9e/0x350 test_hash_buffer_overruns+0x46f/0x5f0 ? kmsan_get_shadow_origin_ptr+0x46/0xa0 ? __pfx_test_hash_buffer_overruns+0x10/0x10 kunit_try_run_case+0x198/0xa00 This occurs when memset() is called on a buffer that is not 4-byte aligned and extends to the end of a guard page, i.e. the next page is unmapped. The bug is that the loop at the end of kmsan_internal_set_shadow_origin() accesses the wrong shadow memory bytes when the address is not 4-byte aligned. Since each 4 bytes are associated with an origin, it rounds the address and size so that it can access all the origins that contain the buffer. However, when it checks the corresponding shadow bytes for a particular origin, it incorrectly uses the original unrounded shadow address. This results in reads from shadow memory beyond the end of the buffer's shadow memory, which crashes when that memory is not mapped. To fix this, correctly align the shadow address before accessing the 4 shadow bytes corresponding to each origin. Link: https://lkml.kernel.org/r/[email protected] Fixes: 2ef3cec ("kmsan: do not wipe out origin when doing partial unpoisoning") Signed-off-by: Eric Biggers <[email protected]> Tested-by: Alexander Potapenko <[email protected]> Reviewed-by: Alexander Potapenko <[email protected]> Cc: Dmitriy Vyukov <[email protected]> Cc: Marco Elver <[email protected]> Cc: <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 3b29f52 commit 386788a

File tree

2 files changed

+23
-3
lines changed

2 files changed

+23
-3
lines changed

mm/kmsan/core.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,8 @@ void kmsan_internal_set_shadow_origin(void *addr, size_t size, int b,
195195
u32 origin, bool checked)
196196
{
197197
u64 address = (u64)addr;
198-
u32 *shadow_start, *origin_start;
198+
void *shadow_start;
199+
u32 *aligned_shadow, *origin_start;
199200
size_t pad = 0;
200201

201202
KMSAN_WARN_ON(!kmsan_metadata_is_contiguous(addr, size));
@@ -214,9 +215,12 @@ void kmsan_internal_set_shadow_origin(void *addr, size_t size, int b,
214215
}
215216
__memset(shadow_start, b, size);
216217

217-
if (!IS_ALIGNED(address, KMSAN_ORIGIN_SIZE)) {
218+
if (IS_ALIGNED(address, KMSAN_ORIGIN_SIZE)) {
219+
aligned_shadow = shadow_start;
220+
} else {
218221
pad = address % KMSAN_ORIGIN_SIZE;
219222
address -= pad;
223+
aligned_shadow = shadow_start - pad;
220224
size += pad;
221225
}
222226
size = ALIGN(size, KMSAN_ORIGIN_SIZE);
@@ -230,7 +234,7 @@ void kmsan_internal_set_shadow_origin(void *addr, size_t size, int b,
230234
* corresponding shadow slot is zero.
231235
*/
232236
for (int i = 0; i < size / KMSAN_ORIGIN_SIZE; i++) {
233-
if (origin || !shadow_start[i])
237+
if (origin || !aligned_shadow[i])
234238
origin_start[i] = origin;
235239
}
236240
}

mm/kmsan/kmsan_test.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,21 @@ DEFINE_TEST_MEMSETXX(16)
556556
DEFINE_TEST_MEMSETXX(32)
557557
DEFINE_TEST_MEMSETXX(64)
558558

559+
/* Test case: ensure that KMSAN does not access shadow memory out of bounds. */
560+
static void test_memset_on_guarded_buffer(struct kunit *test)
561+
{
562+
void *buf = vmalloc(PAGE_SIZE);
563+
564+
kunit_info(test,
565+
"memset() on ends of guarded buffer should not crash\n");
566+
567+
for (size_t size = 0; size <= 128; size++) {
568+
memset(buf, 0xff, size);
569+
memset(buf + PAGE_SIZE - size, 0xff, size);
570+
}
571+
vfree(buf);
572+
}
573+
559574
static noinline void fibonacci(int *array, int size, int start)
560575
{
561576
if (start < 2 || (start == size))
@@ -677,6 +692,7 @@ static struct kunit_case kmsan_test_cases[] = {
677692
KUNIT_CASE(test_memset16),
678693
KUNIT_CASE(test_memset32),
679694
KUNIT_CASE(test_memset64),
695+
KUNIT_CASE(test_memset_on_guarded_buffer),
680696
KUNIT_CASE(test_long_origin_chain),
681697
KUNIT_CASE(test_stackdepot_roundtrip),
682698
KUNIT_CASE(test_unpoison_memory),

0 commit comments

Comments
 (0)