public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [PATCH 0/2] Coverage for MADV_GUARD_* features
@ 2024-12-03 13:36 Andrea Cervesato
  2024-12-03 13:36 ` [LTP] [PATCH 1/2] Fallback for MADV_GUARD_* definitions Andrea Cervesato
  2024-12-03 13:36 ` [LTP] [PATCH 2/2] Add madvise12 test Andrea Cervesato
  0 siblings, 2 replies; 8+ messages in thread
From: Andrea Cervesato @ 2024-12-03 13:36 UTC (permalink / raw)
  To: ltp

Kernel 6.13 has introduced a new feature for madvise() which causes
SIGSEGV when a memory advised with MADV_GUARD_INSTALL has been used.
To release memory, MADV_GUARD_REMOVE must be used.

This patch series adds coverage for both MADV_GUARD_INSTALL and
MADV_GUARD_REMOVE in madvise12.

More information at:
https://lore.kernel.org/linux-ia64/20240430100902.iwmeszr2jzv4wyo7@quack3/T/

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
Andrea Cervesato (2):
      Fallback for MADV_GUARD_* definitions
      Add madvise12 test

 include/lapi/mmap.h                           |   8 ++
 runtest/syscalls                              |   1 +
 testcases/kernel/syscalls/madvise/.gitignore  |   1 +
 testcases/kernel/syscalls/madvise/madvise12.c | 102 ++++++++++++++++++++++++++
 4 files changed, 112 insertions(+)
---
base-commit: 5c108380e95266cd747b81a6c051b5b10be29352
change-id: 20241203-madvise_guard_install-f49f54f38a41

Best regards,
-- 
Andrea Cervesato <andrea.cervesato@suse.com>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [LTP] [PATCH 1/2] Fallback for MADV_GUARD_* definitions
  2024-12-03 13:36 [LTP] [PATCH 0/2] Coverage for MADV_GUARD_* features Andrea Cervesato
@ 2024-12-03 13:36 ` Andrea Cervesato
  2024-12-11  0:18   ` Petr Vorel
  2024-12-03 13:36 ` [LTP] [PATCH 2/2] Add madvise12 test Andrea Cervesato
  1 sibling, 1 reply; 8+ messages in thread
From: Andrea Cervesato @ 2024-12-03 13:36 UTC (permalink / raw)
  To: ltp

From: Andrea Cervesato <andrea.cervesato@suse.com>

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 include/lapi/mmap.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/lapi/mmap.h b/include/lapi/mmap.h
index 7512e9f812a065e5485e042ef0eadd1a05e6b63c..ea9730586234eb0ab4d38ba7450a1e3a30dd49b9 100644
--- a/include/lapi/mmap.h
+++ b/include/lapi/mmap.h
@@ -38,6 +38,14 @@
 # define MADV_SOFT_OFFLINE 101
 #endif
 
+#ifndef MADV_GUARD_INSTALL
+# define MADV_GUARD_INSTALL 102
+#endif
+
+#ifndef MADV_GUARD_REMOVE
+# define MADV_GUARD_REMOVE 103
+#endif
+
 #ifndef MADV_MERGEABLE
 # define MADV_MERGEABLE   12
 #endif

-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [LTP] [PATCH 2/2] Add madvise12 test
  2024-12-03 13:36 [LTP] [PATCH 0/2] Coverage for MADV_GUARD_* features Andrea Cervesato
  2024-12-03 13:36 ` [LTP] [PATCH 1/2] Fallback for MADV_GUARD_* definitions Andrea Cervesato
@ 2024-12-03 13:36 ` Andrea Cervesato
  2024-12-11  0:30   ` Petr Vorel
  1 sibling, 1 reply; 8+ messages in thread
From: Andrea Cervesato @ 2024-12-03 13:36 UTC (permalink / raw)
  To: ltp

From: Andrea Cervesato <andrea.cervesato@suse.com>

