[LTP] [PATCH] syscalls/madvise: Handle zero page poisoning

[LTP] [PATCH] syscalls/madvise: Handle zero page poisoning

[Richard Palethorpe]

Treat failures for an un-populated MAP_PRIVATE mapping as configuration
failures because we are trying to do something which is not necessarily
expected to work. I have included some documentation for what is happening
with the zero page in this instance and what is likely to happen in the
future.

Also add a test case for MAP_PRIVATE with MAP_POPULATE, which is well defined,
and make the test slightly more verbose to help identify which variant is
running when there is an error.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/kernel/syscalls/madvise/madvise07.c | 38 ++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/testcases/kernel/syscalls/madvise/madvise07.c b/testcases/kernel/syscalls/madvise/madvise07.c
index 2f8c42efc..2fa553a07 100644
--- a/testcases/kernel/syscalls/madvise/madvise07.c
+++ b/testcases/kernel/syscalls/madvise/madvise07.c
@@ -23,9 +23,18 @@
  *	      mark memory with MADV_HWPOISON inside child process,
  *	      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.
  */
 
 #include <stdlib.h>
+#include <stdio.h>
 #include <sys/wait.h>
 #include <fcntl.h>
 #include <unistd.h>
@@ -35,18 +44,24 @@
 #include "tst_test.h"
 #include "lapi/mmap.h"
 
-#define MAPTYPE(m) m == MAP_SHARED ? "MAP_SHARED" : "MAP_PRIVATE"
+#define MAPTYPE(m) (m == MAP_SHARED ? "MAP_SHARED" :	\
+		    (m == MAP_PRIVATE ? "MAP_PRIVATE" :	\
+		     "MAP_PRIVATE | MAP_POPULATE"))
 
 static int maptypes[] = {
 	MAP_PRIVATE,
+	MAP_PRIVATE | MAP_POPULATE,
 	MAP_SHARED
 };
 
 static void run_child(int maptype)
 {
-	const size_t msize = 4096;
+	const size_t msize = getpagesize();
 	void *mem = NULL;
+	int first_attempt = 1;
 
+	tst_res(TINFO,
+		"mmap(..., MAP_ANONYMOUS | %s, ...)", MAPTYPE(maptype));
 	mem = SAFE_MMAP(NULL,
 			msize,
 			PROT_READ | PROT_WRITE,
@@ -54,22 +69,31 @@ static void run_child(int maptype)
 			-1,
 			0);
 
+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
+		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");
 		exit(0);
 	}
 
 	*((char *)mem) = 'd';
 
-	tst_res(TFAIL,
-		"Did not receive SIGBUS after accessing %s memory marked "
-		"with MADV_HWPOISON",
-		MAPTYPE(maptype));
+	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", MAPTYPE(maptype));
 }
 
 static void run(unsigned int n)
-- 
2.12.0

[LTP] [PATCH] syscalls/madvise: Handle zero page poisoning

[Cyril Hrubis]

On Wed, Mar 29, 2017 at 03:28:58PM +0200, Richard Palethorpe wrote:
> Treat failures for an un-populated MAP_PRIVATE mapping as configuration
> failures because we are trying to do something which is not necessarily
> expected to work. I have included some documentation for what is happening
> with the zero page in this instance and what is likely to happen in the
> future.
> 
> Also add a test case for MAP_PRIVATE with MAP_POPULATE, which is well defined,
> and make the test slightly more verbose to help identify which variant is
> running when there is an error.
> 
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> ---
>  testcases/kernel/syscalls/madvise/madvise07.c | 38 ++++++++++++++++++++++-----
>  1 file changed, 31 insertions(+), 7 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/madvise/madvise07.c b/testcases/kernel/syscalls/madvise/madvise07.c
> index 2f8c42efc..2fa553a07 100644
> --- a/testcases/kernel/syscalls/madvise/madvise07.c
> +++ b/testcases/kernel/syscalls/madvise/madvise07.c
> @@ -23,9 +23,18 @@
>   *	      mark memory with MADV_HWPOISON inside child process,
>   *	      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.
>   */
>  
>  #include <stdlib.h>
> +#include <stdio.h>
>  #include <sys/wait.h>
>  #include <fcntl.h>
>  #include <unistd.h>
> @@ -35,18 +44,24 @@
>  #include "tst_test.h"
>  #include "lapi/mmap.h"
>  
> -#define MAPTYPE(m) m == MAP_SHARED ? "MAP_SHARED" : "MAP_PRIVATE"
> +#define MAPTYPE(m) (m == MAP_SHARED ? "MAP_SHARED" :	\
> +		    (m == MAP_PRIVATE ? "MAP_PRIVATE" :	\
> +		     "MAP_PRIVATE | MAP_POPULATE"))

This is getting a bit out of hand, can we change this into a function
instead?

static const char *maptype(int mtype)
{
	switch (mtype) {
	case MAP_SHARED:
		return "MAP_SHARED";
	...
	default:
		return "???";
	}
}

>  static int maptypes[] = {
>  	MAP_PRIVATE,
> +	MAP_PRIVATE | MAP_POPULATE,
>  	MAP_SHARED
>  };
>  
>  static void run_child(int maptype)
>  {
> -	const size_t msize = 4096;
> +	const size_t msize = getpagesize();
>  	void *mem = NULL;
> +	int first_attempt = 1;
>  
> +	tst_res(TINFO,
> +		"mmap(..., MAP_ANONYMOUS | %s, ...)", MAPTYPE(maptype));
>  	mem = SAFE_MMAP(NULL,
>  			msize,
>  			PROT_READ | PROT_WRITE,
> @@ -54,22 +69,31 @@ static void run_child(int maptype)
>  			-1,
>  			0);
>  
> +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
> +		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");
>  		exit(0);
>  	}
>  
>  	*((char *)mem) = 'd';
>  
> -	tst_res(TFAIL,
> -		"Did not receive SIGBUS after accessing %s memory marked "
> -		"with MADV_HWPOISON",
> -		MAPTYPE(maptype));
> +	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", MAPTYPE(maptype));
>  }
>  
>  static void run(unsigned int n)
> -- 
> 2.12.0
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz