On Fri, Jul 26, 2024 at 2:43 AM Andrey Konovalov <andreyknvl@xxxxxxxxx> wrote:
> On Thu, Jul 25, 2024 at 5:32 PM Jann Horn <jannh@xxxxxxxxxx> wrote:
> >
> > Currently, when KASAN is combined with init-on-free behavior, the
> > initialization happens before KASAN's "invalid free" checks.
> >
> > More importantly, a subsequent commit will want to use the object metadata
> > region to store an rcu_head, and we should let KASAN check that the object
> > pointer is valid before that. (Otherwise that change will make the existing
> > testcase kmem_cache_invalid_free fail.)
>
> This is not the case since v3, right?
Oh, you're right, this text is now wrong.
> Do we still need this patch?
I just tried removing this patch from the series; without it, the
kmem_cache_invalid_free kunit test fails because the kmem_cache_free()
no longer synchronously notices that the pointer is misaligned. I
guess I could change the testcase like this to make the tests pass
without this patch, but I'd like to hear from you or another KASAN
person whether you think that's a reasonable change:
diff --git a/mm/kasan/kasan_test.c b/mm/kasan/kasan_test.c
index cba782a4b072..f44b0dcb0e84 100644
--- a/mm/kasan/kasan_test.c
+++ b/mm/kasan/kasan_test.c
@@ -981,14 +981,21 @@ static void kmem_cache_invalid_free(struct kunit *test)
if (!p) {
kunit_err(test, "Allocation failed: %s\n", __func__);
kmem_cache_destroy(cache);
return;
}
- /* Trigger invalid free, the object doesn't get freed. */
- KUNIT_EXPECT_KASAN_FAIL(test, kmem_cache_free(cache, p + 1));
+ /*
+ * Trigger invalid free, the object doesn't get freed.
+ * Note that the invalid free detection may happen asynchronously
+ * under CONFIG_SLUB_RCU_DEBUG.
+ */
+ KUNIT_EXPECT_KASAN_FAIL(test, ({
+ kmem_cache_free(cache, p + 1);
+ rcu_barrier();
+ }));
Being able to get rid of this patch would be a nice simplification, so
if you think asynchronous invalid-free detection for TYPESAFE_BY_RCU
slabs is fine, I'll happily throw it out.