Verify that MADV_GUARD_INSTALL is causing SIGSEGV when someone is
trying to access memory advised with it.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                              |   1 +
 testcases/kernel/syscalls/madvise/.gitignore  |   1 +
 testcases/kernel/syscalls/madvise/madvise12.c | 102 ++++++++++++++++++++++++++
 3 files changed, 104 insertions(+)

diff --git a/runtest/syscalls b/runtest/syscalls
index 5fd62617df1a116b1d94c57ff30f74693320a2ab..ded035ee82d0e97c67cc1e7c487b010634b2d1a0 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1000,6 +1000,7 @@ madvise08 madvise08
 madvise09 madvise09
 madvise10 madvise10
 madvise11 madvise11
+madvise12 madvise12
 
 newuname01 newuname01
 
diff --git a/testcases/kernel/syscalls/madvise/.gitignore b/testcases/kernel/syscalls/madvise/.gitignore
index 722ac3c34306bac414313f1ce36ca98d715cd04c..758e601a9c4e7682a925f16184d14f2357009bc2 100644
--- a/testcases/kernel/syscalls/madvise/.gitignore
+++ b/testcases/kernel/syscalls/madvise/.gitignore
@@ -8,3 +8,4 @@
 /madvise09
 /madvise10
 /madvise11
+/madvise12
diff --git a/testcases/kernel/syscalls/madvise/madvise12.c b/testcases/kernel/syscalls/madvise/madvise12.c
new file mode 100644
index 0000000000000000000000000000000000000000..2bdf843f016a7c9d175a31b76ae805d63c4cbc80
--- /dev/null
+++ b/testcases/kernel/syscalls/madvise/madvise12.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Verify that MADV_GUARD_INSTALL is causing SIGSEGV when someone is accessing
+ * memory advised with it.
+ *
+ * [Algorithm]
+ *
+ * - allocate a certain amount of memory
+ * - advise memory with MADV_GUARD_INSTALL
+ * - access to memory from within a child and verify it gets killed by SIGSEGV
+ * - release memory with MADV_GUARD_REMOVE
+ * - verify that memory has not been modified before child got killed
+ * - modify memory within a new child
+ * - verify that memory is accessable and child was not killed by SIGSEGV
+ */
+
+#include "tst_test.h"
+#include "lapi/mmap.h"
+
+#define MAP_SIZE (8 * TST_KB)
+
+static char *addr;
+
+static void run(void)
+{
+	pid_t pid;
+	int status;
+
+	memset(addr, 0, MAP_SIZE);
+
+	TST_EXP_PASS(madvise(addr, MAP_SIZE, MADV_GUARD_INSTALL));
+
+	pid = SAFE_FORK();
+	if (!pid) {
+		tst_res(TINFO, "Modifying memory content");
+
+		memset(addr, 'a', MAP_SIZE);
+		exit(0);
+	}
+
+	SAFE_WAITPID(pid, &status, 0);
+
+	if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV)
+		tst_res(TPASS, "Child ended by SIGSEGV as expected");
+	else
+		tst_res(TFAIL, "Child: %s", tst_strstatus(status));
+
+	TST_EXP_PASS(madvise(addr, MAP_SIZE, MADV_GUARD_REMOVE));
+
+	for (int i = 0; i < MAP_SIZE; i++) {
+		if (addr[i] == 'a') {
+			tst_res(TFAIL, "Memory content has been modified");
+			return;
+		}
+	}
+
+	tst_res(TPASS, "Memory content didn't change");
+
+	pid = SAFE_FORK();
+	if (!pid) {
+		tst_res(TINFO, "Modifying memory content");
+
+		memset(addr, 'b', MAP_SIZE);
+		exit(0);
+	}
+
+	SAFE_WAITPID(pid, &status, 0);
+
+	if (!WIFSIGNALED(status))
+		tst_res(TPASS, "Child ended without being signaled");
+	else
+		tst_res(TFAIL, "Child ended with %s", tst_strstatus(status));
+}
+
+static void setup(void)
+{
+	addr = SAFE_MMAP(NULL, MAP_SIZE,
+			PROT_READ | PROT_WRITE,
+			MAP_PRIVATE | MAP_ANONYMOUS,
+			-1, 0);
+}
+
+static void cleanup(void)
+{
+	if (addr)
+		SAFE_MUNMAP(addr, MAP_SIZE);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.forks_child = 1,
+	.min_kver = "6.13",
+};

