[LTP] [PATCH v2 1/2] clone08: convert to new LTP API

[LTP] [PATCH v2 1/2] clone08: convert to new LTP API

[Alexey Kodanev]

In addition:
* use tst_reap_children()

* remove usage of tst_tmpdir()

* change behaviour in CLONE_STOPPED test-case: set flag after
  clone() and before kill() to make sure the thread updated
  this flag after the signal is sent

* define clone functions with 'void *arg'

* exit forked children with _exit()

* add test-case for CLONE_CHILD_CLEARTID flag.
  Set CLONE_CHILD_CLEARTID in 'CLONE_THREAD' test-case to
  clear ctid when the child clone exits. Use futex to wait
  for ctid to be changed.

Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
---
v2: * move tst_res() to child function where possible
    * merge the patch with CLEARTID test-case
    * remove TCID field

 testcases/kernel/syscalls/clone/clone08.c |  264 +++++++++++------------------
 1 files changed, 97 insertions(+), 167 deletions(-)

diff --git a/testcases/kernel/syscalls/clone/clone08.c b/testcases/kernel/syscalls/clone/clone08.c
index 4c71db4..958755a 100644
--- a/testcases/kernel/syscalls/clone/clone08.c
+++ b/testcases/kernel/syscalls/clone/clone08.c
@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2017 Oracle and/or its affiliates. All Rights Reserved.
  * Copyright (c) 2013 Fujitsu Ltd.
  * Author: Zeng Linggang <zenglg.jy@cn.fujitsu.com>
  *
@@ -15,39 +16,36 @@
  */
 
 #define _GNU_SOURCE
+#include <stdlib.h>
+#include <stdio.h>
 #include <errno.h>
 #include <sched.h>
 #include <sys/wait.h>
-#include "test.h"
+#include <linux/futex.h>
+
+#include "tst_test.h"
 #include "clone_platform.h"
-#include "safe_macros.h"
 #include "linux_syscall_numbers.h"
 
-char *TCID = "clone08";
-
 static pid_t ptid, ctid, tgid;
 static void *child_stack;
 
-static void setup(void);
-static void cleanup(void);
-
 static void test_clone_parent(int t);
-static int child_clone_parent(void);
+static int child_clone_parent(void *);
 static pid_t parent_ppid;
 
 static void test_clone_tid(int t);
-static int child_clone_child_settid(void);
-static int child_clone_parent_settid(void);
+static int child_clone_child_settid(void *);
+static int child_clone_parent_settid(void *);
 
 #ifdef CLONE_STOPPED
 static void test_clone_stopped(int t);
-static int child_clone_stopped(void);
+static int child_clone_stopped(void *);
 static int stopped_flag;
 #endif
 
 static void test_clone_thread(int t);
-static int child_clone_thread(void);
-static int tst_result;
+static int child_clone_thread(void *);
 
 /*
  * Children cloned with CLONE_VM should avoid using any functions that
@@ -61,7 +59,7 @@ static struct test_case {
 	char *name;
 	int flags;
 	void (*testfunc)(int);
-	int (*do_child)();
+	int (*do_child)(void *);
 } test_cases[] = {
 	{"CLONE_PARENT", CLONE_PARENT | SIGCHLD,
 	 test_clone_parent, child_clone_parent},
@@ -73,154 +71,91 @@ static struct test_case {
 	{"CLONE_STOPPED", CLONE_STOPPED | CLONE_VM | SIGCHLD,
 	 test_clone_stopped, child_clone_stopped},
 #endif
-	{"CLONE_THREAD", CLONE_THREAD | CLONE_SIGHAND | CLONE_VM | SIGCHLD,
+	{"CLONE_THREAD", CLONE_THREAD | CLONE_SIGHAND | CLONE_VM |
+	 CLONE_CHILD_CLEARTID | SIGCHLD,
 	 test_clone_thread, child_clone_thread},
 };
 
-int TST_TOTAL = ARRAY_SIZE(test_cases);
-
-int main(int ac, char **av)
+static void do_test(unsigned int i)
 {
-	int i, lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		for (i = 0; i < TST_TOTAL; i++) {
-			tst_resm(TINFO, "running %s", test_cases[i].name);
-			test_cases[i].testfunc(i);
-		}
-	}
-	cleanup();
-	tst_exit();
+	tst_res(TINFO, "running %s", test_cases[i].name);
+	test_cases[i].testfunc(i);
 }
 
 static void setup(void)
 {
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
-
-	tst_tmpdir();
-
-	child_stack = SAFE_MALLOC(cleanup, CHILD_STACK_SIZE);
+	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
 }
 
 static void cleanup(void)
 {
 	free(child_stack);
-
-	tst_rmdir();
 }
 
-static long clone_child(const struct test_case *t, int use_tst)
+static long clone_child(const struct test_case *t)
 {
 	TEST(ltp_clone7(t->flags, t->do_child, NULL, CHILD_STACK_SIZE,
 		child_stack, &ptid, NULL, &ctid));
 
 	if (TEST_RETURN == -1 && TTERRNO == ENOSYS)
-		tst_brkm(TCONF, cleanup, "clone does not support 7 args");
-
-	if (TEST_RETURN == -1) {
-		if (use_tst) {
-			tst_brkm(TBROK | TTERRNO, cleanup, "%s clone() failed",
-				 t->name);
-		} else {
-			printf("%s clone() failed, errno: %d",
-			       t->name, TEST_ERRNO);
-			exit(1);
-		}
-	}
-	return TEST_RETURN;
-}
+		tst_brk(TCONF, "clone does not support 7 args");
 
-static int wait4child(pid_t child)
-{
-	int status;
+	if (TEST_RETURN == -1)
+		tst_brk(TBROK | TTERRNO, "%s clone() failed", t->name);
 
-	if (waitpid(child, &status, 0) == -1)
-		tst_resm(TBROK|TERRNO, "waitpid");
-	if (WIFEXITED(status))
-		return WEXITSTATUS(status);
-	else
-		return status;
+	return TEST_RETURN;
 }
 
 static void test_clone_parent(int t)
 {
-	int status;
 	pid_t child;
 
-	fflush(stdout);
-	child = FORK_OR_VFORK();
-	switch (child) {
-	case 0:
+	child = SAFE_FORK();
+	if (!child) {
 		parent_ppid = getppid();
-		clone_child(&test_cases[t], 0);
-		exit(0);
-	case -1:
-		tst_brkm(TBROK | TERRNO, NULL, "test_clone_parent fork");
-	default:
-		status = wait4child(child);
-		if (status == 0) {
-			/* wait for CLONE_PARENT child */
-			status = wait4child(-1);
-			if (status == 0) {
-				tst_resm(TPASS, "test %s", test_cases[t].name);
-			} else {
-				tst_resm(TFAIL, "test %s, status: %d",
-					 test_cases[t].name, status);
-			}
-		} else {
-			tst_resm(TFAIL, "test %s, status: %d",
-				 test_cases[t].name, status);
-		}
-	};
+		clone_child(&test_cases[t]);
+		_exit(0);
+	}
+	tst_reap_children();
 }
 
-static int child_clone_parent(void)
+static int child_clone_parent(void *arg LTP_ATTRIBUTE_UNUSED)
 {
-	if (parent_ppid == getppid())
-		exit(0);
-	printf("FAIL: getppid != parent_ppid (%d != %d)\n",
-	       parent_ppid, getppid());
-	exit(1);
+	if (parent_ppid == getppid()) {
+		tst_res(TPASS, "clone and forked child has the same parent");
+	} else {
+		tst_res(TFAIL, "getppid != parent_ppid (%d != %d)",
+			parent_ppid, getppid());
+	}
+	tst_syscall(__NR_exit, 0);
+	return 0;
 }
 
 static void test_clone_tid(int t)
 {
-	int status;
 	pid_t child;
 
-	child = clone_child(&test_cases[t], 1);
-	status = wait4child(child);
-	if (status == 0) {
-		tst_resm(TPASS, "test %s", test_cases[t].name);
-	} else {
-		tst_resm(TFAIL, "test %s, status: %d",
-			 test_cases[t].name, status);
-	}
+	child = clone_child(&test_cases[t]);
+	tst_reap_children();
 }
 
-static int child_clone_child_settid(void)
+static int child_clone_child_settid(void *arg LTP_ATTRIBUTE_UNUSED)
 {
-	if (ctid == ltp_syscall(__NR_getpid))
-		ltp_syscall(__NR_exit, 0);
-	printf("FAIL: ctid != getpid() (%d != %d)\n",
-	       ctid, getpid());
-	ltp_syscall(__NR_exit, 1);
+	if (ctid == tst_syscall(__NR_getpid))
+		tst_res(TPASS, "clone() correctly set ctid");
+	else
+		tst_res(TFAIL, "ctid != getpid() (%d != %d)", ctid, getpid());
+	tst_syscall(__NR_exit, 0);
 	return 0;
 }
 
-static int child_clone_parent_settid(void)
+static int child_clone_parent_settid(void *arg LTP_ATTRIBUTE_UNUSED)
 {
-	if (ptid == ltp_syscall(__NR_getpid))
-		ltp_syscall(__NR_exit, 0);
-	printf("FAIL: ptid != getpid() (%d != %d)\n",
-	       ptid, getpid());
-	ltp_syscall(__NR_exit, 1);
+	if (ptid == tst_syscall(__NR_getpid))
+		tst_res(TPASS, "clone() correctly set ptid");
+	else
+		tst_res(TFAIL, "ptid != getpid() (%d != %d)", ptid, getpid());
+	tst_syscall(__NR_exit, 0);
 	return 0;
 }
 
@@ -228,17 +163,14 @@ static int child_clone_parent_settid(void)
 static void test_clone_stopped(int t)
 {
 	int i;
-	int status;
-	int flag;
 	pid_t child;
 
 	if (tst_kvercmp(2, 6, 38) >= 0) {
-		tst_resm(TINFO, "CLONE_STOPPED skipped for kernels >= 2.6.38");
+		tst_res(TCONF, "CLONE_STOPPED skipped for kernels >= 2.6.38");
 		return;
 	}
 
-	stopped_flag = 0;
-	child = clone_child(&test_cases[t], 1);
+	child = clone_child(&test_cases[t]);
 
 	/* give the kernel scheduler chance to run the CLONE_STOPPED thread*/
 	for (i = 0; i < 100; i++) {
@@ -246,23 +178,22 @@ static void test_clone_stopped(int t)
 		usleep(1000);
 	}
 
-	flag = stopped_flag;
-	if (kill(child, SIGCONT) != 0)
-		tst_brkm(TBROK | TERRNO, cleanup, "kill SIGCONT failed");
+	stopped_flag = 0;
 
-	status = wait4child(child);
-	if (status == 0 && flag == 0) {
-		tst_resm(TPASS, "test %s", test_cases[t].name);
-	} else {
-		tst_resm(TFAIL, "test %s, status: %d, flag: %d",
-			 test_cases[t].name, status, flag);
-	}
+	SAFE_KILL(child, SIGCONT);
+
+	tst_reap_children();
+
+	if (stopped_flag == 1)
+		tst_res(TPASS, "clone stopped and resumed as expected");
+	else
+		tst_res(TFAIL, "clone not stopped, flag %d", stopped_flag);
 }
 
-static int child_clone_stopped(void)
+static int child_clone_stopped(void *arg LTP_ATTRIBUTE_UNUSED)
 {
 	stopped_flag = 1;
-	ltp_syscall(__NR_exit, 0);
+	tst_syscall(__NR_exit, 0);
 	return 0;
 }
 #endif
@@ -270,42 +201,41 @@ static int child_clone_stopped(void)
 static void test_clone_thread(int t)
 {
 	pid_t child;
-	int i, status;
-
-	fflush(stdout);
-	child = FORK_OR_VFORK();
-	switch (child) {
-	case 0:
-		tgid = ltp_syscall(__NR_getpid);
-		tst_result = -1;
-		clone_child(&test_cases[t], 0);
-
-		for (i = 0; i < 5000; i++) {
-			sched_yield();
-			usleep(1000);
-			if (tst_result != -1)
-				break;
-		}
-		ltp_syscall(__NR_exit, tst_result);
-	case -1:
-		tst_brkm(TBROK | TERRNO, NULL, "test_clone_thread fork");
-	default:
-		status = wait4child(child);
-		if (status == 0) {
-			tst_resm(TPASS, "test %s", test_cases[t].name);
-		} else {
-			tst_resm(TFAIL, "test %s, status: %d",
-				 test_cases[t].name, status);
-		}
-	};
+
+	child = SAFE_FORK();
+	if (!child) {
+		struct timespec timeout = { 5 /* sec */, 0 };
+
+		tgid = tst_syscall(__NR_getpid);
+		ctid = -1;
+
+		clone_child(&test_cases[t]);
+
+		if (syscall(SYS_futex, &ctid, FUTEX_WAIT, -1, &timeout))
+			tst_res(TFAIL | TERRNO, "futex failed");
+		else
+			tst_res(TPASS, "futex exit on ctid change");
+
+		_exit(0);
+	}
+
+	tst_reap_children();
 }
 
