Marco Elver <elver@xxxxxxxxxx> writes: > On Fri, 18 Oct 2024 at 19:46, Ritesh Harjani (IBM) > <ritesh.list@xxxxxxxxx> wrote: >> >> From: Nirjhar Roy <nirjhar@xxxxxxxxxxxxx> >> >> Faults from copy_from_kernel_nofault() needs to be handled by fixup >> table and should not be handled by kfence. Otherwise while reading >> /proc/kcore which uses copy_from_kernel_nofault(), kfence can generate >> false negatives. This can happen when /proc/kcore ends up reading an >> unmapped address from kfence pool. >> >> Let's add a testcase to cover this case. >> >> Co-developed-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx> >> Signed-off-by: Nirjhar Roy <nirjhar@xxxxxxxxxxxxx> >> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx> >> --- >> >> Will be nice if we can get some feedback on this. > > There was some discussion recently how sanitizers should behave around > these nofault helpers when accessing invalid memory (including freed > memory): > https://lore.kernel.org/all/CANpmjNMAVFzqnCZhEity9cjiqQ9CVN1X7qeeeAp_6yKjwKo8iw@xxxxxxxxxxxxxx/ > > It should be similar for KFENCE, i.e. no report should be generated. > Definitely a good thing to test. > > Tested-by: Marco Elver <elver@xxxxxxxxxx> > Reviewed-by: Marco Elver <elver@xxxxxxxxxx> > Gentle ping. Is this going into -next? -ritesh >> v2 -> v3: >> ========= >> 1. Separated out this kfence kunit test from the larger powerpc+kfence+v3 series. >> 2. Dropped RFC tag >> >> [v2]: https://lore.kernel.org/linuxppc-dev/cover.1728954719.git.ritesh.list@xxxxxxxxx >> [powerpc+kfence+v3]: https://lore.kernel.org/linuxppc-dev/cover.1729271995.git.ritesh.list@xxxxxxxxx >> >> mm/kfence/kfence_test.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/mm/kfence/kfence_test.c b/mm/kfence/kfence_test.c >> index 00fd17285285..f65fb182466d 100644 >> --- a/mm/kfence/kfence_test.c >> +++ b/mm/kfence/kfence_test.c >> @@ -383,6 +383,22 @@ static void test_use_after_free_read(struct kunit *test) >> KUNIT_EXPECT_TRUE(test, report_matches(&expect)); >> } >> >> +static void test_use_after_free_read_nofault(struct kunit *test) >> +{ >> + const size_t size = 32; >> + char *addr; >> + char dst; >> + int ret; >> + >> + setup_test_cache(test, size, 0, NULL); >> + addr = test_alloc(test, size, GFP_KERNEL, ALLOCATE_ANY); >> + test_free(addr); >> + /* Use after free with *_nofault() */ >> + ret = copy_from_kernel_nofault(&dst, addr, 1); >> + KUNIT_EXPECT_EQ(test, ret, -EFAULT); >> + KUNIT_EXPECT_FALSE(test, report_available()); >> +} >> + >> static void test_double_free(struct kunit *test) >> { >> const size_t size = 32; >> @@ -780,6 +796,7 @@ static struct kunit_case kfence_test_cases[] = { >> KFENCE_KUNIT_CASE(test_out_of_bounds_read), >> KFENCE_KUNIT_CASE(test_out_of_bounds_write), >> KFENCE_KUNIT_CASE(test_use_after_free_read), >> + KFENCE_KUNIT_CASE(test_use_after_free_read_nofault), >> KFENCE_KUNIT_CASE(test_double_free), >> KFENCE_KUNIT_CASE(test_invalid_addr_free), >> KFENCE_KUNIT_CASE(test_corruption), >> -- >> 2.46.0 >>