-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [LTP] [PATCH 1/2] Fallback for MADV_GUARD_* definitions
  2024-12-03 13:36 ` [LTP] [PATCH 1/2] Fallback for MADV_GUARD_* definitions Andrea Cervesato
@ 2024-12-11  0:18   ` Petr Vorel
  0 siblings, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2024-12-11  0:18 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi Andrea,

obviously correct, thanks!
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [LTP] [PATCH 2/2] Add madvise12 test
  2024-12-03 13:36 ` [LTP] [PATCH 2/2] Add madvise12 test Andrea Cervesato
@ 2024-12-11  0:30   ` Petr Vorel
  2024-12-11  9:29     ` Lorenzo Stoakes via ltp
  0 siblings, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2024-12-11  0:30 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: Lorenzo Stoakes, ltp

Hi Andrea,

[ I dared to Cc Lorenzo, the author of the kernel implementation ]

> Verify that MADV_GUARD_INSTALL is causing SIGSEGV when someone is
> trying to access memory advised with it.

Test LGTM, thanks for covering a new kernel feature.

Closes: https://github.com/linux-test-project/ltp/issues/1210

> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  runtest/syscalls                              |   1 +
>  testcases/kernel/syscalls/madvise/.gitignore  |   1 +
>  testcases/kernel/syscalls/madvise/madvise12.c | 102 ++++++++++++++++++++++++++
>  3 files changed, 104 insertions(+)

> diff --git a/runtest/syscalls b/runtest/syscalls
> index 5fd62617df1a116b1d94c57ff30f74693320a2ab..ded035ee82d0e97c67cc1e7c487b010634b2d1a0 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1000,6 +1000,7 @@ madvise08 madvise08
>  madvise09 madvise09
>  madvise10 madvise10
>  madvise11 madvise11
> +madvise12 madvise12

>  newuname01 newuname01

> diff --git a/testcases/kernel/syscalls/madvise/.gitignore b/testcases/kernel/syscalls/madvise/.gitignore
> index 722ac3c34306bac414313f1ce36ca98d715cd04c..758e601a9c4e7682a925f16184d14f2357009bc2 100644
> --- a/testcases/kernel/syscalls/madvise/.gitignore
> +++ b/testcases/kernel/syscalls/madvise/.gitignore
> @@ -8,3 +8,4 @@
>  /madvise09
>  /madvise10
>  /madvise11
> +/madvise12
> diff --git a/testcases/kernel/syscalls/madvise/madvise12.c b/testcases/kernel/syscalls/madvise/madvise12.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..2bdf843f016a7c9d175a31b76ae805d63c4cbc80
> --- /dev/null
> +++ b/testcases/kernel/syscalls/madvise/madvise12.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2024 Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Verify that MADV_GUARD_INSTALL is causing SIGSEGV when someone is accessing
> + * memory advised with it.

Maybe add:
	* This is a test for feature implemented in
	* 662df3e5c376 ("mm: madvise: implement lightweight guard page mechanism")

The rest LGTM.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> + *
> + * [Algorithm]
> + *
> + * - allocate a certain amount of memory
> + * - advise memory with MADV_GUARD_INSTALL
> + * - access to memory from within a child and verify it gets killed by SIGSEGV
> + * - release memory with MADV_GUARD_REMOVE
> + * - verify that memory has not been modified before child got killed
> + * - modify memory within a new child
> + * - verify that memory is accessable and child was not killed by SIGSEGV
> + */
> +
> +#include "tst_test.h"
> +#include "lapi/mmap.h"
> +
> +#define MAP_SIZE (8 * TST_KB)
> +
> +static char *addr;
> +
> +static void run(void)
> +{
> +	pid_t pid;
> +	int status;
> +
> +	memset(addr, 0, MAP_SIZE);
> +
> +	TST_EXP_PASS(madvise(addr, MAP_SIZE, MADV_GUARD_INSTALL));
> +
> +	pid = SAFE_FORK();
> +	if (!pid) {
> +		tst_res(TINFO, "Modifying memory content");
> +
> +		memset(addr, 'a', MAP_SIZE);
> +		exit(0);
> +	}
> +
> +	SAFE_WAITPID(pid, &status, 0);
> +
> +	if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV)
> +		tst_res(TPASS, "Child ended by SIGSEGV as expected");
> +	else
> +		tst_res(TFAIL, "Child: %s", tst_strstatus(status));
> +
> +	TST_EXP_PASS(madvise(addr, MAP_SIZE, MADV_GUARD_REMOVE));
> +
> +	for (int i = 0; i < MAP_SIZE; i++) {
> +		if (addr[i] == 'a') {
> +			tst_res(TFAIL, "Memory content has been modified");
> +			return;
> +		}
> +	}
> +
> +	tst_res(TPASS, "Memory content didn't change");
> +
> +	pid = SAFE_FORK();
> +	if (!pid) {
> +		tst_res(TINFO, "Modifying memory content");
> +
> +		memset(addr, 'b', MAP_SIZE);
> +		exit(0);
> +	}
> +
> +	SAFE_WAITPID(pid, &status, 0);
> +
> +	if (!WIFSIGNALED(status))
> +		tst_res(TPASS, "Child ended without being signaled");
> +	else
> +		tst_res(TFAIL, "Child ended with %s", tst_strstatus(status));
> +}
> +
> +static void setup(void)
> +{
> +	addr = SAFE_MMAP(NULL, MAP_SIZE,
> +			PROT_READ | PROT_WRITE,
> +			MAP_PRIVATE | MAP_ANONYMOUS,
> +			-1, 0);
> +}
> +
> +static void cleanup(void)
> +{
> +	if (addr)
> +		SAFE_MUNMAP(addr, MAP_SIZE);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.min_kver = "6.13",
> +};

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [LTP] [PATCH 2/2] Add madvise12 test
  2024-12-11  0:30   ` Petr Vorel