-static int child_clone_thread(void)
+static int child_clone_thread(void *arg LTP_ATTRIBUTE_UNUSED)
 {
-	if (tgid == ltp_syscall(__NR_getpid))
-		tst_result = TPASS;
+	if (tgid == tst_syscall(__NR_getpid))
+		tst_res(TPASS, "clone has the same thread id");
 	else
-		tst_result = TFAIL;
-	ltp_syscall(__NR_exit, 0);
+		tst_res(TFAIL, "clone's thread id not equal parent's id");
+	tst_syscall(__NR_exit, 0);
 	return 0;
 }
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(test_cases),
+	.test = do_test,
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1
+};
-- 
1.7.1

[LTP] [PATCH v2 2/2] clone09: add a test for CLONE_NEWNET flag

[Alexey Kodanev]

Verify that a new thread has new network namespace by
checking interface tag value parameter for lo interface
(/proc/sys/net/ipv4/conf/lo/tag). Change the value in
parent before clone() and then compare with the value saved
inside the thread.

Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
---
v2: remove TCID field

 runtest/syscalls                          |    1 +
 testcases/kernel/syscalls/.gitignore      |    1 +
 testcases/kernel/syscalls/clone/clone09.c |   99 +++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+), 0 deletions(-)
 create mode 100644 testcases/kernel/syscalls/clone/clone09.c

diff --git a/runtest/syscalls b/runtest/syscalls
index fe52272..2189bec 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -84,6 +84,7 @@ clone05 clone05
 clone06 clone06
 clone07 clone07
 clone08 clone08
+clone09 clone09
 
 close01 close01
 close02 close02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index c14c4e6..ea168a7 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -62,6 +62,7 @@
 /clone/clone06
 /clone/clone07
 /clone/clone08
+/clone/clone09
 /close/close01
 /close/close02
 /close/close08
diff --git a/testcases/kernel/syscalls/clone/clone09.c b/testcases/kernel/syscalls/clone/clone09.c
new file mode 100644
index 0000000..193d32c
--- /dev/null
+++ b/testcases/kernel/syscalls/clone/clone09.c
@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2017 Oracle and/or its affiliates. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define _GNU_SOURCE
+#include <sched.h>
+#include <stdlib.h>
+#include <errno.h>
+
+#include "tst_test.h"
+#include "clone_platform.h"
+#include "linux_syscall_numbers.h"
+
+static void *child_stack;
+static int sysctl_net = -1;
+static int sysctl_net_new = -1;
+static const char sysctl_path[] = "/proc/sys/net/ipv4/conf/lo/tag";
+static const char sysctl_path_def[] = "/proc/sys/net/ipv4/conf/default/tag";
+static int flags = CLONE_NEWNET | CLONE_VM | SIGCHLD;
+
+static void setup(void)
+{
+	child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
+}
+
+static void cleanup(void)
+{
+	if (sysctl_net != -1)
+		SAFE_FILE_PRINTF(sysctl_path, "%d", sysctl_net);
+
+	free(child_stack);
+}
+
+static int newnet(void *arg LTP_ATTRIBUTE_UNUSED)
+{
+	SAFE_FILE_SCANF(sysctl_path, "%d", &sysctl_net_new);
+	tst_syscall(__NR_exit, 0);
+	return 0;
+}
+
+static long clone_child(void)
+{
+	TEST(ltp_clone(flags, newnet, NULL, CHILD_STACK_SIZE, child_stack));
+
+	if (TEST_RETURN == -1 && TTERRNO == EINVAL)
+		tst_brk(TCONF, "CLONE_NEWNET not supported, CONFIG_NET_NS?");
+
+	if (TEST_RETURN == -1)
+		tst_brk(TBROK | TTERRNO, "clone(CLONE_NEWNET) failed");
+
+	return TEST_RETURN;
+}
+
+static void do_test(void)
+{
+	int def_val;
+
+	tst_res(TINFO, "create clone in a new netns with 'CLONE_NEWNET' flag");
+
+	SAFE_FILE_SCANF(sysctl_path, "%d", &sysctl_net);
+	SAFE_FILE_PRINTF(sysctl_path, "%d", sysctl_net + 1);
+
+	clone_child();
+	tst_reap_children();
+
+	if (sysctl_net_new == (sysctl_net + 1)) {
+		tst_res(TFAIL, "sysctl params equal: %s=%d",
+			sysctl_path, sysctl_net_new);
+	}
+
+	SAFE_FILE_SCANF(sysctl_path_def, "%d", &def_val);
+
+	if (sysctl_net_new != def_val) {
+		tst_res(TFAIL, "netns param init to non-default value %d",
+			sysctl_net_new);
+	}
+
+	tst_res(TPASS, "sysctl params differ in new netns");
+}
+
+static struct tst_test test = {
+	.test_all = do_test,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+};
-- 
1.7.1

[LTP] [PATCH v2 2/2] clone09: add a test for CLONE_NEWNET flag

[Cyril Hrubis]

Hi!
> +static void do_test(void)
> +{
> +	int def_val;
> +
> +	tst_res(TINFO, "create clone in a new netns with 'CLONE_NEWNET' flag");
> +
> +	SAFE_FILE_SCANF(sysctl_path, "%d", &sysctl_net);
> +	SAFE_FILE_PRINTF(sysctl_path, "%d", sysctl_net + 1);
> +
> +	clone_child();
> +	tst_reap_children();
> +
> +	if (sysctl_net_new == (sysctl_net + 1)) {
> +		tst_res(TFAIL, "sysctl params equal: %s=%d",
> +			sysctl_path, sysctl_net_new);
> +	}
> +
> +	SAFE_FILE_SCANF(sysctl_path_def, "%d", &def_val);
> +
> +	if (sysctl_net_new != def_val) {
> +		tst_res(TFAIL, "netns param init to non-default value %d",
> +			sysctl_net_new);
> +	}

We should restore the sysctl_path value here, in a case that we are
looping (the -i or -I options).

Otherwise it looks fine.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 2/2] clone09: add a test for CLONE_NEWNET flag

[Alexey Kodanev]

On 06/22/2017 06:09 PM, Cyril Hrubis wrote:
> Hi!
>> +static void do_test(void)
>> +{
>> +	int def_val;
>> +
>> +	tst_res(TINFO, "create clone in a new netns with 'CLONE_NEWNET' flag");
>> +
>> +	SAFE_FILE_SCANF(sysctl_path, "%d", &sysctl_net);
>> +	SAFE_FILE_PRINTF(sysctl_path, "%d", sysctl_net + 1);
>> +
>> +	clone_child();
>> +	tst_reap_children();
>> +
>> +	if (sysctl_net_new == (sysctl_net + 1)) {
>> +		tst_res(TFAIL, "sysctl params equal: %s=%d",
>> +			sysctl_path, sysctl_net_new);
>> +	}
>> +
>> +	SAFE_FILE_SCANF(sysctl_path_def, "%d", &def_val);
>> +
>> +	if (sysctl_net_new != def_val) {
>> +		tst_res(TFAIL, "netns param init to non-default value %d",
>> +			sysctl_net_new);
>> +	}
> We should restore the sysctl_path value here, in a case that we are
> looping (the -i or -I options).

Applied with restoring sysctl value in do_test().

Thanks,
Alexey

>
> Otherwise it looks fine.
>

[LTP] [PATCH v2 1/2] clone08: convert to new LTP API

[Cyril Hrubis]

Hi!
>  
> -	stopped_flag = 0;
> -	child = clone_child(&test_cases[t], 1);
> +	child = clone_child(&test_cases[t]);
>  
>  	/* give the kernel scheduler chance to run the CLONE_STOPPED thread*/
>  	for (i = 0; i < 100; i++) {
> @@ -246,23 +178,22 @@ static void test_clone_stopped(int t)
>  		usleep(1000);
>  	}

Can't we check the /proc/$pid/state here to assert that the child is in
'T' state here? We do have PROCESS_STATE_WAIT() for that.

Otherwise this looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 1/2] clone08: convert to new LTP API

[Alexey Kodanev]

On 06/22/2017 06:01 PM, Cyril Hrubis wrote:
> Hi!
>>  
>> -	stopped_flag = 0;
>> -	child = clone_child(&test_cases[t], 1);
>> +	child = clone_child(&test_cases[t]);
>>  
>>  	/* give the kernel scheduler chance to run the CLONE_STOPPED thread*/
>>  	for (i = 0; i < 100; i++) {
>> @@ -246,23 +178,22 @@ static void test_clone_stopped(int t)
>>  		usleep(1000);
>>  	}
> Can't we check the /proc/$pid/state here to assert that the child is in
> 'T' state here? We do have PROCESS_STATE_WAIT() for that.

Hi Cyril,

Sure, changed the loop to the macro and applied.

Thanks,
Alexey

> Otherwise this looks good.
>

[LTP] [PATCH 1/3] tst_safe_sysv_ipc: add shared memory related macros

[Xiao Yang]

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 include/tst_safe_sysv_ipc.h | 25 +++++++++++++++++++
 lib/tst_safe_sysv_ipc.c     | 58 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/include/tst_safe_sysv_ipc.h b/include/tst_safe_sysv_ipc.h
index cbdefb9..e01957f 100644
--- a/include/tst_safe_sysv_ipc.h
+++ b/include/tst_safe_sysv_ipc.h
@@ -18,6 +18,11 @@
 #ifndef TST_SAFE_SYSV_IPC_H__
 #define TST_SAFE_SYSV_IPC_H__
 
+#include <sys/types.h>
+#include <sys/ipc.h>
+#include <sys/msg.h>
+#include <sys/shm.h>
+
 int safe_msgget(const char *file, const int lineno, key_t key, int msgflg);
 #define SAFE_MSGGET(key, msgflg) \
 	safe_msgget(__FILE__, __LINE__, (key), (msgflg))
@@ -39,4 +44,24 @@ int safe_msgctl(const char *file, const int lineno, int msqid, int cmd,
 	(msqid) = ((cmd) == IPC_RMID ? -1 : (msqid)); \
 	} while (0)
 
+int safe_shmget(const char *file, const int lineno, key_t key, size_t size,
+		int shmflg);
+#define SAFE_SHMGET(key, size, shmflg) \
+	safe_shmget(__FILE__, __LINE__, (key), (size), (shmflg))
+
+void *safe_shmat(const char *file, const int lineno, int shmid,
+		const void *shmaddr, int shmflg);
+#define SAFE_SHMAT(shmid, shmaddr, shmflg) \
+	safe_shmat(__FILE__, __LINE__, (shmid), (shmaddr), (shmflg))
+
+int safe_shmdt(const char *file, const int lineno, const void *shmaddr);
+#define SAFE_SHMDT(shmaddr)	safe_shmdt(__FILE__, __LINE__, (shmaddr))
+
+int safe_shmctl(const char *file, const int lineno, int shmid, int cmd,
+		struct shmid_ds *buf);
+#define SAFE_SHMCTL(shmid, cmd, buf) do { \
+	safe_shmctl(__FILE__, __LINE__, (shmid), (cmd), (buf)); \
+	(shmid) = ((cmd) == IPC_RMID ? -1 : (shmid)); \
+	} while (0)
+
 #endif /* TST_SAFE_SYSV_IPC_H__ */
diff --git a/lib/tst_safe_sysv_ipc.c b/lib/tst_safe_sysv_ipc.c
index cb2b304..b2b132b 100644
--- a/lib/tst_safe_sysv_ipc.c
+++ b/lib/tst_safe_sysv_ipc.c
@@ -18,6 +18,7 @@
 #include <sys/types.h>
 #include <sys/ipc.h>
 #include <sys/msg.h>
+#include <sys/shm.h>
 #define TST_NO_DEFAULT_MAIN
 #include "tst_test.h"
 #include "tst_safe_sysv_ipc.h"
@@ -68,7 +69,7 @@ ssize_t safe_msgrcv(const char *file, const int lineno, int msqid, void *msgp,
 int safe_msgctl(const char *file, const int lineno, int msqid, int cmd,
 		struct msqid_ds *buf)
 {
-	int  rval;
+	int rval;
 
 	rval = msgctl(msqid, cmd, buf);
 	if (rval == -1) {
@@ -78,3 +79,58 @@ int safe_msgctl(const char *file, const int lineno, int msqid, int cmd,
 
 	return rval;
 }
+
+int safe_shmget(const char *file, const int lineno, key_t key, size_t size,
+		int shmflg)
+{
+	int rval;
+
+	rval = shmget(key, size, shmflg);
+	if (rval == -1) {
+		tst_brk(TBROK | TERRNO, "%s:%d: shmget(%i, %li, %x) failed",
+			file, lineno, (int)key, size, shmflg);
+	}
+
+	return rval;
+}
+
+void *safe_shmat(const char *file, const int lineno, int shmid,
+		const void *shmaddr, int shmflg)
+{
+	void *rval;
+
+	rval = shmat(shmid, shmaddr, shmflg);
+	if (rval == (void *)-1) {
+		tst_brk(TBROK | TERRNO, "%s:%d: shmat(%i, %p, %x) failed",
+			file, lineno, shmid, shmaddr, shmflg);
+	}
+
+	return rval;
+}
+
+int safe_shmdt(const char *file, const int lineno, const void *shmaddr)
+{
+	int rval;
+
+	rval = shmdt(shmaddr);
+	if (rval == -1) {
+		tst_brk(TBROK | TERRNO, "%s:%d: shmdt(%p) failed",
+			file, lineno, shmaddr);
+	}
+
+	return rval;
+}
+
+int safe_shmctl(const char *file, const int lineno, int shmid, int cmd,
+		struct shmid_ds *buf)
+{
+	int rval;
+
+	rval = shmctl(shmid, cmd, buf);
+	if (rval == -1) {
+		tst_brk(TBROK | TERRNO, "%s:%d: shmctl(%i, %i, %p) failed",
+			file, lineno, shmid, cmd, buf);
+	}
+
+	return rval;
+}
-- 
1.8.3.1

[LTP] [PATCH 3/3] syscalls/shmat03.c: add new regression test

[Xiao Yang]

This kernel bug has been fixed in:

commit 95e91b831f87ac8e1f8ed50c14d709089b4e01b8
Author: Davidlohr Bueso <dave@stgolabs.net>
Date:   Mon Feb 27 14:28:24 2017 -0800

	ipc/shm: Fix shmat mmap nil-page protection

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 runtest/ltplite                               |   1 +
 runtest/stress.part3                          |   1 +
 runtest/syscalls                              |   1 +
 runtest/syscalls-ipc                          |   1 +
 testcases/kernel/syscalls/.gitignore          |   1 +
 testcases/kernel/syscalls/ipc/shmat/shmat03.c | 133 ++++++++++++++++++++++++++
 6 files changed, 138 insertions(+)
 create mode 100644 testcases/kernel/syscalls/ipc/shmat/shmat03.c

diff --git a/runtest/ltplite b/runtest/ltplite
index 03bba7f..0c2e5be 100644
--- a/runtest/ltplite
+++ b/runtest/ltplite
@@ -828,6 +828,7 @@ setuid04 setuid04
 
 shmat01 shmat01
 shmat02 shmat02
+shmat03 shmat03
 
 shmctl01 shmctl01
 shmctl02 shmctl02
diff --git a/runtest/stress.part3 b/runtest/stress.part3
index b028a7f..bd84752 100644
--- a/runtest/stress.part3
+++ b/runtest/stress.part3
@@ -718,6 +718,7 @@ setuid04 setuid04
 
 shmat01 shmat01
 shmat02 shmat02
+shmat03 shmat03
 
 shmctl02 shmctl02
 shmctl03 shmctl03
diff --git a/runtest/syscalls b/runtest/syscalls
index 6276a90..5909456 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1162,6 +1162,7 @@ setxattr03 setxattr03
 
 shmat01 shmat01
 shmat02 shmat02
+shmat03 shmat03
 
 shmctl01 shmctl01
 shmctl02 shmctl02
diff --git a/runtest/syscalls-ipc b/runtest/syscalls-ipc
index de32c6b..91060b9 100644
--- a/runtest/syscalls-ipc
+++ b/runtest/syscalls-ipc
@@ -52,6 +52,7 @@ semop05 semop05
 
 shmat01 shmat01
 shmat02 shmat02
+shmat03 shmat03
 
 shmctl01 shmctl01
 shmctl02 shmctl02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 2b61fa2..d5985cd 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -439,6 +439,7 @@
 /ipc/semop/semop05
 /ipc/shmat/shmat01
 /ipc/shmat/shmat02
+/ipc/shmat/shmat03
 /ipc/shmctl/shmctl01
 /ipc/shmctl/shmctl02
 /ipc/shmctl/shmctl03
diff --git a/testcases/kernel/syscalls/ipc/shmat/shmat03.c b/testcases/kernel/syscalls/ipc/shmat/shmat03.c
new file mode 100644
index 0000000..1194f1b
--- /dev/null
+++ b/testcases/kernel/syscalls/ipc/shmat/shmat03.c
@@ -0,0 +1,133 @@
+/*
+ * Copyright (c) 2017 Fujitsu Ltd.
+ * Author: Xiao Yang <yangx.jy@cn.fujitsu.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * DESCRIPTION
+ * This is a regression test for nil-page protection mechanisms implemented
+ * in shmat(). Both root user and regular user shouldn't map nil-page in
+ * shmat() and was killed by SIGSEGV when writing data to nil-page.  However
+ * root user could succeed to map nil-page.
+ *
+ * This bug has been fixed in:
+ * commit 95e91b831f87ac8e1f8ed50c14d709089b4e01b8
+ * Author: Davidlohr Bueso <dave@stgolabs.net>
+ * Date:   Mon Feb 27 14:28:24 2017 -0800
+ *
+ *     ipc/shm: Fix shmat mmap nil-page protection
+ */
+
+#include <errno.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+#include <sys/types.h>
+#include <sys/ipc.h>
+#include <sys/shm.h>
+#include <pwd.h>
+
+#include "tst_test.h"
+#include "tst_safe_sysv_ipc.h"
+#include "libnewipc.h"
+
+#define LOCATION ((void *)1)
+
+static int shm_id = -1;
+
+static struct tcase {
+	int exp_usr;
+	char *des;
+} tcases[] = {
+	{0, "root user"},
+	{1, "regular user"}
+};
+
+static void verify_shmat(int exp_user)
+{
+	void *addr;
+	struct passwd *pw;
+
+	if (exp_user) {
+		pw = SAFE_GETPWNAM("nobody");
+		SAFE_SETUID(pw->pw_uid);
+	}
+
+	addr = shmat(shm_id, LOCATION, SHM_RND);
+	if (addr != (void *)-1)
+		tst_res(TINFO, "shmat() attached a nil-page unexpectedly");
+	else
+		tst_res(TINFO, "shmat() didn't attach a nil-page");
+
+	((char *)addr)[0] = 'A';
+	tst_res(TINFO, "shmat() wrote data to shmaddr:%p unexpectedly", addr);
+
+	SAFE_SHMDT(addr);
+
+	exit(0);
+}
+
+static void do_shmat(unsigned int n)
+{
+	pid_t pid;
+	int status;
+	struct tcase *tc = &tcases[n];
+
+	pid = SAFE_FORK();
+	if (!pid)
+		verify_shmat(tc->exp_usr);
+
+	SAFE_WAITPID(pid, &status, 0);
+
+	if (WIFEXITED(status)) {
+		tst_res(TFAIL, "%s mapped nil-page in shmat() unexpectedly",
+			tc->des);
+		return;
+	}
+
+	if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV) {
+		tst_res(TPASS, "%s didn't map nil-page in shmat(), and was "
+			"killed by %s as expected", tc->des,
+			tst_strsig(WTERMSIG(status)));
+	} else {
+		tst_res(TFAIL, "%s didn't map nil-page in shmat(), and was "
+			"killed by %s unexpectedly", tc->des,
+			tst_strsig(WTERMSIG(status)));
+	}
+}
+
+static void setup(void)
+{
+	key_t shm_key;
+
+	shm_key = GETIPCKEY();
+	shm_id = SAFE_SHMGET(shm_key, 4096, 0777 | IPC_CREAT);
+}
+
+static void cleanup(void)
+{
+	if (shm_id != -1)
+		SAFE_SHMCTL(shm_id, IPC_RMID, NULL);
+}
+
+static struct tst_test test = {
+	.tid = "shmat03",
+	.needs_root = 1,
+	.forks_child = 1,
+	.test = do_shmat,
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = setup,
+	.cleanup = cleanup
+};
-- 
1.8.3.1

[LTP] [PATCH 3/3] syscalls/shmat03.c: add new regression test

[Cyril Hrubis]

Hi!
> +	addr = shmat(shm_id, LOCATION, SHM_RND);
> +	if (addr != (void *)-1)
> +		tst_res(TINFO, "shmat() attached a nil-page unexpectedly");
> +	else
> +		tst_res(TINFO, "shmat() didn't attach a nil-page");
> +
> +	((char *)addr)[0] = 'A';


So if shmat() fails we try to write to (char*)-1 address, that does not
sound right. Why don't we exit the test with TPASS in that case and skip
the part that tries to write to invalid address?

Or at least dereference NULL here instead of the address returned from
shmat() since that is guaranteed to SEGFAULT.

> +	tst_res(TINFO, "shmat() wrote data to shmaddr:%p unexpectedly", addr);
> +
> +	SAFE_SHMDT(addr);
> +
> +	exit(0);
> +}
> +
> +static void do_shmat(unsigned int n)
> +{
> +	pid_t pid;
> +	int status;
> +	struct tcase *tc = &tcases[n];
> +
> +	pid = SAFE_FORK();
> +	if (!pid)
> +		verify_shmat(tc->exp_usr);
> +
> +	SAFE_WAITPID(pid, &status, 0);
> +
> +	if (WIFEXITED(status)) {
> +		tst_res(TFAIL, "%s mapped nil-page in shmat() unexpectedly",
> +			tc->des);
> +		return;
> +	}
> +
> +	if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV) {
> +		tst_res(TPASS, "%s didn't map nil-page in shmat(), and was "
> +			"killed by %s as expected", tc->des,
> +			tst_strsig(WTERMSIG(status)));
> +	} else {
> +		tst_res(TFAIL, "%s didn't map nil-page in shmat(), and was "
> +			"killed by %s unexpectedly", tc->des,
> +			tst_strsig(WTERMSIG(status)));
> +	}
> +}
> +
> +static void setup(void)
> +{
> +	key_t shm_key;
> +
> +	shm_key = GETIPCKEY();
> +	shm_id = SAFE_SHMGET(shm_key, 4096, 0777 | IPC_CREAT);
> +}
> +
> +static void cleanup(void)
> +{
> +	if (shm_id != -1)
> +		SAFE_SHMCTL(shm_id, IPC_RMID, NULL);
> +}
> +
> +static struct tst_test test = {
> +	.tid = "shmat03",
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.test = do_shmat,
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.setup = setup,
> +	.cleanup = cleanup
> +};
> -- 
> 1.8.3.1
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 3/3] syscalls/shmat03.c: add new regression test

