| 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: <20fb853c-7d79-4d26-9c8a-f6ce9367d424@linux.ibm.com>
Date: Tue, 5 Aug 2025 11:39:15 +0530
From: Donet Tom <donettom@...ux.ibm.com>
To: Wei Yang <richard.weiyang@...il.com>,
Aboorva Devarajan <aboorvad@...ux.ibm.com>
Cc: akpm@...ux-foundation.org, Liam.Howlett@...cle.com,
lorenzo.stoakes@...cle.com, shuah@...nel.org, pfalcato@...e.de,
david@...hat.com, ziy@...dia.com, baolin.wang@...ux.alibaba.com,
npache@...hat.com, ryan.roberts@....com, dev.jain@....com,
baohua@...nel.org, linux-mm@...ck.org, linux-kselftest@...r.kernel.org,
linux-kernel@...r.kernel.org, ritesh.list@...il.com
Subject: Re: [PATCH v3 3/7] selftest/mm: Fix ksm_funtional_test failures
On 8/4/25 2:41 PM, Wei Yang wrote:
> On Tue, Jul 29, 2025 at 11:03:59AM +0530, Aboorva Devarajan wrote:
>> From: Donet Tom <donettom@...ux.ibm.com>
>>
>> This patch fixed 2 issues.
>>
>> 1) After fork() in test_prctl_fork, the child process uses the file
>> descriptors from the parent process to read ksm_stat and
>> ksm_merging_pages. This results in incorrect values being read (parent
>> process ksm_stat and ksm_merging_pages will be read in child), causing
>> the test to fail.
>>
>> This patch calls init_global_file_handles() in the child process to
>> ensure that the current process's file descriptors are used to read
>> ksm_stat and ksm_merging_pages.
>>
>> 2) All tests currently call ksm_merge to trigger page merging.
>> To ensure the system remains in a consistent state for subsequent
>> tests, it is better to call ksm_unmerge during the test cleanup phase.
>>
>> In the test_prctl_fork test, after a fork(), reading ksm_merging_pages
>> in the child process returns a non-zero value because a previous test
>> performed a merge, and the child's memory state is inherited from the
>> parent.
>>
>> Although the child process calls ksm_unmerge, the ksm_merging_pages
>> counter in the parent is reset to zero, while the child's counter
>> remains unchanged. This discrepancy causes the test to fail.
>>
>> To avoid this issue, each test should call ksm_unmerge during cleanup
>> to ensure the counter is reset and the system is in a clean state for
>> subsequent tests.
>>
>> execv argument is an array of pointers to null-terminated strings.
>> In this patch we also added NULL in the execv argument.
>>
>> Fixes: 6c47de3be3a0 ("selftest/mm: ksm_functional_tests: extend test case for ksm fork/exec")
>> Co-developed-by: Aboorva Devarajan <aboorvad@...ux.ibm.com>
>> Signed-off-by: Aboorva Devarajan <aboorvad@...ux.ibm.com>
>> Signed-off-by: Donet Tom <donettom@...ux.ibm.com>
>> ---
>> tools/testing/selftests/mm/ksm_functional_tests.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/mm/ksm_functional_tests.c b/tools/testing/selftests/mm/ksm_functional_tests.c
>> index d8bd1911dfc0..996dc6645570 100644
>> --- a/tools/testing/selftests/mm/ksm_functional_tests.c
>> +++ b/tools/testing/selftests/mm/ksm_functional_tests.c
>> @@ -46,6 +46,8 @@ static int ksm_use_zero_pages_fd;
>> static int pagemap_fd;
>> static size_t pagesize;
>>
>> +static void init_global_file_handles(void);
>> +
>> static bool range_maps_duplicates(char *addr, unsigned long size)
>> {
>> unsigned long offs_a, offs_b, pfn_a, pfn_b;
>> @@ -274,6 +276,7 @@ static void test_unmerge(void)
>> ksft_test_result(!range_maps_duplicates(map, size),
>> "Pages were unmerged\n");
>> unmap:
>> + ksm_unmerge();
> In __mmap_and_merge_range(), we call ksm_unmerge(). Why this one not help?
>
> Not very familiar with ksm stuff. Would you mind giving more on how this fix
> the failure you see?
The issue I was facing here was test_prctl_fork was failing.
# [RUN] test_prctl_fork
# Still pages merged
#
This issue occurred because the previous test performed a merge, causing
the value of /proc/self/ksm_merging_pages to reflect the number of
deduplicated pages. After that, a fork() was called. Post-fork, the
child process
inherited the parent's ksm_merging_pages value.
Then, the child process invoked __mmap_and_merge_range(), which resulted
in unmerging the pages and resetting the value. However, since the
parent process
had performed the merge, its ksm_merging_pages value also got reset to 0.
Meanwhile, the child process had not performed any merge itself, so the
inherited
value remained unchanged. That’s why get_my_merging_page() in the child was
returning a non-zero value.
Initially, I fixed the issue by calling ksm_unmerge() before the fork(),
and that
resolved the problem. Later, I decided it would be cleaner to move the
ksm_unmerge() call to the test cleanup phase.
>
>> munmap(map, size);
>> }
>>
>> @@ -338,6 +341,7 @@ static void test_unmerge_zero_pages(void)
>> ksft_test_result(!range_maps_duplicates(map, size),
>> "KSM zero pages were unmerged\n");
>> unmap:
>> + ksm_unmerge();
>> munmap(map, size);
>> }
>>
>> @@ -366,6 +370,7 @@ static void test_unmerge_discarded(void)
>> ksft_test_result(!range_maps_duplicates(map, size),
>> "Pages were unmerged\n");
>> unmap:
>> + ksm_unmerge();
>> munmap(map, size);
>> }
>>
>> @@ -452,6 +457,7 @@ static void test_unmerge_uffd_wp(void)
>> close_uffd:
>> close(uffd);
>> unmap:
>> + ksm_unmerge();
>> munmap(map, size);
>> }
>> #endif
>> @@ -515,6 +521,7 @@ static int test_child_ksm(void)
>> else if (map == MAP_MERGE_SKIP)
>> return -3;
>>
>> + ksm_unmerge();
>> munmap(map, size);
>> return 0;
>> }
>> @@ -548,6 +555,7 @@ static void test_prctl_fork(void)
>>
>> child_pid = fork();
>> if (!child_pid) {
>> + init_global_file_handles();
> Would this leave fd in parent as orphan?
>
>> exit(test_child_ksm());
>> } else if (child_pid < 0) {
>> ksft_test_result_fail("fork() failed\n");
>> @@ -595,7 +603,7 @@ static void test_prctl_fork_exec(void)
>> return;
>> } else if (child_pid == 0) {
>> char *prg_name = "./ksm_functional_tests";
>> - char *argv_for_program[] = { prg_name, FORK_EXEC_CHILD_PRG_NAME };
>> + char *argv_for_program[] = { prg_name, FORK_EXEC_CHILD_PRG_NAME, NULL };
>>
>> execv(prg_name, argv_for_program);
>> return;
>> @@ -644,6 +652,7 @@ static void test_prctl_unmerge(void)
>> ksft_test_result(!range_maps_duplicates(map, size),
>> "Pages were unmerged\n");
>> unmap:
>> + ksm_unmerge();
>> munmap(map, size);
>> }
>>
>> @@ -677,6 +686,7 @@ static void test_prot_none(void)
>> ksft_test_result(!range_maps_duplicates(map, size),
>> "Pages were unmerged\n");
>> unmap:
>> + ksm_unmerge();
>> munmap(map, size);
>> }
>>
>> --
>> 2.47.1
>>
Powered by blists - more mailing lists