Re: [PATCH v2 4/4] kmsan: introduce test_memcpy_initialized_gap()

[Marco Elver <elver@google.com>]

On Mon, 11 Sept 2023 at 16:57, Alexander Potapenko <glider@google.com> wrote:
>
> Add a regression test for the special case where memcpy() previously
> failed to correctly set the origins: if upon memcpy() four aligned
> initialized bytes with a zero origin value ended up split between two
> aligned four-byte chunks, one of those chunks could've received the zero
> origin value even despite it contained uninitialized bytes from other
> writes.
>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> Suggested-by: Marco Elver <elver@google.com>

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

> ---
>  mm/kmsan/kmsan_test.c | 53 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>
> diff --git a/mm/kmsan/kmsan_test.c b/mm/kmsan/kmsan_test.c
> index 6eb1e1a4d08f9..07d3a3a5a9c52 100644
> --- a/mm/kmsan/kmsan_test.c
> +++ b/mm/kmsan/kmsan_test.c
> @@ -486,6 +486,58 @@ static void test_memcpy_aligned_to_unaligned(struct kunit *test)
>         KUNIT_EXPECT_TRUE(test, report_matches(&expect));
>  }
>
> +/*
> + * Test case: ensure that origin slots do not accidentally get overwritten with
> + * zeroes during memcpy().
> + *
> + * Previously, when copying memory from an aligned buffer to an unaligned one,
> + * if there were zero origins corresponding to zero shadow values in the source
> + * buffer, they could have ended up being copied to nonzero shadow values in the
> + * destination buffer:
> + *
> + *  memcpy(0xffff888080a00000, 0xffff888080900002, 8)
> + *
> + *  src (0xffff888080900002): ..xx .... xx..
> + *  src origins:              o111 0000 o222
> + *  dst (0xffff888080a00000): xx.. ..xx
> + *  dst origins:              o111 0000
> + *                        (or 0000 o222)
> + *
> + * (here . stands for an initialized byte, and x for an uninitialized one.
> + *
> + * Ensure that this does not happen anymore, and for both destination bytes
> + * the origin is nonzero (i.e. KMSAN reports an error).
> + */
> +static void test_memcpy_initialized_gap(struct kunit *test)
> +{
> +       EXPECTATION_UNINIT_VALUE_FN(expect, "test_memcpy_initialized_gap");
> +       volatile char uninit_src[12];
> +       volatile char dst[8] = { 0 };
> +
> +       kunit_info(
> +               test,
> +               "unaligned 4-byte initialized value gets a nonzero origin after memcpy() - (2 UMR reports)\n");
> +
> +       uninit_src[0] = 42;
> +       uninit_src[1] = 42;
> +       uninit_src[4] = 42;
> +       uninit_src[5] = 42;
> +       uninit_src[6] = 42;
> +       uninit_src[7] = 42;
> +       uninit_src[10] = 42;
> +       uninit_src[11] = 42;
> +       memcpy_noinline((void *)&dst[0], (void *)&uninit_src[2], 8);
> +
> +       kmsan_check_memory((void *)&dst[0], 4);
> +       KUNIT_EXPECT_TRUE(test, report_matches(&expect));
> +       report_reset();
> +       kmsan_check_memory((void *)&dst[2], 4);
> +       KUNIT_EXPECT_FALSE(test, report_matches(&expect));
> +       report_reset();
> +       kmsan_check_memory((void *)&dst[4], 4);
> +       KUNIT_EXPECT_TRUE(test, report_matches(&expect));
> +}
> +
>  /* Generate test cases for memset16(), memset32(), memset64(). */
>  #define DEFINE_TEST_MEMSETXX(size)                                          \
>         static void test_memset##size(struct kunit *test)                   \
> @@ -579,6 +631,7 @@ static struct kunit_case kmsan_test_cases[] = {
>         KUNIT_CASE(test_init_memcpy),
>         KUNIT_CASE(test_memcpy_aligned_to_aligned),
>         KUNIT_CASE(test_memcpy_aligned_to_unaligned),
> +       KUNIT_CASE(test_memcpy_initialized_gap),
>         KUNIT_CASE(test_memset16),
>         KUNIT_CASE(test_memset32),
>         KUNIT_CASE(test_memset64),
> --
> 2.42.0.283.g2d96d420d3-goog
>