Skip to contentSkip to conversation

madvise07: Increase probability of testing a supported page type

PATCH New Richard Palethorpe 4 participants 7 messages Aug 28, 2017 – Sep 13, 2017

lore
Share & tools
lore

Richard Palethorpe

author
#0 lore raw

PATCH 1/1 madvise07: Increase probability of testing a supported page type

We were attempting to poison page types which do not support it (e.g. the zero page) due to our usage of mmap. Now we map some anonymous memory and write to it. Hopefully ensuring the underlying page is of a supported type.

Signed-off-by Richard Palethorpe <[email protected]>
---
 testcases/kernel/syscalls/madvise/madvise07.c | 76 ++++++++-------------------
 1 file changed, 23 insertions(+), 53 deletions(-)
testcases/kernel/syscalls/madvise/madvise07.c +23 -53
index 76adb2e06..ef9aa4a26 100644
@@ -16,21 +16,23 @@
  */
 
 /*
- * Check that accessing memory marked with MADV_HWPOISON results in SIGBUS.
+ * Check that accessing a page marked with MADV_HWPOISON results in SIGBUS.
  *
  * Test flow: create child process,
- *	      map memory,
- *	      mark memory with MADV_HWPOISON inside child process,
+ *	      map and write to memory,
+ *	      mark memory with MADV_HWPOISON,
  *	      access memory,
  *	      if SIGBUS is delivered to child the test passes else it fails
  *
- * When MAP_PRIVATE is set (without MAP_POPULATE) madvise() may error with
- * EBUSY on the first attempt and succeed on the second, but without poisoning
- * any memory. A private mapping is only populated with pages once it is
- * accessed and poisoning an unmapped VM range is essentially undefined
- * behaviour. However madvise() itself causes the address to be mapped to the
- * zero page. If/when the zero page can be poisoned then the test may pass
- * without any error. For now we just consider it a configuration failure.
+ * If the underlying page type of the memory we have mapped does not support
+ * poisoning then the test will fail. We try to map and write to the memory in
+ * such a way that by the time madvise is called the virtual memory address
+ * points to a supported page. However there may be some rare circumstances
+ * where the test produces the wrong result because we have somehow obtained
+ * an unsupported page. In such cases madvise will probably return success,
+ * but no SIGBUS will be produced.
+ *
+ * For more information see <linux source>/Documentation/vm/hwpoison.txt.
  */
 
 #include <stdlib.h>

@@ -40,54 +42,32 @@
 #include <unistd.h>
 #include <signal.h>
 #include <errno.h>
+#include <string.h>
 
 #include "tst_test.h"
 #include "lapi/mmap.h"
 
-static int maptypes[] = {
-	MAP_PRIVATE,
-	MAP_PRIVATE | MAP_POPULATE,
-	MAP_SHARED
-};
-
-static char *mapname(int maptype)
-{
-	switch (maptype) {
-	case MAP_PRIVATE: return "MAP_PRIVATE";
-	case MAP_PRIVATE | MAP_POPULATE: return "MAP_PRIVATE | MAP_POPULATE";
-	case MAP_SHARED: return "MAP_SHARED";
-	default:
-		tst_res(TWARN, "Unknown map type: %d", maptype);
-	}
-	return "Unknown";
-}
-
-static void run_child(int maptype)
+static void run_child(void)
 {
 	const size_t msize = getpagesize();
 	void *mem = NULL;
-	int first_attempt = 1;
 
 	tst_res(TINFO,
-		"mmap(..., MAP_ANONYMOUS | %s, ...)", mapname(maptype));
+		"mmap(0, %zu, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0)",
+		msize);
 	mem = SAFE_MMAP(NULL,
 			msize,
 			PROT_READ | PROT_WRITE,
-			MAP_ANONYMOUS | maptype,
+			MAP_ANONYMOUS | MAP_PRIVATE,
 			-1,
 			0);
+	memset(mem, 'L', msize);
 