@ 2024-12-11  9:29     ` Lorenzo Stoakes via ltp
  2024-12-11  9:52       ` Andrea Cervesato via ltp
  2024-12-11  9:55       ` Petr Vorel
  0 siblings, 2 replies; 8+ messages in thread
From: Lorenzo Stoakes via ltp @ 2024-12-11  9:29 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

On Wed, Dec 11, 2024 at 01:30:48AM +0100, Petr Vorel wrote:
> Hi Andrea,
>
> [ I dared to Cc Lorenzo, the author of the kernel implementation ]

I'm not that scary am I? :)))

Feel free to cc- me on anything relating to this even if obviously in the
LTP project happy to be included! :)

>
> > Verify that MADV_GUARD_INSTALL is causing SIGSEGV when someone is
> > trying to access memory advised with it.
>
> Test LGTM, thanks for covering a new kernel feature.

All good, looks fine to me, though might be worth expanding over time, we
have some self tests in the kernel for this, see
tools/testing/selftests/mm/guard-pages.c.

But it's nice to have a basic LTP regression test to assert the fundamental
thing is working as it should, and also nice that you implement it from
your perspective rather than mine, where I am obviously rather influenced
by implementation details.

Also note I submitted man pages for the change, you can pull it from [0]
and view it via:

$ man --manpath=. 2 madvise

[0]:git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git

Hopefully these will get distributed around soon!

Thanks for doing this for my feature, much appreciated overall!

Cheers, Lorenzo

>
> Closes: https://github.com/linux-test-project/ltp/issues/1210
>
> > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> > ---
> >  runtest/syscalls                              |   1 +
> >  testcases/kernel/syscalls/madvise/.gitignore  |   1 +
> >  testcases/kernel/syscalls/madvise/madvise12.c | 102 ++++++++++++++++++++++++++
> >  3 files changed, 104 insertions(+)
>
> > diff --git a/runtest/syscalls b/runtest/syscalls
> > index 5fd62617df1a116b1d94c57ff30f74693320a2ab..ded035ee82d0e97c67cc1e7c487b010634b2d1a0 100644
> > --- a/runtest/syscalls
> > +++ b/runtest/syscalls
> > @@ -1000,6 +1000,7 @@ madvise08 madvise08
> >  madvise09 madvise09
> >  madvise10 madvise10
> >  madvise11 madvise11
> > +madvise12 madvise12
>
> >  newuname01 newuname01
>
> > diff --git a/testcases/kernel/syscalls/madvise/.gitignore b/testcases/kernel/syscalls/madvise/.gitignore
> > index 722ac3c34306bac414313f1ce36ca98d715cd04c..758e601a9c4e7682a925f16184d14f2357009bc2 100644
> > --- a/testcases/kernel/syscalls/madvise/.gitignore
> > +++ b/testcases/kernel/syscalls/madvise/.gitignore
> > @@ -8,3 +8,4 @@
> >  /madvise09
> >  /madvise10
> >  /madvise11
> > +/madvise12
> > diff --git a/testcases/kernel/syscalls/madvise/madvise12.c b/testcases/kernel/syscalls/madvise/madvise12.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..2bdf843f016a7c9d175a31b76ae805d63c4cbc80
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/madvise/madvise12.c
> > @@ -0,0 +1,102 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2024 Andrea Cervesato <andrea.cervesato@suse.com>
> > + */
> > +
> > +/*\
> > + * [Description]
> > + *
> > + * Verify that MADV_GUARD_INSTALL is causing SIGSEGV when someone is accessing
> > + * memory advised with it.
>
> Maybe add:
> 	* This is a test for feature implemented in
> 	* 662df3e5c376 ("mm: madvise: implement lightweight guard page mechanism")
>
> The rest LGTM.
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Kind regards,
> Petr
>
> > + *
> > + * [Algorithm]
> > + *
> > + * - allocate a certain amount of memory
> > + * - advise memory with MADV_GUARD_INSTALL
> > + * - access to memory from within a child and verify it gets killed by SIGSEGV
> > + * - release memory with MADV_GUARD_REMOVE
> > + * - verify that memory has not been modified before child got killed
> > + * - modify memory within a new child
> > + * - verify that memory is accessable and child was not killed by SIGSEGV
> > + */
> > +
> > +#include "tst_test.h"
> > +#include "lapi/mmap.h"
> > +
> > +#define MAP_SIZE (8 * TST_KB)
> > +
> > +static char *addr;
> > +
> > +static void run(void)
> > +{
> > +	pid_t pid;
> > +	int status;
> > +
> > +	memset(addr, 0, MAP_SIZE);
> > +
> > +	TST_EXP_PASS(madvise(addr, MAP_SIZE, MADV_GUARD_INSTALL));
> > +
> > +	pid = SAFE_FORK();
> > +	if (!pid) {
> > +		tst_res(TINFO, "Modifying memory content");
> > +
> > +		memset(addr, 'a', MAP_SIZE);
> > +		exit(0);
> > +	}
> > +
> > +	SAFE_WAITPID(pid, &status, 0);
> > +
> > +	if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV)
> > +		tst_res(TPASS, "Child ended by SIGSEGV as expected");
> > +	else
> > +		tst_res(TFAIL, "Child: %s", tst_strstatus(status));
> > +
> > +	TST_EXP_PASS(madvise(addr, MAP_SIZE, MADV_GUARD_REMOVE));
> > +
> > +	for (int i = 0; i < MAP_SIZE; i++) {
> > +		if (addr[i] == 'a') {
> > +			tst_res(TFAIL, "Memory content has been modified");
> > +			return;
> > +		}
> > +	}
> > +
> > +	tst_res(TPASS, "Memory content didn't change");
> > +
> > +	pid = SAFE_FORK();
> > +	if (!pid) {
> > +		tst_res(TINFO, "Modifying memory content");
> > +
> > +		memset(addr, 'b', MAP_SIZE);
> > +		exit(0);
> > +	}
> > +
> > +	SAFE_WAITPID(pid, &status, 0);
> > +
> > +	if (!WIFSIGNALED(status))
> > +		tst_res(TPASS, "Child ended without being signaled");
> > +	else
> > +		tst_res(TFAIL, "Child ended with %s", tst_strstatus(status));
> > +}
> > +
> > +static void setup(void)
> > +{
> > +	addr = SAFE_MMAP(NULL, MAP_SIZE,
> > +			PROT_READ | PROT_WRITE,
> > +			MAP_PRIVATE | MAP_ANONYMOUS,
> > +			-1, 0);
> > +}
> > +
> > +static void cleanup(void)
> > +{
> > +	if (addr)
> > +		SAFE_MUNMAP(addr, MAP_SIZE);
> > +}
> > +
> > +static struct tst_test test = {
> > +	.test_all = run,
> > +	.setup = setup,
> > +	.cleanup = cleanup,
> > +	.needs_root = 1,
> > +	.forks_child = 1,
> > +	.min_kver = "6.13",
> > +};
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [LTP] [PATCH 2/2] Add madvise12 test
  2024-12-11  9:29     ` Lorenzo Stoakes via ltp