[Xiao Yang]

On 2017/05/29 22:57, Cyril Hrubis wrote:
> Hi!
>> +	addr = shmat(shm_id, LOCATION, SHM_RND);
>> +	if (addr != (void *)-1)
>> +		tst_res(TINFO, "shmat() attached a nil-page unexpectedly");
>> +	else
>> +		tst_res(TINFO, "shmat() didn't attach a nil-page");
>> +
>> +	((char *)addr)[0] = 'A';
>
> So if shmat() fails we try to write to (char*)-1 address, that does not
> sound right. Why don't we exit the test with TPASS in that case and skip
> the part that tries to write to invalid address?
>
> Or at least dereference NULL here instead of the address returned from
> shmat() since that is guaranteed to SEGFAULT.
Hi Cyril

I failed to call shmat() as root and returned EACCES if selinux is 
Enforcing.
Do you know how to fix this problem?

Thanks,
Xiao Yang.
>> +	tst_res(TINFO, "shmat() wrote data to shmaddr:%p unexpectedly", addr);
>> +
>> +	SAFE_SHMDT(addr);
>> +
>> +	exit(0);
>> +}
>> +
>> +static void do_shmat(unsigned int n)
>> +{
>> +	pid_t pid;
>> +	int status;
>> +	struct tcase *tc =&tcases[n];
>> +
>> +	pid = SAFE_FORK();
>> +	if (!pid)
>> +		verify_shmat(tc->exp_usr);
>> +
>> +	SAFE_WAITPID(pid,&status, 0);
>> +
>> +	if (WIFEXITED(status)) {
>> +		tst_res(TFAIL, "%s mapped nil-page in shmat() unexpectedly",
>> +			tc->des);
>> +		return;
>> +	}
>> +
>> +	if (WIFSIGNALED(status)&&  WTERMSIG(status) == SIGSEGV) {
>> +		tst_res(TPASS, "%s didn't map nil-page in shmat(), and was "
>> +			"killed by %s as expected", tc->des,
>> +			tst_strsig(WTERMSIG(status)));
>> +	} else {
>> +		tst_res(TFAIL, "%s didn't map nil-page in shmat(), and was "
>> +			"killed by %s unexpectedly", tc->des,
>> +			tst_strsig(WTERMSIG(status)));
>> +	}
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	key_t shm_key;
>> +
>> +	shm_key = GETIPCKEY();
>> +	shm_id = SAFE_SHMGET(shm_key, 4096, 0777 | IPC_CREAT);
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	if (shm_id != -1)
>> +		SAFE_SHMCTL(shm_id, IPC_RMID, NULL);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.tid = "shmat03",
>> +	.needs_root = 1,
>> +	.forks_child = 1,
>> +	.test = do_shmat,
>> +	.tcnt = ARRAY_SIZE(tcases),
>> +	.setup = setup,
>> +	.cleanup = cleanup
>> +};
>> -- 
>> 1.8.3.1
>>
>>
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/3] syscalls/shmat03.c: add new regression test

