| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <08ce8c61-e7a2-417a-a3cc-d20861753ae8@lucifer.local>
Date: Sun, 27 Aug 2023 10:57:59 +0100
From: Lorenzo Stoakes <lstoakes@...il.com>
To: "Joel Fernandes (Google)" <joel@...lfernandes.org>
Cc: linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
linux-mm@...ck.org, Shuah Khan <shuah@...nel.org>,
Vlastimil Babka <vbabka@...e.cz>,
Michal Hocko <mhocko@...e.com>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Kirill A Shutemov <kirill@...temov.name>,
"Liam R. Howlett" <liam.howlett@...cle.com>,
"Paul E. McKenney" <paulmck@...nel.org>,
Suren Baghdasaryan <surenb@...gle.com>,
Kalesh Singh <kaleshsingh@...gle.com>,
Lokesh Gidra <lokeshgidra@...gle.com>
Subject: Re: [PATCH v5 6/7] selftests: mm: Add a test for remapping within a
range
On Tue, Aug 22, 2023 at 01:54:59AM +0000, Joel Fernandes (Google) wrote:
> Move a block of memory within a memory range. Any alignment optimization
> on the source address may cause corruption. Verify using kselftest that
> it works. I have also verified with tracing that such optimization does
> not happen due to this check in can_align_down():
>
> if (!for_stack && vma->vm_start <= addr_masked)
> return false;
>
> Signed-off-by: Joel Fernandes (Google) <joel@...lfernandes.org>
> ---
> tools/testing/selftests/mm/mremap_test.c | 79 +++++++++++++++++++++++-
> 1 file changed, 78 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
> index d7366074e2a8..f45d1abedc9c 100644
> --- a/tools/testing/selftests/mm/mremap_test.c
> +++ b/tools/testing/selftests/mm/mremap_test.c
> @@ -23,6 +23,7 @@
> #define VALIDATION_NO_THRESHOLD 0 /* Verify the entire region */
>
> #define MIN(X, Y) ((X) < (Y) ? (X) : (Y))
> +#define SIZE_MB(m) ((size_t)m * (1024 * 1024))
Nit in this instance since you always just use an integer, but generally
for good practice's sake - shouldn't we place m in parens e.g. (size_t)(m)
to avoid broken macro expansion?
>
> struct config {
> unsigned long long src_alignment;
> @@ -226,6 +227,79 @@ static void mremap_expand_merge_offset(FILE *maps_fp, unsigned long page_size)
> ksft_test_result_fail("%s\n", test_name);
> }
>
> +/*
> + * Verify that an mremap within a range does not cause corruption
> + * of unrelated part of range.
> + *
> + * Consider the following range which is 2MB aligned and is
> + * a part of a larger 10MB range which is not shown. Each
> + * character is 256KB below making the source and destination
> + * 2MB each. The lower case letters are moved (s to d) and the
> + * upper case letters are not moved. The below test verifies
> + * that the upper case S letters are not corrupted by the
> + * adjacent mremap.
> + *
> + * |DDDDddddSSSSssss|
> + */
> +static void mremap_move_within_range(char pattern_seed)
> +{
> + char *test_name = "mremap mremap move within range";
> + void *src, *dest;
> + int i, success = 1;
> +
> + size_t size = SIZE_MB(20);
> + void *ptr = mmap(NULL, size, PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> + if (ptr == MAP_FAILED) {
> + perror("mmap");
> + success = 0;
> + goto out;
> + }
> + memset(ptr, 0, size);
> +
> + src = ptr + SIZE_MB(6);
> + src = (void *)((unsigned long)src & ~(SIZE_MB(2) - 1));
> +
It's nitty I know for a test (sorry!) but it'd be nice to place a bitwise
alignment trick like this into a helper function or macro if it isn't
otherwise avialable.
> + /* Set byte pattern for source block. */
> + srand(pattern_seed);
> + for (i = 0; i < SIZE_MB(2); i++) {
> + ((char *)src)[i] = (char) rand();
> + }
> +
> + dest = src - SIZE_MB(2);
> +
> + void *new_ptr = mremap(src + SIZE_MB(1), SIZE_MB(1), SIZE_MB(1),
> + MREMAP_MAYMOVE | MREMAP_FIXED, dest + SIZE_MB(1));
> + if (new_ptr == MAP_FAILED) {
> + perror("mremap");
> + success = 0;
> + goto out;
> + }
> +
> + /* Verify byte pattern after remapping */
> + srand(pattern_seed);
> + for (i = 0; i < SIZE_MB(1); i++) {
> + char c = (char) rand();
> +
> + if (((char *)src)[i] != c) {
> + ksft_print_msg("Data at src at %d got corrupted due to unrelated mremap\n",
> + i);
> + ksft_print_msg("Expected: %#x\t Got: %#x\n", c & 0xff,
> + ((char *) src)[i] & 0xff);
> + success = 0;
> + }
> + }
> +
> +out:
> + if (munmap(ptr, size) == -1)
> + perror("munmap");
> +
> + if (success)
> + ksft_test_result_pass("%s\n", test_name);
> + else
> + ksft_test_result_fail("%s\n", test_name);
> +}
> +
> /*
> * Returns the start address of the mapping on success, else returns
> * NULL on failure.
> @@ -491,6 +565,7 @@ int main(int argc, char **argv)
> unsigned int threshold_mb = VALIDATION_DEFAULT_THRESHOLD;
> unsigned int pattern_seed;
> int num_expand_tests = 2;
> + int num_misc_tests = 1;
> struct test test_cases[MAX_TEST] = {};
> struct test perf_test_cases[MAX_PERF_TEST];
> int page_size;
> @@ -572,7 +647,7 @@ int main(int argc, char **argv)
> (threshold_mb * _1MB >= _1GB);
>
> ksft_set_plan(ARRAY_SIZE(test_cases) + (run_perf_tests ?
> - ARRAY_SIZE(perf_test_cases) : 0) + num_expand_tests);
> + ARRAY_SIZE(perf_test_cases) : 0) + num_expand_tests + num_misc_tests);
>
> for (i = 0; i < ARRAY_SIZE(test_cases); i++)
> run_mremap_test_case(test_cases[i], &failures, threshold_mb,
> @@ -590,6 +665,8 @@ int main(int argc, char **argv)
>
> fclose(maps_fp);
>
> + mremap_move_within_range(pattern_seed);
> +
> if (run_perf_tests) {
> ksft_print_msg("\n%s\n",
> "mremap HAVE_MOVE_PMD/PUD optimization time comparison for 1GB region:");
> --
> 2.42.0.rc1.204.g551eb34607-goog
>
I drew a little diagram for myself and was thereby convinced this was a good test, therefore,
Reviewed-by: Lorenzo Stoakes <lstoakes@...il.com>
Powered by blists - more mailing lists