@ 2024-12-11  9:52       ` Andrea Cervesato via ltp
  2024-12-11  9:55       ` Petr Vorel
  1 sibling, 0 replies; 8+ messages in thread
From: Andrea Cervesato via ltp @ 2024-12-11  9:52 UTC (permalink / raw)
  To: ltp

Thanks for the review. Pushed with suggested changes.

Andrea

On 12/11/24 10:29, Lorenzo Stoakes via ltp wrote:
> On Wed, Dec 11, 2024 at 01:30:48AM +0100, Petr Vorel wrote:
>> Hi Andrea,
>>
>> [ I dared to Cc Lorenzo, the author of the kernel implementation ]
> I'm not that scary am I? :)))
>
> Feel free to cc- me on anything relating to this even if obviously in the
> LTP project happy to be included! :)
>
>>> Verify that MADV_GUARD_INSTALL is causing SIGSEGV when someone is
>>> trying to access memory advised with it.
>> Test LGTM, thanks for covering a new kernel feature.
> All good, looks fine to me, though might be worth expanding over time, we
> have some self tests in the kernel for this, see
> tools/testing/selftests/mm/guard-pages.c.
>
> But it's nice to have a basic LTP regression test to assert the fundamental
> thing is working as it should, and also nice that you implement it from
> your perspective rather than mine, where I am obviously rather influenced
> by implementation details.
>
> Also note I submitted man pages for the change, you can pull it from [0]
> and view it via:
>
> $ man --manpath=. 2 madvise
>
> [0]:git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
>
> Hopefully these will get distributed around soon!
>
> Thanks for doing this for my feature, much appreciated overall!
>
> Cheers, Lorenzo
>
>> Closes: https://github.com/linux-test-project/ltp/issues/1210
>>
>>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>>> ---
>>>   runtest/syscalls                              |   1 +
>>>   testcases/kernel/syscalls/madvise/.gitignore  |   1 +
>>>   testcases/kernel/syscalls/madvise/madvise12.c | 102 ++++++++++++++++++++++++++
>>>   3 files changed, 104 insertions(+)
>>> diff --git a/runtest/syscalls b/runtest/syscalls
>>> index 5fd62617df1a116b1d94c57ff30f74693320a2ab..ded035ee82d0e97c67cc1e7c487b010634b2d1a0 100644
>>> --- a/runtest/syscalls
>>> +++ b/runtest/syscalls
>>> @@ -1000,6 +1000,7 @@ madvise08 madvise08
>>>   madvise09 madvise09
>>>   madvise10 madvise10
>>>   madvise11 madvise11
>>> +madvise12 madvise12
>>>   newuname01 newuname01
>>> diff --git a/testcases/kernel/syscalls/madvise/.gitignore b/testcases/kernel/syscalls/madvise/.gitignore
>>> index 722ac3c34306bac414313f1ce36ca98d715cd04c..758e601a9c4e7682a925f16184d14f2357009bc2 100644
>>> --- a/testcases/kernel/syscalls/madvise/.gitignore
>>> +++ b/testcases/kernel/syscalls/madvise/.gitignore
>>> @@ -8,3 +8,4 @@
>>>   /madvise09
>>>   /madvise10
>>>   /madvise11
>>> +/madvise12
>>> diff --git a/testcases/kernel/syscalls/madvise/madvise12.c b/testcases/kernel/syscalls/madvise/madvise12.c
>>> new file mode 100644
>>> index 0000000000000000000000000000000000000000..2bdf843f016a7c9d175a31b76ae805d63c4cbc80
>>> --- /dev/null
>>> +++ b/testcases/kernel/syscalls/madvise/madvise12.c
>>> @@ -0,0 +1,102 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>> +/*
>>> + * Copyright (c) 2024 Andrea Cervesato <andrea.cervesato@suse.com>
>>> + */
>>> +
>>> +/*\
>>> + * [Description]
>>> + *
>>> + * Verify that MADV_GUARD_INSTALL is causing SIGSEGV when someone is accessing
>>> + * memory advised with it.
>> Maybe add:
>> 	* This is a test for feature implemented in
>> 	* 662df3e5c376 ("mm: madvise: implement lightweight guard page mechanism")
>>
>> The rest LGTM.
>>
>> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>>
>> Kind regards,
>> Petr
>>
>>> + *
>>> + * [Algorithm]
>>> + *
>>> + * - allocate a certain amount of memory
>>> + * - advise memory with MADV_GUARD_INSTALL
>>> + * - access to memory from within a child and verify it gets killed by SIGSEGV
>>> + * - release memory with MADV_GUARD_REMOVE
>>> + * - verify that memory has not been modified before child got killed
>>> + * - modify memory within a new child
>>> + * - verify that memory is accessable and child was not killed by SIGSEGV
>>> + */
>>> +
>>> +#include "tst_test.h"
>>> +#include "lapi/mmap.h"
>>> +
>>> +#define MAP_SIZE (8 * TST_KB)
>>> +
>>> +static char *addr;
>>> +
>>> +static void run(void)
>>> +{
>>> +	pid_t pid;
>>> +	int status;
>>> +
>>> +	memset(addr, 0, MAP_SIZE);
>>> +
>>> +	TST_EXP_PASS(madvise(addr, MAP_SIZE, MADV_GUARD_INSTALL));
>>> +
>>> +	pid = SAFE_FORK();
>>> +	if (!pid) {
>>> +		tst_res(TINFO, "Modifying memory content");
>>> +
>>> +		memset(addr, 'a', MAP_SIZE);
>>> +		exit(0);
>>> +	}
>>> +
>>> +	SAFE_WAITPID(pid, &status, 0);
>>> +
>>> +	if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV)
>>> +		tst_res(TPASS, "Child ended by SIGSEGV as expected");
>>> +	else
>>> +		tst_res(TFAIL, "Child: %s", tst_strstatus(status));
>>> +
>>> +	TST_EXP_PASS(madvise(addr, MAP_SIZE, MADV_GUARD_REMOVE));
>>> +
>>> +	for (int i = 0; i < MAP_SIZE; i++) {
>>> +		if (addr[i] == 'a') {
>>> +			tst_res(TFAIL, "Memory content has been modified");
>>> +			return;
>>> +		}
>>> +	}
>>> +
>>> +	tst_res(TPASS, "Memory content didn't change");
>>> +
>>> +	pid = SAFE_FORK();
>>> +	if (!pid) {
>>> +		tst_res(TINFO, "Modifying memory content");
>>> +
>>> +		memset(addr, 'b', MAP_SIZE);
>>> +		exit(0);
>>> +	}
>>> +
>>> +	SAFE_WAITPID(pid, &status, 0);
>>> +
>>> +	if (!WIFSIGNALED(status))
>>> +		tst_res(TPASS, "Child ended without being signaled");
>>> +	else
>>> +		tst_res(TFAIL, "Child ended with %s", tst_strstatus(status));
>>> +}
>>> +
>>> +static void setup(void)
>>> +{
>>> +	addr = SAFE_MMAP(NULL, MAP_SIZE,
>>> +			PROT_READ | PROT_WRITE,
>>> +			MAP_PRIVATE | MAP_ANONYMOUS,
>>> +			-1, 0);
>>> +}
>>> +
>>> +static void cleanup(void)
>>> +{
>>> +	if (addr)
>>> +		SAFE_MUNMAP(addr, MAP_SIZE);
>>> +}
>>> +
>>> +static struct tst_test test = {
>>> +	.test_all = run,
>>> +	.setup = setup,
>>> +	.cleanup = cleanup,
>>> +	.needs_root = 1,
>>> +	.forks_child = 1,
>>> +	.min_kver = "6.13",
>>> +};
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [LTP] [PATCH 2/2] Add madvise12 test
  2024-12-11  9:29     ` Lorenzo Stoakes via ltp
  2024-12-11  9:52       ` Andrea Cervesato via ltp