[Xiao Yang]

On 2017/06/01 19:53, Xiao Yang wrote:
> On 2017/05/29 22:57, Cyril Hrubis wrote:
>> Hi!
>>> +    addr = shmat(shm_id, LOCATION, SHM_RND);
>>> +    if (addr != (void *)-1)
>>> +        tst_res(TINFO, "shmat() attached a nil-page unexpectedly");
>>> +    else
>>> +        tst_res(TINFO, "shmat() didn't attach a nil-page");
>>> +
>>> +    ((char *)addr)[0] = 'A';
>>
>> So if shmat() fails we try to write to (char*)-1 address, that does not
>> sound right. Why don't we exit the test with TPASS in that case and skip
>> the part that tries to write to invalid address?
>>
>> Or at least dereference NULL here instead of the address returned from
>> shmat() since that is guaranteed to SEGFAULT.
> Hi Cyril
>
> I failed to call shmat() as root and returned EACCES if selinux is 
> Enforcing.
> Do you know how to fix this problem?
Hi Cyril

Sorry, I have fixed this issue, I will send v3 patch soon.

Thanks,
Xiao Yang.
>
> Thanks,
> Xiao Yang.
>>> +    tst_res(TINFO, "shmat() wrote data to shmaddr:%p unexpectedly", 
>>> addr);
>>> +
>>> +    SAFE_SHMDT(addr);
>>> +
>>> +    exit(0);
>>> +}
>>> +
>>> +static void do_shmat(unsigned int n)
>>> +{
>>> +    pid_t pid;
>>> +    int status;
>>> +    struct tcase *tc =&tcases[n];
>>> +
>>> +    pid = SAFE_FORK();
>>> +    if (!pid)
>>> +        verify_shmat(tc->exp_usr);
>>> +
>>> +    SAFE_WAITPID(pid,&status, 0);
>>> +
>>> +    if (WIFEXITED(status)) {
>>> +        tst_res(TFAIL, "%s mapped nil-page in shmat() unexpectedly",
>>> +            tc->des);
>>> +        return;
>>> +    }
>>> +
>>> +    if (WIFSIGNALED(status)&&  WTERMSIG(status) == SIGSEGV) {
>>> +        tst_res(TPASS, "%s didn't map nil-page in shmat(), and was "
>>> +            "killed by %s as expected", tc->des,
>>> +            tst_strsig(WTERMSIG(status)));
>>> +    } else {
>>> +        tst_res(TFAIL, "%s didn't map nil-page in shmat(), and was "
>>> +            "killed by %s unexpectedly", tc->des,
>>> +            tst_strsig(WTERMSIG(status)));
>>> +    }
>>> +}
>>> +
>>> +static void setup(void)
>>> +{
>>> +    key_t shm_key;
>>> +
>>> +    shm_key = GETIPCKEY();
>>> +    shm_id = SAFE_SHMGET(shm_key, 4096, 0777 | IPC_CREAT);
>>> +}
>>> +
>>> +static void cleanup(void)
>>> +{
>>> +    if (shm_id != -1)
>>> +        SAFE_SHMCTL(shm_id, IPC_RMID, NULL);
>>> +}
>>> +
>>> +static struct tst_test test = {
>>> +    .tid = "shmat03",
>>> +    .needs_root = 1,
>>> +    .forks_child = 1,
>>> +    .test = do_shmat,
>>> +    .tcnt = ARRAY_SIZE(tcases),
>>> +    .setup = setup,
>>> +    .cleanup = cleanup
>>> +};
>>> -- 
>>> 1.8.3.1
>>>
>>>
>>>
>>>
>>> -- 
>>> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>
>
>

[LTP] [PATCH 3/3] syscalls/shmat03.c: add new regression test

[Xiao Yang]

On 2017/06/22 18:09, Xiao Yang wrote:
> On 2017/06/01 19:53, Xiao Yang wrote:
>> On 2017/05/29 22:57, Cyril Hrubis wrote:
>>> Hi!
>>>> +    addr = shmat(shm_id, LOCATION, SHM_RND);
>>>> +    if (addr != (void *)-1)
>>>> +        tst_res(TINFO, "shmat() attached a nil-page unexpectedly");
>>>> +    else
>>>> +        tst_res(TINFO, "shmat() didn't attach a nil-page");
>>>> +
>>>> +    ((char *)addr)[0] = 'A';
>>>
>>> So if shmat() fails we try to write to (char*)-1 address, that does not
>>> sound right. Why don't we exit the test with TPASS in that case and 
>>> skip
>>> the part that tries to write to invalid address?
>>>
>>> Or at least dereference NULL here instead of the address returned from
>>> shmat() since that is guaranteed to SEGFAULT.
>> Hi Cyril
>>
>> I failed to call shmat() as root and returned EACCES if selinux is 
>> Enforcing.
>> Do you know how to fix this problem?
> Hi Cyril
>
> Sorry, I have fixed this issue, I will send v3 patch soon.
Hi Cyril and jan

