On Thu, Oct 17, 2024 at 1:18 AM Lorenzo Stoakes lorenzo.stoakes@oracle.com wrote:
NACK. Greg's bot got to it but...
As per Greg's bot, no signed-off-by line.
Sorry for confusion, I wasn't meant to send this as a PATCH, but reporting the issue. The diff was just sent as reference to repro the bug, and I forgot to remove PATCH from the title. I apologize for the confusion.
-Jeff
The subject should be something about adding a test.
You later say you are somehow dependning on things (what?) to make this work but it's broken.
Jeff - you're doing things that were raised on previous reviews as if we never said them. It's starting to get annoying now. Please try to listen to upstream.
On Thu, Oct 17, 2024 at 02:26:27AM +0000, jeffxu@chromium.org wrote:
From: Jeff Xu jeffxu@google.com
It appears there is a regression on the latest mm, when munmap sealed memory, it can cause unexpected VMA split. E.g. repro use this test.
This is an unacceptably short commit message. You've been told about this before.
tools/testing/selftests/mm/mseal_test.c | 76 +++++++++++++++++++++++++ 1 file changed, 76 insertions(+)
diff --git a/tools/testing/selftests/mm/mseal_test.c b/tools/testing/selftests/mm/mseal_test.c index fa74dbe4a684..0af33e13b606 100644 --- a/tools/testing/selftests/mm/mseal_test.c +++ b/tools/testing/selftests/mm/mseal_test.c @@ -1969,6 +1969,79 @@ static void test_madvise_filebacked_was_writable(bool seal) REPORT_TEST_PASS(); }
+static void test_munmap_free_multiple_ranges_with_split(bool seal) +{
void *ptr;
unsigned long page_size = getpagesize();
unsigned long size = 12 * page_size;
int ret;
int prot;
setup_single_address(size, &ptr);
FAIL_TEST_IF_FALSE(ptr != (void *)-1);
I'm not going to accept any test where you do:
FAIL_TEST_IF_FALSE(<negation>)
As that's totally unreadable. I asked you before for justification and you didn't provide it, no other tests appear to do this, I wrote thousands of lines of tests recently without doing this - stop it.
Also referene MAP_FAILED here please. You've been told before.
/* seal the middle 4 page */
if (seal) {
ret = sys_mseal(ptr + 4 * page_size, 4 * page_size);
FAIL_TEST_IF_FALSE(!ret);
size = get_vma_size(ptr, &prot);
FAIL_TEST_IF_FALSE(size == 4 * page_size);
FAIL_TEST_IF_FALSE(prot == 4);
Again, you've been told before, stop referencing numbers instead of PROT_... flags.
OK I'm stopping at this point, you _must listen to review_ Jeff.
size = get_vma_size(ptr + 4 * page_size, &prot);
FAIL_TEST_IF_FALSE(size == 4 * page_size);
FAIL_TEST_IF_FALSE(prot == 4);
size = get_vma_size(ptr + 8 * page_size, &prot);
FAIL_TEST_IF_FALSE(size == 4 * page_size);
FAIL_TEST_IF_FALSE(prot == 4);
}
/* munmap 4 pages from the third page */
ret = sys_munmap(ptr + 2 * page_size, 4 * page_size);
if (seal) {
FAIL_TEST_IF_FALSE(ret);
FAIL_TEST_IF_FALSE(errno == EPERM);
size = get_vma_size(ptr, &prot);
FAIL_TEST_IF_FALSE(size == 4 * page_size);
FAIL_TEST_IF_FALSE(prot == 4);
size = get_vma_size(ptr + 4 * page_size, &prot);
FAIL_TEST_IF_FALSE(size == 4 * page_size);
FAIL_TEST_IF_FALSE(prot == 4);
size = get_vma_size(ptr + 8 * page_size, &prot);
FAIL_TEST_IF_FALSE(size == 4 * page_size);
FAIL_TEST_IF_FALSE(prot == 4);
} else
FAIL_TEST_IF_FALSE(!ret);
/* munmap 4 pages from the sealed page */
ret = sys_munmap(ptr + 6 * page_size, 4 * page_size);
if (seal) {
FAIL_TEST_IF_FALSE(ret);
FAIL_TEST_IF_FALSE(errno == EPERM);
size = get_vma_size(ptr + 4 * page_size, &prot);
FAIL_TEST_IF_FALSE(size == 4 * page_size);
FAIL_TEST_IF_FALSE(prot == 4);
size = get_vma_size(ptr + 4 * page_size, &prot);
FAIL_TEST_IF_FALSE(size == 4 * page_size);
FAIL_TEST_IF_FALSE(prot == 4);
size = get_vma_size(ptr + 8 * page_size, &prot);
FAIL_TEST_IF_FALSE(size == 4 * page_size);
FAIL_TEST_IF_FALSE(prot == 4);
} else
FAIL_TEST_IF_FALSE(!ret);
REPORT_TEST_PASS();
+}
int main(int argc, char **argv) { bool test_seal = seal_support(); @@ -2099,5 +2172,8 @@ int main(int argc, char **argv) test_madvise_filebacked_was_writable(false); test_madvise_filebacked_was_writable(true);
test_munmap_free_multiple_ranges_with_split(false);
test_munmap_free_multiple_ranges_with_split(true);
ksft_finished();
}
2.47.0.rc1.288.g06298d1525-goog