@ 2024-12-11  9:55       ` Petr Vorel
  1 sibling, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2024-12-11  9:55 UTC (permalink / raw)
  To: Lorenzo Stoakes; +Cc: ltp

Hi Lorenzo, Andrea,

> On Wed, Dec 11, 2024 at 01:30:48AM +0100, Petr Vorel wrote:
> > Hi Andrea,

> > [ I dared to Cc Lorenzo, the author of the kernel implementation ]

> I'm not that scary am I? :)))

> Feel free to cc- me on anything relating to this even if obviously in the
> LTP project happy to be included! :)

Thank you, we really appreciate that!

> > > Verify that MADV_GUARD_INSTALL is causing SIGSEGV when someone is
> > > trying to access memory advised with it.

> > Test LGTM, thanks for covering a new kernel feature.

> All good, looks fine to me, though might be worth expanding over time, we
> have some self tests in the kernel for this, see
> tools/testing/selftests/mm/guard-pages.c.

Great! Noted at the ticket.

> But it's nice to have a basic LTP regression test to assert the fundamental
> thing is working as it should, and also nice that you implement it from
> your perspective rather than mine, where I am obviously rather influenced
> by implementation details.

> Also note I submitted man pages for the change, you can pull it from [0]
> and view it via:

> $ man --manpath=. 2 madvise

> [0]:git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git

+1

Kind regards,
Petr

> Hopefully these will get distributed around soon!

> Thanks for doing this for my feature, much appreciated overall!

> Cheers, Lorenzo
...

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-12-11  9:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-03 13:36 [LTP] [PATCH 0/2] Coverage for MADV_GUARD_* features Andrea Cervesato
2024-12-03 13:36 ` [LTP] [PATCH 1/2] Fallback for MADV_GUARD_* definitions Andrea Cervesato
2024-12-11  0:18   ` Petr Vorel
2024-12-03 13:36 ` [LTP] [PATCH 2/2] Add madvise12 test Andrea Cervesato
2024-12-11  0:30   ` Petr Vorel
2024-12-11  9:29     ` Lorenzo Stoakes via ltp
2024-12-11  9:52       ` Andrea Cervesato via ltp
2024-12-11  9:55       ` Petr Vorel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox