* [PATCH v3] mm/kfence: Add a new kunit test test_use_after_free_read_nofault()
@ 2024-10-18 17:46 Ritesh Harjani (IBM)
2024-10-18 21:02 ` Marco Elver
0 siblings, 1 reply; 3+ messages in thread
From: Ritesh Harjani (IBM) @ 2024-10-18 17:46 UTC (permalink / raw)
To: kasan-dev
Cc: linuxppc-dev, linux-mm, Dmitry Vyukov, Marco Elver,
Alexander Potapenko, Heiko Carstens, Nirjhar Roy,
Ritesh Harjani (IBM)
From: Nirjhar Roy <nirjhar@linux.ibm.com>
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 <nirjhar@linux.ibm.com>
Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
Will be nice if we can get some feedback on this.
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
[powerpc+kfence+v3]: https://lore.kernel.org/linuxppc-dev/cover.1729271995.git.ritesh.list@gmail.com
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3] mm/kfence: Add a new kunit test test_use_after_free_read_nofault()
2024-10-18 17:46 [PATCH v3] mm/kfence: Add a new kunit test test_use_after_free_read_nofault() Ritesh Harjani (IBM)
@ 2024-10-18 21:02 ` Marco Elver
2024-11-13 1:56 ` Ritesh Harjani
0 siblings, 1 reply; 3+ messages in thread
From: Marco Elver @ 2024-10-18 21:02 UTC (permalink / raw)
To: Ritesh Harjani (IBM)
Cc: kasan-dev, linuxppc-dev, linux-mm, Dmitry Vyukov,
Alexander Potapenko, Heiko Carstens, Nirjhar Roy
On Fri, 18 Oct 2024 at 19:46, Ritesh Harjani (IBM)
<ritesh.list@gmail.com> wrote:
>
> From: Nirjhar Roy <nirjhar@linux.ibm.com>
>
> 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 <nirjhar@linux.ibm.com>
> 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/
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>
> 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
> [powerpc+kfence+v3]: https://lore.kernel.org/linuxppc-dev/cover.1729271995.git.ritesh.list@gmail.com
>
> 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
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3] mm/kfence: Add a new kunit test test_use_after_free_read_nofault()
2024-10-18 21:02 ` Marco Elver
@ 2024-11-13 1:56 ` Ritesh Harjani
0 siblings, 0 replies; 3+ messages in thread
From: Ritesh Harjani @ 2024-11-13 1:56 UTC (permalink / raw)
To: Marco Elver
Cc: kasan-dev, linuxppc-dev, linux-mm, Dmitry Vyukov,
Alexander Potapenko, Heiko Carstens, Nirjhar Roy, Andrew Morton
Marco Elver <elver@google.com> writes:
> On Fri, 18 Oct 2024 at 19:46, Ritesh Harjani (IBM)
> <ritesh.list@gmail.com> wrote:
>>
>> From: Nirjhar Roy <nirjhar@linux.ibm.com>
>>
>> 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 <nirjhar@linux.ibm.com>
>> 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/
>
> 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
>> 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
>> [powerpc+kfence+v3]: https://lore.kernel.org/linuxppc-dev/cover.1729271995.git.ritesh.list@gmail.com
>>
>> 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
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-11-13 1:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-18 17:46 [PATCH v3] mm/kfence: Add a new kunit test test_use_after_free_read_nofault() Ritesh Harjani (IBM)
2024-10-18 21:02 ` Marco Elver
2024-11-13 1:56 ` Ritesh Harjani
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox