[LTP] [PATCH 1/2] lib: allow checkpoints to be used by any uid

[LTP] [PATCH 1/2] lib: allow checkpoints to be used by any uid

[Jan Stancek]

Allow unprivileged child to synchronize with privileged parent.
Use chmod after open, because effective permissions set by open()
are modified by the process's umask: (mode & ~umask).

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 lib/tst_test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 26414e31ca77..233b370794a6 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -93,6 +93,7 @@ static void setup_ipc(void)
 	ipc_fd = open(shm_path, O_CREAT | O_EXCL | O_RDWR, 0600);
 	if (ipc_fd < 0)
 		tst_brk(TBROK | TERRNO, "open(%s)", shm_path);
+	SAFE_CHMOD(shm_path, 0666);
 
 	SAFE_FTRUNCATE(ipc_fd, size);
 
-- 
1.8.3.1

[LTP] [PATCH 2/2] security/dirtyc0w: synchronize parent and child

[Jan Stancek]

Add checkpoint to guarantee that parent doesn't send
signal to child before it sets up signal handler.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/security/dirtyc0w/dirtyc0w.c       | 14 +++++++++++++-
 testcases/kernel/security/dirtyc0w/dirtyc0w_child.c |  3 +++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/security/dirtyc0w/dirtyc0w.c b/testcases/kernel/security/dirtyc0w/dirtyc0w.c
index b8094ba977ab..5658f3233f59 100644
--- a/testcases/kernel/security/dirtyc0w/dirtyc0w.c
+++ b/testcases/kernel/security/dirtyc0w/dirtyc0w.c
@@ -36,6 +36,7 @@
  *   mm: remove gup_flags FOLL_WRITE games from __get_user_pages()
  */
 
+#include <errno.h>
 #include <sys/mman.h>
 #include <fcntl.h>
 #include <pthread.h>
@@ -49,6 +50,7 @@
 
 #define FNAME "test"
 #define STR   "this is not a test\n"
+#define TEST_APP "dirtyc0w_child"
 
 static uid_t nobody_uid;
 static gid_t nobody_gid;
@@ -67,6 +69,7 @@ void dirtyc0w_test(void)
 {
 	int i, fd, pid, fail = 0;
 	char c;
+	char *av[] = {TEST_APP, NULL};
 
 	/* Create file */
 	fd = SAFE_OPEN(FNAME, O_WRONLY|O_CREAT|O_EXCL, 0444);
@@ -78,9 +81,11 @@ void dirtyc0w_test(void)
 	if (!pid) {
 		SAFE_SETGID(nobody_gid);
 		SAFE_SETUID(nobody_uid);
-		SAFE_EXECLP("dirtyc0w_child", "dirtyc0w_child", NULL);
+		(void)execve(TEST_APP, av, tst_ipc_envp);
+		tst_brk(TBROK|TERRNO, "execve failed");
 	}
 
+	TST_CHECKPOINT_WAIT(0);
 	for (i = 0; i < 100; i++)  {
 		usleep(10000);
 
@@ -102,10 +107,17 @@ void dirtyc0w_test(void)
 		tst_res(TPASS, "Bug not reproduced");
 }
 
+static const char *const resource_files[] = {
+	TEST_APP,
+	NULL,
+};
+
 static struct tst_test test = {
 	.needs_tmpdir = 1,
+	.needs_checkpoints = 1,
 	.forks_child = 1,
 	.needs_root = 1,
 	.setup = setup,
 	.test_all = dirtyc0w_test,
+	.resource_files = resource_files,
 };
diff --git a/testcases/kernel/security/dirtyc0w/dirtyc0w_child.c b/testcases/kernel/security/dirtyc0w/dirtyc0w_child.c
index 49abdd6ba52e..bb93c62cb979 100644
--- a/testcases/kernel/security/dirtyc0w/dirtyc0w_child.c
+++ b/testcases/kernel/security/dirtyc0w/dirtyc0w_child.c
@@ -104,7 +104,10 @@ int main(void)
 	int fd;
 	struct stat st;
 
+	tst_reinit();
+
 	SAFE_SIGNAL(SIGUSR1, sighandler);
+	TST_CHECKPOINT_WAKE(0);
 
 	/* Open it read only and map */
 	fd = SAFE_OPEN(FNAME, O_RDONLY);
-- 
1.8.3.1

[LTP] [PATCH 2/2] security/dirtyc0w: synchronize parent and child

[Cyril Hrubis]

Hi!
> +static const char *const resource_files[] = {
> +	TEST_APP,
> +	NULL,
> +};

I do not get why we need the test binary to be listed as resource file,
the rest seems OK to me.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/2] security/dirtyc0w: synchronize parent and child

[Jan Stancek]

----- Original Message -----
> Hi!
> > +static const char *const resource_files[] = {
> > +	TEST_APP,
> > +	NULL,
> > +};
> 
> I do not get why we need the test binary to be listed as resource file,
> the rest seems OK to me.

Since the test is using tmpdir, we either need exec*p* variant and
set PATH or copy binary to tmpdir. I took inspiration from creat07.

Copy approach is slightly more convenient when running test from git tree:
# ./dirtyc0w

But I can replace it with execvpe() - user would just need to setup PATH
before running it from git tree:
# env PATH=`pwd`:$PATH ./dirtyc0w

Regards,
Jan

[LTP] [PATCH 2/2] security/dirtyc0w: synchronize parent and child

[Cyril Hrubis]

Hi!
> > > +static const char *const resource_files[] = {
> > > +	TEST_APP,
> > > +	NULL,
> > > +};
> > 
> > I do not get why we need the test binary to be listed as resource file,
> > the rest seems OK to me.
> 
> Since the test is using tmpdir, we either need exec*p* variant and
> set PATH or copy binary to tmpdir. I took inspiration from creat07.

We actually copy the binary for creat07 since are are trying to
overwrite it, which is supposed to fail, but I do not want to overwrite
the installed files in a case that the kernel is buggy.

> Copy approach is slightly more convenient when running test from git tree:
> # ./dirtyc0w
> 
> But I can replace it with execvpe() - user would just need to setup PATH
> before running it from git tree:
> # env PATH=`pwd`:$PATH ./dirtyc0w

You are supposed to do that, but maybe we can add the code to change
PATH to the test library itself, so it will include $PWD automatically,
which would fix this problem for quite a lot of test without a need to
modify these. Or add a modified exec function to the test library that
locates the binary first.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/2] security/dirtyc0w: synchronize parent and child

[Jan Stancek]

----- Original Message -----
> Hi!
> > > > +static const char *const resource_files[] = {
> > > > +	TEST_APP,
> > > > +	NULL,
> > > > +};
> > > 
> > > I do not get why we need the test binary to be listed as resource file,
> > > the rest seems OK to me.
> > 
> > Since the test is using tmpdir, we either need exec*p* variant and
> > set PATH or copy binary to tmpdir. I took inspiration from creat07.
> 
> We actually copy the binary for creat07 since are are trying to
> overwrite it, which is supposed to fail, but I do not want to overwrite
> the installed files in a case that the kernel is buggy.

I see, I'll send v2 which drops resource_files.

> 
> > Copy approach is slightly more convenient when running test from git tree:
> > # ./dirtyc0w
> > 
> > But I can replace it with execvpe() - user would just need to setup PATH
> > before running it from git tree:
> > # env PATH=`pwd`:$PATH ./dirtyc0w
> 
> You are supposed to do that, but maybe we can add the code to change
> PATH to the test library itself, so it will include $PWD automatically,
> which would fix this problem for quite a lot of test without a need to
> modify these. 

We'd also have to make sure all tests use exec*p* variant, right?

Regards,
Jan