-do_madvise:
 	tst_res(TINFO, "madvise(%p, %zu, MADV_HWPOISON)", mem, msize);
 	if (madvise(mem, msize, MADV_HWPOISON) == -1) {
 		if (errno == EINVAL) {
 			tst_res(TCONF | TERRNO,
 				"CONFIG_MEMORY_FAILURE probably not set in kconfig");
-		} else if (errno == EBUSY && maptype == MAP_PRIVATE) {
-			tst_res(TCONF,
-				"Madvise failed with EBUSY");
-			if (first_attempt--)
-				goto do_madvise;
 		} else {
 			tst_res(TFAIL | TERRNO, "Could not poison memory");
 		}

@@ -96,39 +76,29 @@ do_madvise:
 
 	*((char *)mem) = 'd';
 
-	if (maptype == MAP_PRIVATE) {
-		tst_res(TCONF,
-			"Zero page poisoning is probably not implemented");
-	} else {
-		tst_res(TFAIL,
-			"Did not receive SIGBUS after accessing %s memory marked with MADV_HWPOISON",
-			mapname(maptype));
-	}
+	tst_res(TFAIL, "Did not receive SIGBUS on accessing poisoned page");
 }
 
-static void run(unsigned int n)
+static void run(void)
 {
 	int status;
 	pid_t pid;
 
 	pid = SAFE_FORK();
 	if (pid == 0) {
-		run_child(maptypes[n]);
+		run_child();
 		exit(0);
 	}
 
 	SAFE_WAITPID(pid, &status, 0);
 	if (WIFSIGNALED(status) && WTERMSIG(status) == SIGBUS)
-		tst_res(TPASS,
-			"madvise(..., MADV_HWPOISON) on %s memory",
-			mapname(maptypes[n]));
+		tst_res(TPASS, "Received SIGBUS after accessing poisoned page");
 	else if (WIFEXITED(status) && WEXITSTATUS(status) == TBROK)
 		tst_res(TBROK, "Child exited abnormally");
 }
 
 static struct tst_test test = {
-	.test = run,
-	.tcnt = ARRAY_SIZE(maptypes),
+	.test_all = run,
 	.min_kver = "2.6.31",
 	.needs_root = 1,
 	.forks_child = 1
Signature · 1 line
2.14.1
#1 lore raw

PATCH 1/1 madvise07: Increase probability of testing a supported page type

----- Original Message -----

Quoted 11 lines

We were attempting to poison page types which do not support it (e.g. the zero
page) due to our usage of mmap. Now we map some anonymous memory and write to it. Hopefully ensuring the underlying page is of a supported type.
+ * If the underlying page type of the memory we have mapped does not support
+ * poisoning then the test will fail. We try to map and write to the memory in
+ * such a way that by the time madvise is called the virtual memory address
+ * points to a supported page. However there may be some rare circumstances
+ * where the test produces the wrong result because we have somehow obtained
+ * an unsupported page.

Hi,

Can you elaborate please? If we always do mmap+touch anonymous memory, how would we sometime end up with different page type?

I don't have objections to patch, but I'm thinking if we should go further if there's possibility the test still won't be reliable. We could relax the condition, for example by FAILing only if child dies unexpectedly (signal != SIGBUS).

Regards,
Jan

#2 lore raw

PATCH 1/1 madvise07: Increase probability of testing a supported page type

Hello Jan,

Quoted 13 lines · Jan Stancek

----- Original Message -----

Quoted 11 lines

We were attempting to poison page types which do not support it (e.g. the zero
page) due to our usage of mmap. Now we map some anonymous memory and write to it. Hopefully ensuring the underlying page is of a supported type.
+ * If the underlying page type of the memory we have mapped does not support
+ * poisoning then the test will fail. We try to map and write to the memory in
+ * such a way that by the time madvise is called the virtual memory address
+ * points to a supported page. However there may be some rare circumstances
+ * where the test produces the wrong result because we have somehow obtained
+ * an unsupported page.

Quoted 4 lines

Hi,

Can you elaborate please? If we always do mmap+touch anonymous memory, how would we sometime end up with different page type?

Possibly if the page is completely swapped out and it is not in the page cache. I am not sure if that is even possible, but my point was that we can't fully gaurantee that we are testing a valid page type because it is not something you can fully control from user land (unless there is some debug interface which allows it). I'm not sure if it is a helpful comment or not.

Quoted 5 lines

I don't have objections to patch, but I'm thinking if we should go further if there's possibility the test still won't be reliable. We could relax the condition, for example by FAILing only if child dies unexpectedly (signal != SIGBUS).

I think that if we see another failure and it is not a real bug, then we do that.

--
Thank you,
Richard.

#3 lore raw

PATCH 1/1 madvise07: Increase probability of testing a supported page type

In reply to Jan Stancek

Hi!

Quoted 12 lines
Quoted 11 lines

We were attempting to poison page types which do not support it (e.g. the zero
page) due to our usage of mmap. Now we map some anonymous memory and write to it. Hopefully ensuring the underlying page is of a supported type.
+ * If the underlying page type of the memory we have mapped does not support
+ * poisoning then the test will fail. We try to map and write to the memory in
+ * such a way that by the time madvise is called the virtual memory address
+ * points to a supported page. However there may be some rare circumstances
+ * where the test produces the wrong result because we have somehow obtained
+ * an unsupported page.

Quoted 4 lines

Hi,

Can you elaborate please? If we always do mmap+touch anonymous memory, how would we sometime end up with different page type?

Wouldn't this mean that the API is broken by desing?

One thing is that the call does not work on an unmapped page and fails to return an error. But if we cannot even guarantee that it will work if we make an effort to fault the page in advance its horribly broken by design.

Quoted 4 lines

I don't have objections to patch, but I'm thinking if we should go further if there's possibility the test still won't be reliable. We could relax the condition, for example by FAILing only if child dies unexpectedly (signal != SIGBUS).

What would that mean, producing TCONF on any error from the madvise() call? Looking at manual pages the only error we may get running MADVISE_HWPOISON as a root on a mapped page is the EINVAL we handle anyway.

Signature · 2 lines
Cyril Hrubis
[email protected]
#4 lore raw

PATCH 1/1 madvise07: Increase probability of testing a supported page type

----- Original Message -----

Quoted 24 lines

Hi!

Quoted 18 lines
Quoted 9 lines

We were attempting to poison page types which do not support it (e.g. the zero
page) due to our usage of mmap. Now we map some anonymous memory and write to
it. Hopefully ensuring the underlying page is of a supported type.
+ * If the underlying page type of the memory we have mapped does not support
+ * poisoning then the test will fail. We try to map and write to the memory

Quoted 8 lines

in
+ * such a way that by the time madvise is called the virtual memory address
+ * points to a supported page. However there may be some rare circumstances
+ * where the test produces the wrong result because we have somehow obtained
+ * an unsupported page.

Quoted 4 lines

Hi,

Can you elaborate please? If we always do mmap+touch anonymous memory, how would we sometime end up with different page type?

Quoted 6 lines

Wouldn't this mean that the API is broken by desing?

One thing is that the call does not work on an unmapped page and fails to return an error. But if we cannot even guarantee that it will work if we make an effort to fault the page in advance its horribly broken by design.

I think Richard was talking about scenario where something happens to page you just faulted in, e.g. it's swapped out for some reason. That should be quite unlikely.

Quoted 10 lines
Quoted 5 lines

I don't have objections to patch, but I'm thinking if we should go further if there's possibility the test still won't be reliable. We could relax the condition, for example by FAILing only if child dies unexpectedly (signal != SIGBUS).

What would that mean, producing TCONF on any error from the madvise() call? Looking at manual pages the only error we may get running MADVISE_HWPOISON as a root on a mapped page is the EINVAL we handle anyway.

I'd stay with just "mmap+touch anonymous memory" for now and see if that ever fails.

Regards,
Jan

#5 lore raw

PATCH 1/1 madvise07: Increase probability of testing a supported page type

Hi!

Quoted 10 lines
Quoted 6 lines

Wouldn't this mean that the API is broken by desing?

One thing is that the call does not work on an unmapped page and fails to return an error. But if we cannot even guarantee that it will work if we make an effort to fault the page in advance its horribly broken by design.

I think Richard was talking about scenario where something happens to page you just faulted in, e.g. it's swapped out for some reason. That should be quite unlikely.

We can always mlock() the page if that ever happens...

Quoted 5 lines
Quoted 5 lines
Quoted 4 lines

I don't have objections to patch, but I'm thinking if we should go further if there's possibility the test still won't be reliable. We could relax the condition, for example by FAILing only if child dies unexpectedly (signal != SIGBUS).

Quoted 7 lines
Quoted 4 lines

What would that mean, producing TCONF on any error from the madvise() call? Looking at manual pages the only error we may get running MADVISE_HWPOISON as a root on a mapped page is the EINVAL we handle anyway.

I'd stay with just "mmap+touch anonymous memory" for now and see if that ever fails.

I've applied the patch from Richard.

Signature · 2 lines
Cyril Hrubis
[email protected]