Sorry， i tried to fix this issue， but failed.  Could you help me to 
look into it? Thanks a lot! :-(
shmat（） only attached a nil-page as root when selinux is not Enforcing.

Thanks,
Xiao Yang.
>
> Thanks,
> Xiao Yang.
>>
>> Thanks,
>> Xiao Yang.
>>>> +    tst_res(TINFO, "shmat() wrote data to shmaddr:%p 
>>>> unexpectedly", addr);
>>>> +
>>>> +    SAFE_SHMDT(addr);
>>>> +
>>>> +    exit(0);
>>>> +}
>>>> +
>>>> +static void do_shmat(unsigned int n)
>>>> +{
>>>> +    pid_t pid;
>>>> +    int status;
>>>> +    struct tcase *tc =&tcases[n];
>>>> +
>>>> +    pid = SAFE_FORK();
>>>> +    if (!pid)
>>>> +        verify_shmat(tc->exp_usr);
>>>> +
>>>> +    SAFE_WAITPID(pid,&status, 0);
>>>> +
>>>> +    if (WIFEXITED(status)) {
>>>> +        tst_res(TFAIL, "%s mapped nil-page in shmat() unexpectedly",
>>>> +            tc->des);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    if (WIFSIGNALED(status)&&  WTERMSIG(status) == SIGSEGV) {
>>>> +        tst_res(TPASS, "%s didn't map nil-page in shmat(), and was "
>>>> +            "killed by %s as expected", tc->des,
>>>> +            tst_strsig(WTERMSIG(status)));
>>>> +    } else {
>>>> +        tst_res(TFAIL, "%s didn't map nil-page in shmat(), and was "
>>>> +            "killed by %s unexpectedly", tc->des,
>>>> +            tst_strsig(WTERMSIG(status)));
>>>> +    }
>>>> +}
>>>> +
>>>> +static void setup(void)
>>>> +{
>>>> +    key_t shm_key;
>>>> +
>>>> +    shm_key = GETIPCKEY();
>>>> +    shm_id = SAFE_SHMGET(shm_key, 4096, 0777 | IPC_CREAT);
>>>> +}
>>>> +
>>>> +static void cleanup(void)
>>>> +{
>>>> +    if (shm_id != -1)
>>>> +        SAFE_SHMCTL(shm_id, IPC_RMID, NULL);
>>>> +}
>>>> +
>>>> +static struct tst_test test = {
>>>> +    .tid = "shmat03",
>>>> +    .needs_root = 1,
>>>> +    .forks_child = 1,
>>>> +    .test = do_shmat,
>>>> +    .tcnt = ARRAY_SIZE(tcases),
>>>> +    .setup = setup,
>>>> +    .cleanup = cleanup
>>>> +};
>>>> -- 
>>>> 1.8.3.1
>>>>
>>>>
>>>>
>>>>
>>>> -- 
>>>> Mailing list info: https://lists.linux.it/listinfo/ltp
>>
>>
>>
>>
>
>
>
>

[LTP] [PATCH 3/3] syscalls/shmat03.c: add new regression test

[Jan Stancek]

----- Original Message -----
> From: "Xiao Yang" <yangx.jy@cn.fujitsu.com>
> To: "Cyril Hrubis" <chrubis@suse.cz>, "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Thursday, 22 June, 2017 1:08:03 PM
> Subject: Re: [LTP] [PATCH 3/3] syscalls/shmat03.c: add new regression test
> 
> On 2017/06/22 18:09, Xiao Yang wrote:
> > On 2017/06/01 19:53, Xiao Yang wrote:
> >> On 2017/05/29 22:57, Cyril Hrubis wrote:
> >>> Hi!
> >>>> +    addr = shmat(shm_id, LOCATION, SHM_RND);
> >>>> +    if (addr != (void *)-1)
> >>>> +        tst_res(TINFO, "shmat() attached a nil-page unexpectedly");
> >>>> +    else
> >>>> +        tst_res(TINFO, "shmat() didn't attach a nil-page");
> >>>> +
> >>>> +    ((char *)addr)[0] = 'A';
> >>>
> >>> So if shmat() fails we try to write to (char*)-1 address, that does not
> >>> sound right. Why don't we exit the test with TPASS in that case and
> >>> skip
> >>> the part that tries to write to invalid address?
> >>>
> >>> Or at least dereference NULL here instead of the address returned from
> >>> shmat() since that is guaranteed to SEGFAULT.
> >> Hi Cyril
> >>
> >> I failed to call shmat() as root and returned EACCES if selinux is
> >> Enforcing.
> >> Do you know how to fix this problem?
> > Hi Cyril
> >
> > Sorry, I have fixed this issue, I will send v3 patch soon.
> Hi Cyril and jan
> 
> Sorry， i tried to fix this issue， but failed.  Could you help me to
> look into it? Thanks a lot! :-(
> shmat（） only attached a nil-page as root when selinux is not Enforcing.

Hi,

as Richard mentioned already, this appears to be same test as his:
  http://lists.linux.it/pipermail/ltp/2017-May/004568.html

so I guess we can drop 3/3 from your set, and use Richard's
version, which is doing what Cyril suggested.

Regards,
Jan

[LTP] [PATCH v2 2/2] clone09: add a test for CLONE_NEWNET flag

[Cyril Hrubis]

Hi!
> as Richard mentioned already, this appears to be same test as his:
>   http://lists.linux.it/pipermail/ltp/2017-May/004568.html
> 
> so I guess we can drop 3/3 from your set, and use Richard's
> version, which is doing what Cyril suggested.

Sounds reasonable to me as well. We should probably include credits for
both you for the signed-off-by and copyright lines for that test
though.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 2/2] clone09: add a test for CLONE_NEWNET flag

[Richard Palethorpe]

Hi,

Cyril Hrubis writes:

> Hi!
>> as Richard mentioned already, this appears to be same test as his:
>>   http://lists.linux.it/pipermail/ltp/2017-May/004568.html
>> 
>> so I guess we can drop 3/3 from your set, and use Richard's
>> version, which is doing what Cyril suggested.
>
> Sounds reasonable to me as well. We should probably include credits for
> both you for the signed-off-by and copyright lines for that test
> though.

Yes, I agree.

-- 
Thank you,
Richard.