Marco Elver [off-list ref] writes:

On Fri, 18 Oct 2024 at 19:46, Ritesh Harjani (IBM)
[off-list ref] wrote:

quoted

From: Nirjhar Roy <redacted>

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@gmail.com>
Signed-off-by: Nirjhar Roy <redacted>
Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---

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@mail.gmail.com/ (local)

It should be similar for KFENCE, i.e. no report should be generated.
Definitely a good thing to test.

Tested-by: Marco Elver <elver@google.com>
Reviewed-by: Marco Elver <elver@google.com>

Gentle ping. Is this going into -next?

-ritesh

quoted

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@gmail.com (local)
[powerpc+kfence+v3]: https://lore.kernel.org/linuxppc-dev/cover.1729271995.git.ritesh.list@gmail.com (local)

 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