Bug 212513

Here's a draft of test case that checks functions from include/asm-generic/rwonce.h:

diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index bf9225002a7e..b9d942ae5c7a 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -1062,6 +1062,37 @@ static void match_all_mem_tag(struct kunit *test)
        kfree(ptr);
 }
 
+/* Check that different variants of READ/WRITE_ONCE() work properly. */
+/* XXX: some of these fail in QEMU due to:
+ * https://bugs.launchpad.net/qemu/+bug/1921948 */
+static void access_once(struct kunit *test)
+{
+       char *data;
+       unsigned long *aligned_ptr, *unaligned_ptr;
+       size_t size = 128 - KASAN_GRANULE_SIZE;
+
+       data = kmalloc(size, GFP_KERNEL);
+       aligned_ptr = (unsigned long *)(data + size);
+       unaligned_ptr = (unsigned long *)(data + size - 4);
+
+       KUNIT_EXPECT_KASAN_FAIL(test, WRITE_ONCE(*aligned_ptr, 0));
+       KUNIT_EXPECT_KASAN_FAIL(test, READ_ONCE(*aligned_ptr));
+
+       KUNIT_EXPECT_KASAN_FAIL(test, WRITE_ONCE(*unaligned_ptr, 0));
+       KUNIT_EXPECT_KASAN_FAIL(test, READ_ONCE(*unaligned_ptr));
+
+       /* XXX: these fail due to missing kasan_reset_tag(). */
+       READ_ONCE_NOCHECK(*aligned_ptr);
+       READ_ONCE_NOCHECK(*unaligned_ptr);
+
+       KUNIT_EXPECT_KASAN_FAIL(test,
+               kasan_int_result = read_word_at_a_time(aligned_ptr));
+       /* XXX: this fails due to missing kasan_reset_tag(). */
+       kasan_int_result = read_word_at_a_time(unaligned_ptr);
+
+       kfree(data);
+}
+
 static struct kunit_case kasan_kunit_test_cases[] = {
        KUNIT_CASE(kmalloc_oob_right),
        KUNIT_CASE(kmalloc_oob_left),
@@ -1111,6 +1142,7 @@ static struct kunit_case kasan_kunit_test_cases[] = {
        KUNIT_CASE(match_all_not_assigned),
        KUNIT_CASE(match_all_ptr_tag),
        KUNIT_CASE(match_all_mem_tag),
+       KUNIT_CASE(access_once),
        {}
 };

Note, that there are more functions that need to be annotated. Grep for __no_sanitize_address, __no_kasan_or_inline, __no_sanitize_or_inline.

Note, that the test relies on the patch below, which is not yet in the mainline:

https://lore.kernel.org/linux-mm/CANpmjNMpT0rYKfywkGvqLy8tk3iP6wAuGxHpHVJA77+EG4c5Gg@mail.gmail.com/T/#t

An alternative idea would be to create a compiler attribute that does

mrs reg, tco
msr tco, #1

on entry and

msr tco, reg

on exit.

This sounds like an idea worth considering. This way we don't need to add more annotations. But we would need to bump the compiler version requirement.

One complication though is that it would need alternatives in order to support hardware without MTE, which implies that the compiler would need to know about alternatives.

Perhaps we could use __attribute__((cleanup)) to run the code on exit instead of an attribute. It looks like this attribute is allowed in the kernel, e.g. it's used in include/linux/kcsan-checks.h.

Re __attribute__((cleanup)): "allowed" under very restricted circumstances. I'd rather say it's "tolerated" :-)

For KCSAN I decided to use it because the abstraction it's hidden behind is not explicitly about resource management, but a special assertion. Furthermore, we know that all compilers that support KCSAN support the cleanup attribute (although these days, with the kernel requiring at least GCC 4.9 and Clang 10, this isn't a problem either). Last but not least, nobody really objected I believe because KCSAN is a debugging tool that is usually off and nobody would notice.

My guess is that any attempts to diversify the kernel's resource management strategies (e.g. vs normal "goto out" or some such) with __attribute__((cleanup)) will be frowned upon and be rejected -- which I'd also agree with because it'd create inconsistencies and in some cases also makes it hard to reason about what is executed on function return vs. a simple "goto out" (at least that's one argument I recall hearing somewhere), which can be quite crucial in certain contexts (e.g. see all the 'noinstr' functions restrictions).

So, my guess is that if the usecase is closer to KCSAN's usecase, then it should be fine, i.e. it's hidden behind some abstraction that is for KASAN_HW_TAGS only, and not about resource management explicitly.