[LTP] [PATCH v1 0/2] ptrace: Refactor

[LTP] [PATCH v1 0/2] ptrace: Refactor

[Wei Gao via ltp]

Wei Gao (2):
  ptrace05: Refactor the test using new LTP API
  ptrace06: Refactor the test using new LTP API

 testcases/kernel/syscalls/ptrace/ptrace05.c | 147 +++-------
 testcases/kernel/syscalls/ptrace/ptrace06.c | 306 +++++++++++---------
 2 files changed, 214 insertions(+), 239 deletions(-)

-- 
2.35.3


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

[LTP] [PATCH v1 1/2] ptrace05: Refactor the test using new LTP API

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/ptrace/ptrace05.c | 147 ++++++--------------
 1 file changed, 39 insertions(+), 108 deletions(-)

diff --git a/testcases/kernel/syscalls/ptrace/ptrace05.c b/testcases/kernel/syscalls/ptrace/ptrace05.c
index 54cfa4d7b..4904b959c 100644
--- a/testcases/kernel/syscalls/ptrace/ptrace05.c
+++ b/testcases/kernel/syscalls/ptrace/ptrace05.c
@@ -1,122 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
- ******************************************************************************
- *
- *   ptrace05 - an app which ptraces itself as per arbitrarily specified signals,
- *   over a user specified range.
- *
- *   Copyright (C) 2009, Ngie Cooper
- *
- *   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.
+ * Copyright (C) 2009, Ngie Cooper
+ * Copyright (c) 2023 Wei Gao <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
  *
- *   You should have received a copy of the GNU General Public License along
- *   with this program; if not, write to the Free Software Foundation, Inc.,
- *   51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *  ptrace05 - an app which ptraces itself as per arbitrarily specified signals
  *
- ******************************************************************************
  */
 
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <signal.h>
-#include <errno.h>
-#include <libgen.h>
-#include <math.h>
 #include <stdlib.h>
-#include <stdio.h>
-#include <string.h>
-#include <unistd.h>
-
 #include <config.h>
 #include "ptrace.h"
 
-#include "test.h"
 #include "lapi/signal.h"
+#include "tst_test.h"
 
-char *TCID = "ptrace05";
-int TST_TOTAL = 0;
-
-int usage(const char *);
-
-int usage(const char *argv0)
-{
-	fprintf(stderr, "usage: %s [start-signum] [end-signum]\n", argv0);
-	return 1;
-}
-
-int main(int argc, char **argv)
+static void run(void)
 {
 
-	int end_signum = -1;
-	int signum;
-	int start_signum = -1;
+	int end_signum = SIGRTMAX;
+	int signum = 0;
+	int start_signum = 0;
 	int status;
 
 	pid_t child;
 
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	if (start_signum == -1) {
-		start_signum = 0;
-	}
-	if (end_signum == -1) {
-		end_signum = SIGRTMAX;
-	}
-
 	for (signum = start_signum; signum <= end_signum; signum++) {
 
-		if (signum >= __SIGRTMIN && signum < SIGRTMIN)
-			continue;
-
-		switch (child = fork()) {
+		switch (child = SAFE_FORK()) {
 		case -1:
-			tst_brkm(TBROK | TERRNO, NULL, "fork() failed");
+			tst_brk(TBROK | TERRNO, "fork() failed");
 		case 0:
 
-			if (ptrace(PTRACE_TRACEME, 0, NULL, NULL) != -1) {
-				tst_resm(TINFO, "[child] Sending kill(.., %d)",
-					 signum);
-				if (kill(getpid(), signum) < 0) {
-					tst_resm(TINFO | TERRNO,
-						 "[child] kill(.., %d) failed.",
-						 signum);
-				}
+			TEST(ptrace(PTRACE_TRACEME, 0, NULL, NULL));
+			if (TST_RET != -1) {
+				tst_res(TINFO, "[child] Sending kill(.., %d)",
+						signum);
+				SAFE_KILL(getpid(), signum);
 			} else {
-
-				/*
-				 * This won't increment the TST_COUNT var.
-				 * properly, but it'll show up as a failure
-				 * nonetheless.
-				 */
-				tst_resm(TFAIL | TERRNO,
+				tst_brk(TFAIL | TERRNO,
 					 "Failed to ptrace(PTRACE_TRACEME, ...) "
 					 "properly");
-
 			}
-			/* Shouldn't get here if signum == 0. */
-			exit((signum == 0 ? 0 : 2));
+
+			exit(0);
 			break;
 
 		default:
 
-			waitpid(child, &status, 0);
+			SAFE_WAITPID(child, &status, 0);
 
 			switch (signum) {
 			case 0:
 				if (WIFEXITED(status)
 				    && WEXITSTATUS(status) == 0) {
-					tst_resm(TPASS,
+					tst_res(TPASS,
 						 "kill(.., 0) exited "
 						 "with 0, as expected.");
 				} else {
-					tst_resm(TFAIL,
+					tst_brk(TFAIL | TERRNO,
 						 "kill(.., 0) didn't exit "
 						 "with 0.");
 				}
@@ -125,20 +70,20 @@ int main(int argc, char **argv)
 				if (WIFSIGNALED(status)) {
 					/* SIGKILL must be uncatchable. */
 					if (WTERMSIG(status) == SIGKILL) {
-						tst_resm(TPASS,
+						tst_res(TPASS,
 							 "Killed with SIGKILL, "
 							 "as expected.");
 					} else {
-						tst_resm(TPASS,
+						tst_brk(TFAIL | TERRNO,
 							 "Didn't die with "
 							 "SIGKILL (?!) ");
 					}
 				} else if (WIFEXITED(status)) {
-					tst_resm(TFAIL,
+					tst_brk(TFAIL | TERRNO,
 						 "Exited unexpectedly instead "
 						 "of dying with SIGKILL.");
 				} else if (WIFSTOPPED(status)) {
-					tst_resm(TFAIL,
+					tst_brk(TFAIL | TERRNO,
 						 "Stopped instead of dying "
 						 "with SIGKILL.");
 				}
@@ -146,35 +91,21 @@ int main(int argc, char **argv)
 				/* All other processes should be stopped. */
 			default:
 				if (WIFSTOPPED(status)) {
-					tst_resm(TPASS, "Stopped as expected");
+					tst_res(TPASS, "Stopped as expected");
 				} else {
-					tst_resm(TFAIL, "Didn't stop as "
+					tst_brk(TFAIL | TERRNO, "Didn't stop as "
 						 "expected.");
-					if (kill(child, 0)) {
-						tst_resm(TINFO,
-							 "Is still alive!?");
-					} else if (WIFEXITED(status)) {
-						tst_resm(TINFO,
-							 "Exited normally");
-					} else if (WIFSIGNALED(status)) {
-						tst_resm(TINFO,
-							 "Was signaled with "
-							 "signum=%d",
-							 WTERMSIG(status));
-					}
-
 				}
-
 				break;
-
 			}
-
 		}
-		/* Make sure the child dies a quick and painless death ... */
-		kill(child, 9);
 
+		if (signum != 0 && signum != 9)
+			SAFE_PTRACE(PTRACE_CONT, child, NULL, NULL);
 	}
-
-	tst_exit();
-
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.forks_child = 1,
+};
-- 
2.35.3


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

[LTP] [PATCH v1 2/2] ptrace06: Refactor the test using new LTP API

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/ptrace/ptrace06.c | 306 +++++++++++---------
 1 file changed, 175 insertions(+), 131 deletions(-)

diff --git a/testcases/kernel/syscalls/ptrace/ptrace06.c b/testcases/kernel/syscalls/ptrace/ptrace06.c
index c0cb3b9bd..5829faea4 100644
--- a/testcases/kernel/syscalls/ptrace/ptrace06.c
+++ b/testcases/kernel/syscalls/ptrace/ptrace06.c
@@ -1,32 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
+ * Copyright (c) 2008 Analog Devices Inc.
+ * Copyright (c) 2023 Wei Gao <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
  * check out-of-bound/unaligned addresses given to
  *  - {PEEK,POKE}{DATA,TEXT,USER}
  *  - {GET,SET}{,FG}REGS
  *  - {GET,SET}SIGINFO
  *
- * Copyright (c) 2008 Analog Devices Inc.
- *
- * Licensed under the GPL-2 or later
  */
 
 #define _GNU_SOURCE
 
-#include <errno.h>
-#include <stdbool.h>
-#include <stdio.h>
 #include <stdlib.h>
-#include <unistd.h>
-
 #include <config.h>
-#include "ptrace.h"
 
-#include "test.h"
-#include "spawn_ptrace_child.h"
-#include "config.h"
+#include "ptrace.h"
+#include "tst_test.h"
 
 /* this should be sizeof(struct user), but that info is only found
  * in the kernel asm/user.h which is not exported to userspace.
  */
+
 #if defined(__i386__)
 #define SIZEOF_USER 284
 #elif defined(__x86_64__)
@@ -35,168 +34,213 @@
 #define SIZEOF_USER 0x1000	/* just pick a big number */
 #endif
 
-char *TCID = "ptrace06";
-
 struct test_case_t {
 	int request;
 	long addr;
 	long data;
 } test_cases[] = {
 	{
-	PTRACE_PEEKDATA,.addr = 0}, {
-	PTRACE_PEEKDATA,.addr = 1}, {
-	PTRACE_PEEKDATA,.addr = 2}, {
-	PTRACE_PEEKDATA,.addr = 3}, {
-	PTRACE_PEEKDATA,.addr = -1}, {
-	PTRACE_PEEKDATA,.addr = -2}, {
-	PTRACE_PEEKDATA,.addr = -3}, {
-	PTRACE_PEEKDATA,.addr = -4}, {
-	PTRACE_PEEKTEXT,.addr = 0}, {
-	PTRACE_PEEKTEXT,.addr = 1}, {
-	PTRACE_PEEKTEXT,.addr = 2}, {
-	PTRACE_PEEKTEXT,.addr = 3}, {
-	PTRACE_PEEKTEXT,.addr = -1}, {
-	PTRACE_PEEKTEXT,.addr = -2}, {
-	PTRACE_PEEKTEXT,.addr = -3}, {
-	PTRACE_PEEKTEXT,.addr = -4}, {
-	PTRACE_PEEKUSER,.addr = SIZEOF_USER + 1}, {
-	PTRACE_PEEKUSER,.addr = SIZEOF_USER + 2}, {
-	PTRACE_PEEKUSER,.addr = SIZEOF_USER + 3}, {
-	PTRACE_PEEKUSER,.addr = SIZEOF_USER + 4}, {
-	PTRACE_PEEKUSER,.addr = -1}, {
-	PTRACE_PEEKUSER,.addr = -2}, {
-	PTRACE_PEEKUSER,.addr = -3}, {
-	PTRACE_PEEKUSER,.addr = -4}, {
-	PTRACE_POKEDATA,.addr = 0}, {
-	PTRACE_POKEDATA,.addr = 1}, {
-	PTRACE_POKEDATA,.addr = 2}, {
-	PTRACE_POKEDATA,.addr = 3}, {
-	PTRACE_POKEDATA,.addr = -1}, {
-	PTRACE_POKEDATA,.addr = -2}, {
-	PTRACE_POKEDATA,.addr = -3}, {
-	PTRACE_POKEDATA,.addr = -4}, {
-	PTRACE_POKETEXT,.addr = 0}, {
-	PTRACE_POKETEXT,.addr = 1}, {
-	PTRACE_POKETEXT,.addr = 2}, {
-	PTRACE_POKETEXT,.addr = 3}, {
-	PTRACE_POKETEXT,.addr = -1}, {
-	PTRACE_POKETEXT,.addr = -2}, {
-	PTRACE_POKETEXT,.addr = -3}, {
-	PTRACE_POKETEXT,.addr = -4}, {
-	PTRACE_POKEUSER,.addr = SIZEOF_USER + 1}, {
-	PTRACE_POKEUSER,.addr = SIZEOF_USER + 2}, {
-	PTRACE_POKEUSER,.addr = SIZEOF_USER + 3}, {
-	PTRACE_POKEUSER,.addr = SIZEOF_USER + 4}, {
-	PTRACE_POKEUSER,.addr = -1}, {
-	PTRACE_POKEUSER,.addr = -2}, {
-	PTRACE_POKEUSER,.addr = -3}, {
-	PTRACE_POKEUSER,.addr = -4},
+	PTRACE_PEEKDATA, .addr = 0}, {
+	PTRACE_PEEKDATA, .addr = 1}, {
+	PTRACE_PEEKDATA, .addr = 2}, {
+	PTRACE_PEEKDATA, .addr = 3}, {
+	PTRACE_PEEKDATA, .addr = -1}, {
+	PTRACE_PEEKDATA, .addr = -2}, {
+	PTRACE_PEEKDATA, .addr = -3}, {
+	PTRACE_PEEKDATA, .addr = -4}, {
+	PTRACE_PEEKTEXT, .addr = 0}, {
+	PTRACE_PEEKTEXT, .addr = 1}, {
+	PTRACE_PEEKTEXT, .addr = 2}, {
+	PTRACE_PEEKTEXT, .addr = 3}, {
+	PTRACE_PEEKTEXT, .addr = -1}, {
+	PTRACE_PEEKTEXT, .addr = -2}, {
+	PTRACE_PEEKTEXT, .addr = -3}, {
+	PTRACE_PEEKTEXT, .addr = -4}, {
+	PTRACE_PEEKUSER, .addr = SIZEOF_USER + 1}, {
+	PTRACE_PEEKUSER, .addr = SIZEOF_USER + 2}, {
+	PTRACE_PEEKUSER, .addr = SIZEOF_USER + 3}, {
+	PTRACE_PEEKUSER, .addr = SIZEOF_USER + 4}, {
+	PTRACE_PEEKUSER, .addr = -1}, {
+	PTRACE_PEEKUSER, .addr = -2}, {
+	PTRACE_PEEKUSER, .addr = -3}, {
+	PTRACE_PEEKUSER, .addr = -4}, {
+	PTRACE_POKEDATA, .addr = 0}, {
+	PTRACE_POKEDATA, .addr = 1}, {
+	PTRACE_POKEDATA, .addr = 2}, {
+	PTRACE_POKEDATA, .addr = 3}, {
+	PTRACE_POKEDATA, .addr = -1}, {
+	PTRACE_POKEDATA, .addr = -2}, {
+	PTRACE_POKEDATA, .addr = -3}, {
+	PTRACE_POKEDATA, .addr = -4}, {
+	PTRACE_POKETEXT, .addr = 0}, {
+	PTRACE_POKETEXT, .addr = 1}, {
+	PTRACE_POKETEXT, .addr = 2}, {
+	PTRACE_POKETEXT, .addr = 3}, {
+	PTRACE_POKETEXT, .addr = -1}, {
+	PTRACE_POKETEXT, .addr = -2}, {
+	PTRACE_POKETEXT, .addr = -3}, {
+	PTRACE_POKETEXT, .addr = -4}, {
+	PTRACE_POKEUSER, .addr = SIZEOF_USER + 1}, {
+	PTRACE_POKEUSER, .addr = SIZEOF_USER + 2}, {
+	PTRACE_POKEUSER, .addr = SIZEOF_USER + 3}, {
+	PTRACE_POKEUSER, .addr = SIZEOF_USER + 4}, {
+	PTRACE_POKEUSER, .addr = -1}, {
+	PTRACE_POKEUSER, .addr = -2}, {
+	PTRACE_POKEUSER, .addr = -3}, {
+	PTRACE_POKEUSER, .addr = -4},
 #ifdef PTRACE_GETREGS
 	{
-	PTRACE_GETREGS,.data = 0}, {
-	PTRACE_GETREGS,.data = 1}, {
-	PTRACE_GETREGS,.data = 2}, {
-	PTRACE_GETREGS,.data = 3}, {
-	PTRACE_GETREGS,.data = -1}, {
-	PTRACE_GETREGS,.data = -2}, {
-	PTRACE_GETREGS,.data = -3}, {
-	PTRACE_GETREGS,.data = -4},
+	PTRACE_GETREGS, .data = 0}, {
+	PTRACE_GETREGS, .data = 1}, {
+	PTRACE_GETREGS, .data = 2}, {
+	PTRACE_GETREGS, .data = 3}, {
+	PTRACE_GETREGS, .data = -1}, {
+	PTRACE_GETREGS, .data = -2}, {
+	PTRACE_GETREGS, .data = -3}, {
+	PTRACE_GETREGS, .data = -4},
 #endif
 #ifdef PTRACE_GETFGREGS
 	{
-	PTRACE_GETFGREGS,.data = 0}, {
-	PTRACE_GETFGREGS,.data = 1}, {
-	PTRACE_GETFGREGS,.data = 2}, {
-	PTRACE_GETFGREGS,.data = 3}, {
-	PTRACE_GETFGREGS,.data = -1}, {
-	PTRACE_GETFGREGS,.data = -2}, {
-	PTRACE_GETFGREGS,.data = -3}, {
-	PTRACE_GETFGREGS,.data = -4},
+	PTRACE_GETFGREGS, .data = 0}, {
+	PTRACE_GETFGREGS, .data = 1}, {
+	PTRACE_GETFGREGS, .data = 2}, {
+	PTRACE_GETFGREGS, .data = 3}, {
+	PTRACE_GETFGREGS, .data = -1}, {
+	PTRACE_GETFGREGS, .data = -2}, {
+	PTRACE_GETFGREGS, .data = -3}, {
+	PTRACE_GETFGREGS, .data = -4},
 #endif
 #ifdef PTRACE_SETREGS
 	{
-	PTRACE_SETREGS,.data = 0}, {
-	PTRACE_SETREGS,.data = 1}, {
-	PTRACE_SETREGS,.data = 2}, {
-	PTRACE_SETREGS,.data = 3}, {
-	PTRACE_SETREGS,.data = -1}, {
-	PTRACE_SETREGS,.data = -2}, {
-	PTRACE_SETREGS,.data = -3}, {
-	PTRACE_SETREGS,.data = -4},
+	PTRACE_SETREGS, .data = 0}, {
+	PTRACE_SETREGS, .data = 1}, {
+	PTRACE_SETREGS, .data = 2}, {
+	PTRACE_SETREGS, .data = 3}, {
+	PTRACE_SETREGS, .data = -1}, {
+	PTRACE_SETREGS, .data = -2}, {
+	PTRACE_SETREGS, .data = -3}, {
+	PTRACE_SETREGS, .data = -4},
 #endif
 #ifdef PTRACE_SETFGREGS
 	{
-	PTRACE_SETFGREGS,.data = 0}, {
-	PTRACE_SETFGREGS,.data = 1}, {
-	PTRACE_SETFGREGS,.data = 2}, {
-	PTRACE_SETFGREGS,.data = 3}, {
-	PTRACE_SETFGREGS,.data = -1}, {
-	PTRACE_SETFGREGS,.data = -2}, {
-	PTRACE_SETFGREGS,.data = -3}, {
-	PTRACE_SETFGREGS,.data = -4},
+	PTRACE_SETFGREGS, .data = 0}, {
+	PTRACE_SETFGREGS, .data = 1}, {
+	PTRACE_SETFGREGS, .data = 2}, {
+	PTRACE_SETFGREGS, .data = 3}, {
+	PTRACE_SETFGREGS, .data = -1}, {
+	PTRACE_SETFGREGS, .data = -2}, {
+	PTRACE_SETFGREGS, .data = -3}, {
+	PTRACE_SETFGREGS, .data = -4},
 #endif
 #if HAVE_DECL_PTRACE_GETSIGINFO
 	{
-	PTRACE_GETSIGINFO,.data = 0}, {
-	PTRACE_GETSIGINFO,.data = 1}, {
-	PTRACE_GETSIGINFO,.data = 2}, {
-	PTRACE_GETSIGINFO,.data = 3}, {
-	PTRACE_GETSIGINFO,.data = -1}, {
-	PTRACE_GETSIGINFO,.data = -2}, {
-	PTRACE_GETSIGINFO,.data = -3}, {
-	PTRACE_GETSIGINFO,.data = -4},
+	PTRACE_GETSIGINFO, .data = 0}, {
+	PTRACE_GETSIGINFO, .data = 1}, {
+	PTRACE_GETSIGINFO, .data = 2}, {
+	PTRACE_GETSIGINFO, .data = 3}, {
+	PTRACE_GETSIGINFO, .data = -1}, {
+	PTRACE_GETSIGINFO, .data = -2}, {
+	PTRACE_GETSIGINFO, .data = -3}, {
+	PTRACE_GETSIGINFO, .data = -4},
 #endif
 #if HAVE_DECL_PTRACE_SETSIGINFO
 	{
-	PTRACE_SETSIGINFO,.data = 0}, {
-	PTRACE_SETSIGINFO,.data = 1}, {
-	PTRACE_SETSIGINFO,.data = 2}, {
-	PTRACE_SETSIGINFO,.data = 3}, {
-	PTRACE_SETSIGINFO,.data = -1}, {
-	PTRACE_SETSIGINFO,.data = -2}, {
-	PTRACE_SETSIGINFO,.data = -3}, {
-	PTRACE_SETSIGINFO,.data = -4},
+	PTRACE_SETSIGINFO, .data = 0}, {
+	PTRACE_SETSIGINFO, .data = 1}, {
+	PTRACE_SETSIGINFO, .data = 2}, {
+	PTRACE_SETSIGINFO, .data = 3}, {
+	PTRACE_SETSIGINFO, .data = -1}, {
+	PTRACE_SETSIGINFO, .data = -2}, {
+	PTRACE_SETSIGINFO, .data = -3}, {
+	PTRACE_SETSIGINFO, .data = -4},
+#endif
+};
+
+#define SPT(x) [PTRACE_##x] = #x,
+static char *strings[] = {
+	SPT(TRACEME)
+	SPT(PEEKTEXT)
+	SPT(PEEKDATA)
+	SPT(PEEKUSER)
+	SPT(POKETEXT)
+	SPT(POKEDATA)
+	SPT(POKEUSER)
+#ifdef PTRACE_GETREGS
+	SPT(GETREGS)
+#endif
+#ifdef PTRACE_SETREGS
+	SPT(SETREGS)
+#endif
+#ifdef PTRACE_GETSIGINFO
+	SPT(GETSIGINFO)
+#endif
+#ifdef PTRACE_SETSIGINFO
+	SPT(SETSIGINFO)
+#endif
+#ifdef PTRACE_GETFGREGS
+	SPT(GETFGREGS)
+#endif
+#ifdef PTRACE_SETFGREGS
+	SPT(SETFGREGS)
 #endif
+	SPT(KILL)
+	SPT(SINGLESTEP)
 };
 
-int TST_TOTAL = ARRAY_SIZE(test_cases);
+static inline char *strptrace(int request)
+{
+	return strings[request];
+}
+
+static void child(void)
+{
+	SAFE_PTRACE(PTRACE_TRACEME, 0, NULL, NULL);
+	execl("/bin/echo", "/bin/echo", NULL);
+	exit(0);
+}
 
-int main(int argc, char *argv[])
+static void run(void)
 {
 	size_t i;
-	long ret;
-	int saved_errno;
+	int pid;
+	int status;
 
-	tst_parse_opts(argc, argv, NULL, NULL);
+	pid = SAFE_FORK();
 
-	make_a_baby(argc, argv);
+	if (!pid)
+		child();
+
+	SAFE_WAIT(&status);
+
+	if (!WIFSTOPPED(status))
+		tst_brk(TBROK, "child %d was not stopped", pid);
 
 	for (i = 0; i < ARRAY_SIZE(test_cases); ++i) {
 		struct test_case_t *tc = &test_cases[i];
 
-		errno = 0;
-		ret =
-		    ptrace(tc->request, pid, (void *)tc->addr,
-			   (void *)tc->data);
-		saved_errno = errno;
-		if (ret != -1)
-			tst_resm(TFAIL,
+		TEST(ptrace(tc->request, pid, (void *)tc->addr,
+					(void *)tc->data));
+		if (TST_RET != -1)
+			tst_brk(TFAIL | TERRNO,
 				 "ptrace(%s, ..., %li, %li) returned %li instead of -1",
 				 strptrace(tc->request), tc->addr, tc->data,
-				 ret);
-		else if (saved_errno != EIO && saved_errno != EFAULT)
-			tst_resm(TFAIL,
+				 TST_RET);
+		else if (TST_ERR != EIO && TST_ERR != EFAULT)
+			tst_brk(TFAIL | TERRNO,
 				 "ptrace(%s, ..., %li, %li) expected errno EIO or EFAULT; actual: %i (%s)",
 				 strptrace(tc->request), tc->addr, tc->data,
-				 saved_errno, strerror(saved_errno));
+				 TST_ERR, strerror(TST_ERR));
 		else
-			tst_resm(TPASS,
+			tst_res(TPASS,
 				 "ptrace(%s, ..., %li, %li) failed as expected",
 				 strptrace(tc->request), tc->addr, tc->data);
 	}
 
-	/* hopefully this worked */
-	ptrace(PTRACE_KILL, pid, NULL, NULL);
+	SAFE_PTRACE(PTRACE_CONT, pid, NULL, NULL);
 
-	tst_exit();
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.forks_child = 1,
+};
-- 
2.35.3


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

Re: [LTP] [PATCH v1 1/2] ptrace05: Refactor the test using new LTP API

[Richard Palethorpe]

Hello,

Wei Gao via ltp <ltp@lists.linux.it> writes:

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  testcases/kernel/syscalls/ptrace/ptrace05.c | 147 ++++++--------------
>  1 file changed, 39 insertions(+), 108 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/ptrace/ptrace05.c b/testcases/kernel/syscalls/ptrace/ptrace05.c
> index 54cfa4d7b..4904b959c 100644
> --- a/testcases/kernel/syscalls/ptrace/ptrace05.c
> +++ b/testcases/kernel/syscalls/ptrace/ptrace05.c
> @@ -1,122 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0-only
>  /*
> - ******************************************************************************
> - *
> - *   ptrace05 - an app which ptraces itself as per arbitrarily specified signals,
> - *   over a user specified range.
> - *
> - *   Copyright (C) 2009, Ngie Cooper
> - *
> - *   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.
> + * Copyright (C) 2009, Ngie Cooper
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
>   *
> - *   You should have received a copy of the GNU General Public License along
> - *   with this program; if not, write to the Free Software Foundation, Inc.,
> - *   51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + *  ptrace05 - an app which ptraces itself as per arbitrarily specified signals
>   *
> - ******************************************************************************
>   */
>  
> -#include <sys/types.h>
> -#include <sys/wait.h>
> -#include <signal.h>
> -#include <errno.h>
> -#include <libgen.h>
> -#include <math.h>
>  #include <stdlib.h>
> -#include <stdio.h>
> -#include <string.h>
> -#include <unistd.h>
> -
>  #include <config.h>
>  #include "ptrace.h"
>  
> -#include "test.h"
>  #include "lapi/signal.h"
> +#include "tst_test.h"
>  
> -char *TCID = "ptrace05";
> -int TST_TOTAL = 0;
> -
> -int usage(const char *);
> -
> -int usage(const char *argv0)
> -{
> -	fprintf(stderr, "usage: %s [start-signum] [end-signum]\n", argv0);
> -	return 1;
> -}
> -
> -int main(int argc, char **argv)
> +static void run(void)
>  {
>  
> -	int end_signum = -1;
> -	int signum;
> -	int start_signum = -1;
> +	int end_signum = SIGRTMAX;
> +	int signum = 0;
> +	int start_signum = 0;
>  	int status;

{start,end}_signum don't appear to serve a purpose anymore.

>  
>  	pid_t child;
>  
> -	tst_parse_opts(argc, argv, NULL, NULL);
> -
> -	if (start_signum == -1) {
> -		start_signum = 0;
> -	}
> -	if (end_signum == -1) {
> -		end_signum = SIGRTMAX;
> -	}
> -
>  	for (signum = start_signum; signum <= end_signum; signum++) {
>  
> -		if (signum >= __SIGRTMIN && signum < SIGRTMIN)
> -			continue;

Why can this be removed?

I remember we had an issue on some systems because some signals are
reserved by libc.

> -
> -		switch (child = fork()) {
> +		switch (child = SAFE_FORK()) {
>  		case -1:
> -			tst_brkm(TBROK | TERRNO, NULL, "fork() failed");
> +			tst_brk(TBROK | TERRNO, "fork() failed");
>  		case 0:
>  
> -			if (ptrace(PTRACE_TRACEME, 0, NULL, NULL) != -1) {
> -				tst_resm(TINFO, "[child] Sending kill(.., %d)",
> -					 signum);
> -				if (kill(getpid(), signum) < 0) {
> -					tst_resm(TINFO | TERRNO,
> -						 "[child] kill(.., %d) failed.",
> -						 signum);
> -				}
> +			TEST(ptrace(PTRACE_TRACEME, 0, NULL, NULL));
> +			if (TST_RET != -1) {
> +				tst_res(TINFO, "[child] Sending kill(.., %d)",
> +						signum);
> +				SAFE_KILL(getpid(), signum);
>  			} else {
> -
> -				/*
> -				 * This won't increment the TST_COUNT var.
> -				 * properly, but it'll show up as a failure
> -				 * nonetheless.
> -				 */
> -				tst_resm(TFAIL | TERRNO,
> +				tst_brk(TFAIL | TERRNO,
>  					 "Failed to ptrace(PTRACE_TRACEME, ...) "
>  					 "properly");
> -
>  			}
> -			/* Shouldn't get here if signum == 0. */
> -			exit((signum == 0 ? 0 : 2));
> +
> +			exit(0);
>  			break;
>  
>  		default:
>  
> -			waitpid(child, &status, 0);
> +			SAFE_WAITPID(child, &status, 0);
>  
>  			switch (signum) {
>  			case 0:
>  				if (WIFEXITED(status)
>  				    && WEXITSTATUS(status) == 0) {
> -					tst_resm(TPASS,
> +					tst_res(TPASS,
>  						 "kill(.., 0) exited "
>  						 "with 0, as expected.");
>  				} else {
> -					tst_resm(TFAIL,
> +					tst_brk(TFAIL | TERRNO,
>  						 "kill(.., 0) didn't exit "
>  						 "with 0.");
>  				}
> @@ -125,20 +70,20 @@ int main(int argc, char **argv)
>  				if (WIFSIGNALED(status)) {
>  					/* SIGKILL must be uncatchable. */
>  					if (WTERMSIG(status) == SIGKILL) {
> -						tst_resm(TPASS,
> +						tst_res(TPASS,
>  							 "Killed with SIGKILL, "
>  							 "as expected.");
>  					} else {
> -						tst_resm(TPASS,
> +						tst_brk(TFAIL | TERRNO,
>  							 "Didn't die with "
>  							 "SIGKILL (?!) ");
>  					}
>  				} else if (WIFEXITED(status)) {
> -					tst_resm(TFAIL,
> +					tst_brk(TFAIL | TERRNO,
>  						 "Exited unexpectedly instead "
>  						 "of dying with SIGKILL.");
>  				} else if (WIFSTOPPED(status)) {
> -					tst_resm(TFAIL,
> +					tst_brk(TFAIL | TERRNO,
>  						 "Stopped instead of dying "
>  						 "with SIGKILL.");
>  				}
> @@ -146,35 +91,21 @@ int main(int argc, char **argv)
>  				/* All other processes should be stopped. */
>  			default:
>  				if (WIFSTOPPED(status)) {
> -					tst_resm(TPASS, "Stopped as expected");
> +					tst_res(TPASS, "Stopped as expected");
>  				} else {
> -					tst_resm(TFAIL, "Didn't stop as "
> +					tst_brk(TFAIL | TERRNO, "Didn't stop as "
>  						 "expected.");
> -					if (kill(child, 0)) {
> -						tst_resm(TINFO,
> -							 "Is still alive!?");
> -					} else if (WIFEXITED(status)) {
> -						tst_resm(TINFO,
> -							 "Exited normally");
> -					} else if (WIFSIGNALED(status)) {
> -						tst_resm(TINFO,
> -							 "Was signaled with "
> -							 "signum=%d",
> -							 WTERMSIG(status));
> -					}
> -
>  				}
> -
>  				break;
> -
>  			}
> -
>  		}
> -		/* Make sure the child dies a quick and painless death ... */
> -		kill(child, 9);
>  
> +		if (signum != 0 && signum != 9)
> +			SAFE_PTRACE(PTRACE_CONT, child, NULL, NULL);

nit; it's clearer to write SIGKILL than 9 also some other signals change
number between platforms.

>  	}
> -
> -	tst_exit();
> -
>  }
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.forks_child = 1,
> +};
> -- 
> 2.35.3


-- 
Thank you,
Richard.

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

Re: [LTP] [PATCH v1 1/2] ptrace05: Refactor the test using new LTP API

[Petr Vorel]

Hi Wei,

make check-ptrace05 shows various redefinitions, that points on ptrace.h cleanup
needed, I'll fix it as a separate change.

CHECK testcases/kernel/syscalls/ptrace/ptrace05.c
ptrace05.c: note: in included file (through ptrace.h):
/usr/include/linux/ptrace.h:50:9: warning: preprocessor token PTRACE_GETREGSET redefined
ptrace05.c: note: in included file (through ptrace.h):
/usr/include/sys/ptrace.h:153:9: this was the original definition
ptrace05.c: note: in included file (through ptrace.h):
/usr/include/linux/ptrace.h:51:9: warning: preprocessor token PTRACE_SETREGSET redefined
ptrace05.c: note: in included file (through ptrace.h):
/usr/include/sys/ptrace.h:157:9: this was the original definition
ptrace05.c: note: in included file (through ptrace.h):
/usr/include/linux/ptrace.h:53:9: warning: preprocessor token PTRACE_SEIZE redefined

I handled this in separate patchset [3], I Cc you. Could you please base v2 on it?
I hope it will be merged soon.

[1] https://sourceware.org/glibc/wiki/Synchronizing_Headers
[2] https://github.com/linux-test-project/ltp/wiki/Supported-kernel,-libc,-toolchain-versions#11-oldest-tested-distributions
[3] https://patchwork.ozlabs.org/project/ltp/list/?series=384172&state=*

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  testcases/kernel/syscalls/ptrace/ptrace05.c | 147 ++++++--------------
>  1 file changed, 39 insertions(+), 108 deletions(-)

> diff --git a/testcases/kernel/syscalls/ptrace/ptrace05.c b/testcases/kernel/syscalls/ptrace/ptrace05.c
> index 54cfa4d7b..4904b959c 100644
> --- a/testcases/kernel/syscalls/ptrace/ptrace05.c
> +++ b/testcases/kernel/syscalls/ptrace/ptrace05.c
> @@ -1,122 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0-only
// SPDX-License-Identifier: GPL-2.0-or-later

>  /*
> - ******************************************************************************
> - *
> - *   ptrace05 - an app which ptraces itself as per arbitrarily specified signals,
> - *   over a user specified range.
> - *
> - *   Copyright (C) 2009, Ngie Cooper
> - *
> - *   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.
"any later version" is why GPL-2.0-or-later instead of GPL-2.0-only.

> - *   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.
> + * Copyright (C) 2009, Ngie Cooper
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
Please add also LTP copyright (more people contributed to this test):
* Copyright (c) Linux Test Project, 2009-2019
> + */
> +
> +/*\
> + * [Description]
>   *
> - *   You should have received a copy of the GNU General Public License along
> - *   with this program; if not, write to the Free Software Foundation, Inc.,
> - *   51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + *  ptrace05 - an app which ptraces itself as per arbitrarily specified signals
I'm not really sure for a proper description, but we don't list the test name,
also s/app/test/

>   *
> - ******************************************************************************
>   */

> -#include <sys/types.h>
> -#include <sys/wait.h>
> -#include <signal.h>
> -#include <errno.h>
> -#include <libgen.h>
> -#include <math.h>
>  #include <stdlib.h>
> -#include <stdio.h>
> -#include <string.h>
> -#include <unistd.h>
> -
>  #include <config.h>
>  #include "ptrace.h"

> -#include "test.h"
>  #include "lapi/signal.h"
> +#include "tst_test.h"

> -char *TCID = "ptrace05";
> -int TST_TOTAL = 0;
> -
> -int usage(const char *);
> -
> -int usage(const char *argv0)
> -{
> -	fprintf(stderr, "usage: %s [start-signum] [end-signum]\n", argv0);
> -	return 1;
> -}
> -
> -int main(int argc, char **argv)
> +static void run(void)
>  {

> -	int end_signum = -1;
> -	int signum;
> -	int start_signum = -1;
> +	int end_signum = SIGRTMAX;
> +	int signum = 0;
> +	int start_signum = 0;
>  	int status;

>  	pid_t child;

> -	tst_parse_opts(argc, argv, NULL, NULL);
> -
> -	if (start_signum == -1) {
> -		start_signum = 0;
> -	}
> -	if (end_signum == -1) {
> -		end_signum = SIGRTMAX;
> -	}
> -
>  	for (signum = start_signum; signum <= end_signum; signum++) {

> -		if (signum >= __SIGRTMIN && signum < SIGRTMIN)
> -			continue;
> -
> -		switch (child = fork()) {
> +		switch (child = SAFE_FORK()) {
>  		case -1:
> -			tst_brkm(TBROK | TERRNO, NULL, "fork() failed");
> +			tst_brk(TBROK | TERRNO, "fork() failed");
We have this handled in SAFE_FORK(), it should be removed here.
Therefore switch+case should be replaced with if+else.

>  		case 0:

> -				tst_resm(TFAIL | TERRNO,
> +				tst_brk(TFAIL | TERRNO,
>  					 "Failed to ptrace(PTRACE_TRACEME, ...) "
>  					 "properly");
> -
>  			}
...
> -			/* Shouldn't get here if signum == 0. */
> -			exit((signum == 0 ? 0 : 2));
> +
> +			exit(0);
Why there can be always exit(0) ?
>  			break;

>  		default:

> -			waitpid(child, &status, 0);
> +			SAFE_WAITPID(child, &status, 0);

>  			switch (signum) {
>  			case 0:
>  				if (WIFEXITED(status)
>  				    && WEXITSTATUS(status) == 0) {
> -					tst_resm(TPASS,
> +					tst_res(TPASS,
>  						 "kill(.., 0) exited "
>  						 "with 0, as expected.");
Please join strings (this applies to code below as well).

>  				} else {
> -					tst_resm(TFAIL,
> +					tst_brk(TFAIL | TERRNO,
>  						 "kill(.., 0) didn't exit "
>  						 "with 0.");
Why not tst_res(TFAIL, ...) ?  (this applies to code below as well)

...

Kind regards,
Petr

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

Re: [LTP] [PATCH v1 2/2] ptrace06: Refactor the test using new LTP API

[Richard Palethorpe]

Hello,

Wei Gao via ltp <ltp@lists.linux.it> writes:

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  testcases/kernel/syscalls/ptrace/ptrace06.c | 306 +++++++++++---------
>  1 file changed, 175 insertions(+), 131 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/ptrace/ptrace06.c b/testcases/kernel/syscalls/ptrace/ptrace06.c
> index c0cb3b9bd..5829faea4 100644
> --- a/testcases/kernel/syscalls/ptrace/ptrace06.c
> +++ b/testcases/kernel/syscalls/ptrace/ptrace06.c
> @@ -1,32 +1,31 @@
> +// SPDX-License-Identifier: GPL-2.0-only
>  /*
> + * Copyright (c) 2008 Analog Devices Inc.
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
>   * check out-of-bound/unaligned addresses given to
>   *  - {PEEK,POKE}{DATA,TEXT,USER}
>   *  - {GET,SET}{,FG}REGS
>   *  - {GET,SET}SIGINFO
>   *
> - * Copyright (c) 2008 Analog Devices Inc.
> - *
> - * Licensed under the GPL-2 or later
>   */
>  
>  #define _GNU_SOURCE
>  
> -#include <errno.h>
> -#include <stdbool.h>
> -#include <stdio.h>
>  #include <stdlib.h>
> -#include <unistd.h>
> -
>  #include <config.h>
> -#include "ptrace.h"
>  
> -#include "test.h"
> -#include "spawn_ptrace_child.h"
> -#include "config.h"
> +#include "ptrace.h"
> +#include "tst_test.h"
>  
>  /* this should be sizeof(struct user), but that info is only found
>   * in the kernel asm/user.h which is not exported to userspace.
>   */
> +
>  #if defined(__i386__)
>  #define SIZEOF_USER 284
>  #elif defined(__x86_64__)
> @@ -35,168 +34,213 @@
>  #define SIZEOF_USER 0x1000	/* just pick a big number */
>  #endif
>  
> -char *TCID = "ptrace06";
> -
>  struct test_case_t {
>  	int request;
>  	long addr;
>  	long data;
>  } test_cases[] = {
>  	{
> -	PTRACE_PEEKDATA,.addr = 0}, {
> -	PTRACE_PEEKDATA,.addr = 1}, {
> -	PTRACE_PEEKDATA,.addr = 2}, {
> -	PTRACE_PEEKDATA,.addr = 3}, {
> -	PTRACE_PEEKDATA,.addr = -1}, {
> -	PTRACE_PEEKDATA,.addr = -2}, {
> -	PTRACE_PEEKDATA,.addr = -3}, {
> -	PTRACE_PEEKDATA,.addr = -4}, {
> -	PTRACE_PEEKTEXT,.addr = 0}, {
> -	PTRACE_PEEKTEXT,.addr = 1}, {
> -	PTRACE_PEEKTEXT,.addr = 2}, {
> -	PTRACE_PEEKTEXT,.addr = 3}, {
> -	PTRACE_PEEKTEXT,.addr = -1}, {
> -	PTRACE_PEEKTEXT,.addr = -2}, {
> -	PTRACE_PEEKTEXT,.addr = -3}, {
> -	PTRACE_PEEKTEXT,.addr = -4}, {
> -	PTRACE_PEEKUSER,.addr = SIZEOF_USER + 1}, {
> -	PTRACE_PEEKUSER,.addr = SIZEOF_USER + 2}, {
> -	PTRACE_PEEKUSER,.addr = SIZEOF_USER + 3}, {
> -	PTRACE_PEEKUSER,.addr = SIZEOF_USER + 4}, {
> -	PTRACE_PEEKUSER,.addr = -1}, {
> -	PTRACE_PEEKUSER,.addr = -2}, {
> -	PTRACE_PEEKUSER,.addr = -3}, {
> -	PTRACE_PEEKUSER,.addr = -4}, {
> -	PTRACE_POKEDATA,.addr = 0}, {
> -	PTRACE_POKEDATA,.addr = 1}, {
> -	PTRACE_POKEDATA,.addr = 2}, {
> -	PTRACE_POKEDATA,.addr = 3}, {
> -	PTRACE_POKEDATA,.addr = -1}, {
> -	PTRACE_POKEDATA,.addr = -2}, {
> -	PTRACE_POKEDATA,.addr = -3}, {
> -	PTRACE_POKEDATA,.addr = -4}, {
> -	PTRACE_POKETEXT,.addr = 0}, {
> -	PTRACE_POKETEXT,.addr = 1}, {
> -	PTRACE_POKETEXT,.addr = 2}, {
> -	PTRACE_POKETEXT,.addr = 3}, {
> -	PTRACE_POKETEXT,.addr = -1}, {
> -	PTRACE_POKETEXT,.addr = -2}, {
> -	PTRACE_POKETEXT,.addr = -3}, {
> -	PTRACE_POKETEXT,.addr = -4}, {
> -	PTRACE_POKEUSER,.addr = SIZEOF_USER + 1}, {
> -	PTRACE_POKEUSER,.addr = SIZEOF_USER + 2}, {
> -	PTRACE_POKEUSER,.addr = SIZEOF_USER + 3}, {
> -	PTRACE_POKEUSER,.addr = SIZEOF_USER + 4}, {
> -	PTRACE_POKEUSER,.addr = -1}, {
> -	PTRACE_POKEUSER,.addr = -2}, {
> -	PTRACE_POKEUSER,.addr = -3}, {
> -	PTRACE_POKEUSER,.addr = -4},
> +	PTRACE_PEEKDATA, .addr = 0}, {
> +	PTRACE_PEEKDATA, .addr = 1}, {
> +	PTRACE_PEEKDATA, .addr = 2}, {
> +	PTRACE_PEEKDATA, .addr = 3}, {
> +	PTRACE_PEEKDATA, .addr = -1}, {
> +	PTRACE_PEEKDATA, .addr = -2}, {
> +	PTRACE_PEEKDATA, .addr = -3}, {
> +	PTRACE_PEEKDATA, .addr = -4}, {
> +	PTRACE_PEEKTEXT, .addr = 0}, {
> +	PTRACE_PEEKTEXT, .addr = 1}, {
> +	PTRACE_PEEKTEXT, .addr = 2}, {
> +	PTRACE_PEEKTEXT, .addr = 3}, {
> +	PTRACE_PEEKTEXT, .addr = -1}, {
> +	PTRACE_PEEKTEXT, .addr = -2}, {
> +	PTRACE_PEEKTEXT, .addr = -3}, {
> +	PTRACE_PEEKTEXT, .addr = -4}, {
> +	PTRACE_PEEKUSER, .addr = SIZEOF_USER + 1}, {
> +	PTRACE_PEEKUSER, .addr = SIZEOF_USER + 2}, {
> +	PTRACE_PEEKUSER, .addr = SIZEOF_USER + 3}, {
> +	PTRACE_PEEKUSER, .addr = SIZEOF_USER + 4}, {
> +	PTRACE_PEEKUSER, .addr = -1}, {
> +	PTRACE_PEEKUSER, .addr = -2}, {
> +	PTRACE_PEEKUSER, .addr = -3}, {
> +	PTRACE_PEEKUSER, .addr = -4}, {
> +	PTRACE_POKEDATA, .addr = 0}, {
> +	PTRACE_POKEDATA, .addr = 1}, {
> +	PTRACE_POKEDATA, .addr = 2}, {
> +	PTRACE_POKEDATA, .addr = 3}, {
> +	PTRACE_POKEDATA, .addr = -1}, {
> +	PTRACE_POKEDATA, .addr = -2}, {
> +	PTRACE_POKEDATA, .addr = -3}, {
> +	PTRACE_POKEDATA, .addr = -4}, {
> +	PTRACE_POKETEXT, .addr = 0}, {
> +	PTRACE_POKETEXT, .addr = 1}, {
> +	PTRACE_POKETEXT, .addr = 2}, {
> +	PTRACE_POKETEXT, .addr = 3}, {
> +	PTRACE_POKETEXT, .addr = -1}, {
> +	PTRACE_POKETEXT, .addr = -2}, {
> +	PTRACE_POKETEXT, .addr = -3}, {
> +	PTRACE_POKETEXT, .addr = -4}, {
> +	PTRACE_POKEUSER, .addr = SIZEOF_USER + 1}, {
> +	PTRACE_POKEUSER, .addr = SIZEOF_USER + 2}, {
> +	PTRACE_POKEUSER, .addr = SIZEOF_USER + 3}, {
> +	PTRACE_POKEUSER, .addr = SIZEOF_USER + 4}, {
> +	PTRACE_POKEUSER, .addr = -1}, {
> +	PTRACE_POKEUSER, .addr = -2}, {
> +	PTRACE_POKEUSER, .addr = -3}, {
> +	PTRACE_POKEUSER, .addr = -4},
>  #ifdef PTRACE_GETREGS
>  	{
> -	PTRACE_GETREGS,.data = 0}, {
> -	PTRACE_GETREGS,.data = 1}, {
> -	PTRACE_GETREGS,.data = 2}, {
> -	PTRACE_GETREGS,.data = 3}, {
> -	PTRACE_GETREGS,.data = -1}, {
> -	PTRACE_GETREGS,.data = -2}, {
> -	PTRACE_GETREGS,.data = -3}, {
> -	PTRACE_GETREGS,.data = -4},
> +	PTRACE_GETREGS, .data = 0}, {
> +	PTRACE_GETREGS, .data = 1}, {
> +	PTRACE_GETREGS, .data = 2}, {
> +	PTRACE_GETREGS, .data = 3}, {
> +	PTRACE_GETREGS, .data = -1}, {
> +	PTRACE_GETREGS, .data = -2}, {
> +	PTRACE_GETREGS, .data = -3}, {
> +	PTRACE_GETREGS, .data = -4},
>  #endif
>  #ifdef PTRACE_GETFGREGS
>  	{
> -	PTRACE_GETFGREGS,.data = 0}, {
> -	PTRACE_GETFGREGS,.data = 1}, {
> -	PTRACE_GETFGREGS,.data = 2}, {
> -	PTRACE_GETFGREGS,.data = 3}, {
> -	PTRACE_GETFGREGS,.data = -1}, {
> -	PTRACE_GETFGREGS,.data = -2}, {
> -	PTRACE_GETFGREGS,.data = -3}, {
> -	PTRACE_GETFGREGS,.data = -4},
> +	PTRACE_GETFGREGS, .data = 0}, {
> +	PTRACE_GETFGREGS, .data = 1}, {
> +	PTRACE_GETFGREGS, .data = 2}, {
> +	PTRACE_GETFGREGS, .data = 3}, {
> +	PTRACE_GETFGREGS, .data = -1}, {
> +	PTRACE_GETFGREGS, .data = -2}, {
> +	PTRACE_GETFGREGS, .data = -3}, {
> +	PTRACE_GETFGREGS, .data = -4},
>  #endif
>  #ifdef PTRACE_SETREGS
>  	{
> -	PTRACE_SETREGS,.data = 0}, {
> -	PTRACE_SETREGS,.data = 1}, {
> -	PTRACE_SETREGS,.data = 2}, {
> -	PTRACE_SETREGS,.data = 3}, {
> -	PTRACE_SETREGS,.data = -1}, {
> -	PTRACE_SETREGS,.data = -2}, {
> -	PTRACE_SETREGS,.data = -3}, {
> -	PTRACE_SETREGS,.data = -4},
> +	PTRACE_SETREGS, .data = 0}, {
> +	PTRACE_SETREGS, .data = 1}, {
> +	PTRACE_SETREGS, .data = 2}, {
> +	PTRACE_SETREGS, .data = 3}, {
> +	PTRACE_SETREGS, .data = -1}, {
> +	PTRACE_SETREGS, .data = -2}, {
> +	PTRACE_SETREGS, .data = -3}, {
> +	PTRACE_SETREGS, .data = -4},
>  #endif
>  #ifdef PTRACE_SETFGREGS
>  	{
> -	PTRACE_SETFGREGS,.data = 0}, {
> -	PTRACE_SETFGREGS,.data = 1}, {
> -	PTRACE_SETFGREGS,.data = 2}, {
> -	PTRACE_SETFGREGS,.data = 3}, {
> -	PTRACE_SETFGREGS,.data = -1}, {
> -	PTRACE_SETFGREGS,.data = -2}, {
> -	PTRACE_SETFGREGS,.data = -3}, {
> -	PTRACE_SETFGREGS,.data = -4},
> +	PTRACE_SETFGREGS, .data = 0}, {
> +	PTRACE_SETFGREGS, .data = 1}, {
> +	PTRACE_SETFGREGS, .data = 2}, {
> +	PTRACE_SETFGREGS, .data = 3}, {
> +	PTRACE_SETFGREGS, .data = -1}, {
> +	PTRACE_SETFGREGS, .data = -2}, {
> +	PTRACE_SETFGREGS, .data = -3}, {
> +	PTRACE_SETFGREGS, .data = -4},
>  #endif
>  #if HAVE_DECL_PTRACE_GETSIGINFO
>  	{
> -	PTRACE_GETSIGINFO,.data = 0}, {
> -	PTRACE_GETSIGINFO,.data = 1}, {
> -	PTRACE_GETSIGINFO,.data = 2}, {
> -	PTRACE_GETSIGINFO,.data = 3}, {
> -	PTRACE_GETSIGINFO,.data = -1}, {
> -	PTRACE_GETSIGINFO,.data = -2}, {
> -	PTRACE_GETSIGINFO,.data = -3}, {
> -	PTRACE_GETSIGINFO,.data = -4},
> +	PTRACE_GETSIGINFO, .data = 0}, {
> +	PTRACE_GETSIGINFO, .data = 1}, {
> +	PTRACE_GETSIGINFO, .data = 2}, {
> +	PTRACE_GETSIGINFO, .data = 3}, {
> +	PTRACE_GETSIGINFO, .data = -1}, {
> +	PTRACE_GETSIGINFO, .data = -2}, {
> +	PTRACE_GETSIGINFO, .data = -3}, {
> +	PTRACE_GETSIGINFO, .data = -4},
>  #endif
>  #if HAVE_DECL_PTRACE_SETSIGINFO
>  	{
> -	PTRACE_SETSIGINFO,.data = 0}, {
> -	PTRACE_SETSIGINFO,.data = 1}, {
> -	PTRACE_SETSIGINFO,.data = 2}, {
> -	PTRACE_SETSIGINFO,.data = 3}, {
> -	PTRACE_SETSIGINFO,.data = -1}, {
> -	PTRACE_SETSIGINFO,.data = -2}, {
> -	PTRACE_SETSIGINFO,.data = -3}, {
> -	PTRACE_SETSIGINFO,.data = -4},
> +	PTRACE_SETSIGINFO, .data = 0}, {
> +	PTRACE_SETSIGINFO, .data = 1}, {
> +	PTRACE_SETSIGINFO, .data = 2}, {
> +	PTRACE_SETSIGINFO, .data = 3}, {
> +	PTRACE_SETSIGINFO, .data = -1}, {
> +	PTRACE_SETSIGINFO, .data = -2}, {
> +	PTRACE_SETSIGINFO, .data = -3}, {
> +	PTRACE_SETSIGINFO, .data = -4},
> +#endif
> +};
> +
> +#define SPT(x) [PTRACE_##x] = #x,
> +static char *strings[] = {
> +	SPT(TRACEME)
> +	SPT(PEEKTEXT)
> +	SPT(PEEKDATA)
> +	SPT(PEEKUSER)
> +	SPT(POKETEXT)
> +	SPT(POKEDATA)
> +	SPT(POKEUSER)
> +#ifdef PTRACE_GETREGS
> +	SPT(GETREGS)
> +#endif
> +#ifdef PTRACE_SETREGS
> +	SPT(SETREGS)
> +#endif
> +#ifdef PTRACE_GETSIGINFO
> +	SPT(GETSIGINFO)
> +#endif
> +#ifdef PTRACE_SETSIGINFO
> +	SPT(SETSIGINFO)
> +#endif
> +#ifdef PTRACE_GETFGREGS
> +	SPT(GETFGREGS)
> +#endif
> +#ifdef PTRACE_SETFGREGS
> +	SPT(SETFGREGS)
>  #endif
> +	SPT(KILL)
> +	SPT(SINGLESTEP)
>  };
>  
> -int TST_TOTAL = ARRAY_SIZE(test_cases);
> +static inline char *strptrace(int request)
> +{
> +	return strings[request];
> +}
> +
> +static void child(void)
> +{
> +	SAFE_PTRACE(PTRACE_TRACEME, 0, NULL, NULL);
> +	execl("/bin/echo", "/bin/echo", NULL);

This isn't guaranteed to exist.

-- 
Thank you,
Richard.

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

Re: [LTP] [PATCH v1 2/2] ptrace06: Refactor the test using new LTP API

[Petr Vorel]

Hi Wei,

note ptrace06 was not even been compiled. I tested that even in old API it
worked, thus I re-enable it in patch

https://patchwork.ozlabs.org/project/ltp/patch/20231128091524.340808-3-pvorel@suse.cz/
(part of https://patchwork.ozlabs.org/project/ltp/list/?series=384172&state=*)

> +++ b/testcases/kernel/syscalls/ptrace/ptrace06.c
> @@ -1,32 +1,31 @@
> +// SPDX-License-Identifier: GPL-2.0-only
Again:
// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> + * Copyright (c) 2008 Analog Devices Inc.
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
>   * check out-of-bound/unaligned addresses given to
Without missing blank line here the list will not be working.
>   *  - {PEEK,POKE}{DATA,TEXT,USER}
>   *  - {GET,SET}{,FG}REGS
>   *  - {GET,SET}SIGINFO
>   *
Why this blank line above?

> - * Copyright (c) 2008 Analog Devices Inc.
> - *
> - * Licensed under the GPL-2 or later
... because this:
>   */

>  #define _GNU_SOURCE
This might not be needed (needs to be verified in CI if also the oldest distros
does not need it).

>  /* this should be sizeof(struct user), but that info is only found
>   * in the kernel asm/user.h which is not exported to userspace.
>   */
> +
>  #if defined(__i386__)
>  #define SIZEOF_USER 284
>  #elif defined(__x86_64__)
> @@ -35,168 +34,213 @@
>  #define SIZEOF_USER 0x1000	/* just pick a big number */
>  #endif

I wonder if this SIZEOF_USER is valid. I haven't found what value they mean
(they talk about arch/*/include/asm/user*.h in kernel).

> -char *TCID = "ptrace06";
> -
>  struct test_case_t {
>  	int request;
>  	long addr;
>  	long data;
>  } test_cases[] = {
>  	{
> +	PTRACE_PEEKDATA, .addr = 0}, {
> +	PTRACE_PEEKDATA, .addr = 1}, {
IMHO This is ugly formatting, brackets shold be always on the same place.
.e.g 
{ PTRACE_PEEKDATA, .addr = 0},
{ PTRACE_PEEKDATA, .addr = 1},


> +	PTRACE_PEEKDATA, .addr = 2}, {
> +	PTRACE_PEEKDATA, .addr = 3}, {
> +	PTRACE_PEEKDATA, .addr = -1}, {
> +	PTRACE_PEEKDATA, .addr = -2}, {
...
> +static void child(void)
> +{
> +	SAFE_PTRACE(PTRACE_TRACEME, 0, NULL, NULL);
> +	execl("/bin/echo", "/bin/echo", NULL);
This will not work for AOSP (Android). Maybe adding ptrace06_child.c with very
simple code (printf or tst_res(TINFO) something, use TST_NO_DEFAULT_MAIN) would
be better.

> +	exit(0);
> +}

...

Kind regards,
Petr

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

[LTP] [PATCH v2 0/2] ptrace: Refactor

[Wei Gao via ltp]

Wei Gao (2):
  ptrace05: Refactor the test using new LTP API
  ptrace06: Refactor the test using new LTP API

 testcases/kernel/syscalls/ptrace/ptrace05.c   | 179 +++-------
 testcases/kernel/syscalls/ptrace/ptrace06.c   | 326 ++++++++++--------
 .../kernel/syscalls/ptrace/ptrace06_child.c   |  16 +
 3 files changed, 252 insertions(+), 269 deletions(-)
 create mode 100644 testcases/kernel/syscalls/ptrace/ptrace06_child.c

-- 
2.35.3


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

[LTP] [PATCH v2 1/2] ptrace05: Refactor the test using new LTP API

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/ptrace/ptrace05.c | 179 ++++++--------------
 1 file changed, 50 insertions(+), 129 deletions(-)

diff --git a/testcases/kernel/syscalls/ptrace/ptrace05.c b/testcases/kernel/syscalls/ptrace/ptrace05.c
index 541018393..267d5f9d2 100644
--- a/testcases/kernel/syscalls/ptrace/ptrace05.c
+++ b/testcases/kernel/syscalls/ptrace/ptrace05.c
@@ -1,178 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- ******************************************************************************
- *
- *   ptrace05 - an app which ptraces itself as per arbitrarily specified signals,
- *   over a user specified range.
- *
- *   Copyright (C) 2009, Ngie Cooper
- *
- *   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.
+ * Copyright (c) Linux Test Project, 2009-2019
+ * Copyright (C) 2009, Ngie Cooper
+ * Copyright (c) 2023 Wei Gao <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
  *
- *   You should have received a copy of the GNU General Public License along
- *   with this program; if not, write to the Free Software Foundation, Inc.,
- *   51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ * This test ptraces itself as per arbitrarily specified signals,
+ * over 0 to SIGRTMAX range.
  *
- ******************************************************************************
  */
 
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <signal.h>
-#include <errno.h>
-#include <libgen.h>
-#include <math.h>
 #include <stdlib.h>
-#include <stdio.h>
-#include <string.h>
-#include <unistd.h>
 #include <sys/ptrace.h>
-
-#include "test.h"
 #include "lapi/signal.h"
+#include "tst_test.h"
 
-char *TCID = "ptrace05";
-int TST_TOTAL = 0;
-
-int usage(const char *);
-
-int usage(const char *argv0)
-{
-	fprintf(stderr, "usage: %s [start-signum] [end-signum]\n", argv0);
-	return 1;
-}
-
-int main(int argc, char **argv)
+static void run(void)
 {
 
-	int end_signum = -1;
-	int signum;
-	int start_signum = -1;
+	int signum = 0;
 	int status;
 
 	pid_t child;
 
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	if (start_signum == -1) {
-		start_signum = 0;
-	}
-	if (end_signum == -1) {
-		end_signum = SIGRTMAX;
-	}
-
-	for (signum = start_signum; signum <= end_signum; signum++) {
-
+	for (signum = 0; signum <= SIGRTMAX; signum++) {
 		if (signum >= __SIGRTMIN && signum < SIGRTMIN)
 			continue;
 
-		switch (child = fork()) {
-		case -1:
-			tst_brkm(TBROK | TERRNO, NULL, "fork() failed");
-		case 0:
+		child = SAFE_FORK();
 
-			if (ptrace(PTRACE_TRACEME, 0, NULL, NULL) != -1) {
-				tst_resm(TINFO, "[child] Sending kill(.., %d)",
-					 signum);
-				if (kill(getpid(), signum) < 0) {
-					tst_resm(TINFO | TERRNO,
-						 "[child] kill(.., %d) failed.",
-						 signum);
-				}
+		if (child == 0) {
+			TEST(ptrace(PTRACE_TRACEME, 0, NULL, NULL));
+			if (TST_RET != -1) {
+				tst_res(TINFO, "[child] Sending kill(.., %d)",
+						signum);
+				SAFE_KILL(getpid(), signum);
 			} else {
-
-				/*
-				 * This won't increment the TST_COUNT var.
-				 * properly, but it'll show up as a failure
-				 * nonetheless.
-				 */
-				tst_resm(TFAIL | TERRNO,
-					 "Failed to ptrace(PTRACE_TRACEME, ...) "
-					 "properly");
-
+				tst_brk(TFAIL | TERRNO,
+						"Failed to ptrace(PTRACE_TRACEME, ...) "
+						"properly");
 			}
+
 			/* Shouldn't get here if signum == 0. */
 			exit((signum == 0 ? 0 : 2));
-			break;
-
-		default:
-
-			waitpid(child, &status, 0);
+		} else {
+			SAFE_WAITPID(child, &status, 0);
 
 			switch (signum) {
 			case 0:
 				if (WIFEXITED(status)
 				    && WEXITSTATUS(status) == 0) {
-					tst_resm(TPASS,
-						 "kill(.., 0) exited "
-						 "with 0, as expected.");
+					tst_res(TPASS,
+						 "kill(.., 0) exited with 0, as expected.");
 				} else {
-					tst_resm(TFAIL,
-						 "kill(.., 0) didn't exit "
-						 "with 0.");
+					tst_brk(TFAIL | TERRNO,
+						 "kill(.., 0) didn't exit with 0.");
 				}
 				break;
 			case SIGKILL:
 				if (WIFSIGNALED(status)) {
 					/* SIGKILL must be uncatchable. */
 					if (WTERMSIG(status) == SIGKILL) {
-						tst_resm(TPASS,
-							 "Killed with SIGKILL, "
-							 "as expected.");
+						tst_res(TPASS,
+							 "Killed with SIGKILL, as expected.");
 					} else {
-						tst_resm(TPASS,
-							 "Didn't die with "
-							 "SIGKILL (?!) ");
+						tst_brk(TFAIL | TERRNO,
+							 "Didn't die with SIGKILL (?!) ");
 					}
 				} else if (WIFEXITED(status)) {
-					tst_resm(TFAIL,
-						 "Exited unexpectedly instead "
-						 "of dying with SIGKILL.");
+					tst_res(TFAIL | TERRNO,
+						 "Exited unexpectedly instead of dying with SIGKILL.");
 				} else if (WIFSTOPPED(status)) {
-					tst_resm(TFAIL,
-						 "Stopped instead of dying "
-						 "with SIGKILL.");
+					tst_res(TFAIL | TERRNO,
+						 "Stopped instead of dying with SIGKILL.");
 				}
 				break;
 				/* All other processes should be stopped. */
 			default:
-				if (WIFSTOPPED(status)) {
-					tst_resm(TPASS, "Stopped as expected");
-				} else {
-					tst_resm(TFAIL, "Didn't stop as "
-						 "expected.");
-					if (kill(child, 0)) {
-						tst_resm(TINFO,
-							 "Is still alive!?");
-					} else if (WIFEXITED(status)) {
-						tst_resm(TINFO,
-							 "Exited normally");
-					} else if (WIFSIGNALED(status)) {
-						tst_resm(TINFO,
-							 "Was signaled with "
-							 "signum=%d",
-							 WTERMSIG(status));
-					}
-
-				}
-
+				if (WIFSTOPPED(status))
+					tst_res(TPASS, "Stopped as expected");
+				else
+					tst_res(TFAIL | TERRNO, "Didn't stop as expected.");
 				break;
-
 			}
 
 		}
-		/* Make sure the child dies a quick and painless death ... */
-		kill(child, 9);
-
+		if (signum != 0 && signum != SIGKILL)
+			SAFE_PTRACE(PTRACE_CONT, child, NULL, NULL);
 	}
-
-	tst_exit();
-
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.forks_child = 1,
+};
-- 
2.35.3


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

[LTP] [PATCH v2 2/2] ptrace06: Refactor the test using new LTP API

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/ptrace/ptrace06.c   | 326 ++++++++++--------
 .../kernel/syscalls/ptrace/ptrace06_child.c   |  16 +
 2 files changed, 202 insertions(+), 140 deletions(-)
 create mode 100644 testcases/kernel/syscalls/ptrace/ptrace06_child.c

diff --git a/testcases/kernel/syscalls/ptrace/ptrace06.c b/testcases/kernel/syscalls/ptrace/ptrace06.c
index a1db3bab8..225d9d466 100644
--- a/testcases/kernel/syscalls/ptrace/ptrace06.c
+++ b/testcases/kernel/syscalls/ptrace/ptrace06.c
@@ -1,29 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
+ * Copyright (c) 2008 Analog Devices Inc.
+ * Copyright (c) Linux Test Project, 2009-2022
+ * Copyright (c) 2023 Wei Gao <wegao@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
  * check out-of-bound/unaligned addresses given to
+ *
  *  - {PEEK,POKE}{DATA,TEXT,USER}
  *  - {GET,SET}{,FG}REGS
  *  - {GET,SET}SIGINFO
- *
- * Copyright (c) 2008 Analog Devices Inc.
- *
- * Licensed under the GPL-2 or later
  */
 
-#define _GNU_SOURCE
-
-#include <errno.h>
-#include <stdbool.h>
-#include <stdio.h>
 #include <stdlib.h>
-#include <unistd.h>
 #include <sys/ptrace.h>
-
-#include "test.h"
-#include "spawn_ptrace_child.h"
+#include "tst_test.h"
 
 /* this should be sizeof(struct user), but that info is only found
  * in the kernel asm/user.h which is not exported to userspace.
  */
+
 #if defined(__i386__)
 #define SIZEOF_USER 284
 #elif defined(__x86_64__)
@@ -32,168 +31,215 @@
 #define SIZEOF_USER 0x1000	/* just pick a big number */
 #endif
 
-char *TCID = "ptrace06";
-
-struct test_case_t {
+static struct test_case_t {
 	int request;
 	long addr;
 	long data;
 } test_cases[] = {
-	{
-	PTRACE_PEEKDATA,.addr = 0}, {
-	PTRACE_PEEKDATA,.addr = 1}, {
-	PTRACE_PEEKDATA,.addr = 2}, {
-	PTRACE_PEEKDATA,.addr = 3}, {
-	PTRACE_PEEKDATA,.addr = -1}, {
-	PTRACE_PEEKDATA,.addr = -2}, {
-	PTRACE_PEEKDATA,.addr = -3}, {
-	PTRACE_PEEKDATA,.addr = -4}, {
-	PTRACE_PEEKTEXT,.addr = 0}, {
-	PTRACE_PEEKTEXT,.addr = 1}, {
-	PTRACE_PEEKTEXT,.addr = 2}, {
-	PTRACE_PEEKTEXT,.addr = 3}, {
-	PTRACE_PEEKTEXT,.addr = -1}, {
-	PTRACE_PEEKTEXT,.addr = -2}, {
-	PTRACE_PEEKTEXT,.addr = -3}, {
-	PTRACE_PEEKTEXT,.addr = -4}, {
-	PTRACE_PEEKUSER,.addr = SIZEOF_USER + 1}, {
-	PTRACE_PEEKUSER,.addr = SIZEOF_USER + 2}, {
-	PTRACE_PEEKUSER,.addr = SIZEOF_USER + 3}, {
-	PTRACE_PEEKUSER,.addr = SIZEOF_USER + 4}, {
-	PTRACE_PEEKUSER,.addr = -1}, {
-	PTRACE_PEEKUSER,.addr = -2}, {
-	PTRACE_PEEKUSER,.addr = -3}, {
-	PTRACE_PEEKUSER,.addr = -4}, {
-	PTRACE_POKEDATA,.addr = 0}, {
-	PTRACE_POKEDATA,.addr = 1}, {
-	PTRACE_POKEDATA,.addr = 2}, {
-	PTRACE_POKEDATA,.addr = 3}, {
-	PTRACE_POKEDATA,.addr = -1}, {
-	PTRACE_POKEDATA,.addr = -2}, {
-	PTRACE_POKEDATA,.addr = -3}, {
-	PTRACE_POKEDATA,.addr = -4}, {
-	PTRACE_POKETEXT,.addr = 0}, {
-	PTRACE_POKETEXT,.addr = 1}, {
-	PTRACE_POKETEXT,.addr = 2}, {
-	PTRACE_POKETEXT,.addr = 3}, {
-	PTRACE_POKETEXT,.addr = -1}, {
-	PTRACE_POKETEXT,.addr = -2}, {
-	PTRACE_POKETEXT,.addr = -3}, {
-	PTRACE_POKETEXT,.addr = -4}, {
-	PTRACE_POKEUSER,.addr = SIZEOF_USER + 1}, {
-	PTRACE_POKEUSER,.addr = SIZEOF_USER + 2}, {
-	PTRACE_POKEUSER,.addr = SIZEOF_USER + 3}, {
-	PTRACE_POKEUSER,.addr = SIZEOF_USER + 4}, {
-	PTRACE_POKEUSER,.addr = -1}, {
-	PTRACE_POKEUSER,.addr = -2}, {
-	PTRACE_POKEUSER,.addr = -3}, {
-	PTRACE_POKEUSER,.addr = -4},
+	{PTRACE_PEEKDATA, .addr = 0},
+	{PTRACE_PEEKDATA, .addr = 1},
+	{PTRACE_PEEKDATA, .addr = 2},
+	{PTRACE_PEEKDATA, .addr = 3},
+	{PTRACE_PEEKDATA, .addr = -1},
+	{PTRACE_PEEKDATA, .addr = -2},
+	{PTRACE_PEEKDATA, .addr = -3},
+	{PTRACE_PEEKDATA, .addr = -4},
+	{PTRACE_PEEKTEXT, .addr = 0},
+	{PTRACE_PEEKTEXT, .addr = 1},
+	{PTRACE_PEEKTEXT, .addr = 2},
+	{PTRACE_PEEKTEXT, .addr = 3},
+	{PTRACE_PEEKTEXT, .addr = -1},
+	{PTRACE_PEEKTEXT, .addr = -2},
+	{PTRACE_PEEKTEXT, .addr = -3},
+	{PTRACE_PEEKTEXT, .addr = -4},
+	{PTRACE_PEEKUSER, .addr = SIZEOF_USER + 1},
+	{PTRACE_PEEKUSER, .addr = SIZEOF_USER + 2},
+	{PTRACE_PEEKUSER, .addr = SIZEOF_USER + 3},
+	{PTRACE_PEEKUSER, .addr = SIZEOF_USER + 4},
+	{PTRACE_PEEKUSER, .addr = -1},
+	{PTRACE_PEEKUSER, .addr = -2},
+	{PTRACE_PEEKUSER, .addr = -3},
+	{PTRACE_PEEKUSER, .addr = -4},
+	{PTRACE_POKEDATA, .addr = 0},
+	{PTRACE_POKEDATA, .addr = 1},
+	{PTRACE_POKEDATA, .addr = 2},
+	{PTRACE_POKEDATA, .addr = 3},
+	{PTRACE_POKEDATA, .addr = -1},
+	{PTRACE_POKEDATA, .addr = -2},
+	{PTRACE_POKEDATA, .addr = -3},
+	{PTRACE_POKEDATA, .addr = -4},
+	{PTRACE_POKETEXT, .addr = 0},
+	{PTRACE_POKETEXT, .addr = 1},
+	{PTRACE_POKETEXT, .addr = 2},
+	{PTRACE_POKETEXT, .addr = 3},
+	{PTRACE_POKETEXT, .addr = -1},
+	{PTRACE_POKETEXT, .addr = -2},
+	{PTRACE_POKETEXT, .addr = -3},
+	{PTRACE_POKETEXT, .addr = -4},
+	{PTRACE_POKEUSER, .addr = SIZEOF_USER + 1},
+	{PTRACE_POKEUSER, .addr = SIZEOF_USER + 2},
+	{PTRACE_POKEUSER, .addr = SIZEOF_USER + 3},
+	{PTRACE_POKEUSER, .addr = SIZEOF_USER + 4},
+	{PTRACE_POKEUSER, .addr = -1},
+	{PTRACE_POKEUSER, .addr = -2},
+	{PTRACE_POKEUSER, .addr = -3},
+	{PTRACE_POKEUSER, .addr = -4},
 #ifdef PTRACE_GETREGS
-	{
-	PTRACE_GETREGS,.data = 0}, {
-	PTRACE_GETREGS,.data = 1}, {
-	PTRACE_GETREGS,.data = 2}, {
-	PTRACE_GETREGS,.data = 3}, {
-	PTRACE_GETREGS,.data = -1}, {
-	PTRACE_GETREGS,.data = -2}, {
-	PTRACE_GETREGS,.data = -3}, {
-	PTRACE_GETREGS,.data = -4},
+	{PTRACE_GETREGS, .data = 0},
+	{PTRACE_GETREGS, .data = 1},
+	{PTRACE_GETREGS, .data = 2},
+	{PTRACE_GETREGS, .data = 3},
+	{PTRACE_GETREGS, .data = -1},
+	{PTRACE_GETREGS, .data = -2},
+	{PTRACE_GETREGS, .data = -3},
+	{PTRACE_GETREGS, .data = -4},
 #endif
 #ifdef PTRACE_GETFGREGS
-	{
-	PTRACE_GETFGREGS,.data = 0}, {
-	PTRACE_GETFGREGS,.data = 1}, {
-	PTRACE_GETFGREGS,.data = 2}, {
-	PTRACE_GETFGREGS,.data = 3}, {
-	PTRACE_GETFGREGS,.data = -1}, {
-	PTRACE_GETFGREGS,.data = -2}, {
-	PTRACE_GETFGREGS,.data = -3}, {
-	PTRACE_GETFGREGS,.data = -4},
+	{PTRACE_GETFGREGS, .data = 0},
+	{PTRACE_GETFGREGS, .data = 1},
+	{PTRACE_GETFGREGS, .data = 2},
+	{PTRACE_GETFGREGS, .data = 3},
+	{PTRACE_GETFGREGS, .data = -1},
+	{PTRACE_GETFGREGS, .data = -2},
+	{PTRACE_GETFGREGS, .data = -3},
+	{PTRACE_GETFGREGS, .data = -4},
 #endif
 #ifdef PTRACE_SETREGS
-	{
-	PTRACE_SETREGS,.data = 0}, {
-	PTRACE_SETREGS,.data = 1}, {
-	PTRACE_SETREGS,.data = 2}, {
-	PTRACE_SETREGS,.data = 3}, {
-	PTRACE_SETREGS,.data = -1}, {
-	PTRACE_SETREGS,.data = -2}, {
-	PTRACE_SETREGS,.data = -3}, {
-	PTRACE_SETREGS,.data = -4},
+	{PTRACE_SETREGS, .data = 0},
+	{PTRACE_SETREGS, .data = 1},
+	{PTRACE_SETREGS, .data = 2},
+	{PTRACE_SETREGS, .data = 3},
+	{PTRACE_SETREGS, .data = -1},
+	{PTRACE_SETREGS, .data = -2},
+	{PTRACE_SETREGS, .data = -3},
+	{PTRACE_SETREGS, .data = -4},
 #endif
 #ifdef PTRACE_SETFGREGS
-	{
-	PTRACE_SETFGREGS,.data = 0}, {
-	PTRACE_SETFGREGS,.data = 1}, {
-	PTRACE_SETFGREGS,.data = 2}, {
-	PTRACE_SETFGREGS,.data = 3}, {
-	PTRACE_SETFGREGS,.data = -1}, {
-	PTRACE_SETFGREGS,.data = -2}, {
-	PTRACE_SETFGREGS,.data = -3}, {
-	PTRACE_SETFGREGS,.data = -4},
+	{PTRACE_SETFGREGS, .data = 0},
+	{PTRACE_SETFGREGS, .data = 1},
+	{PTRACE_SETFGREGS, .data = 2},
+	{PTRACE_SETFGREGS, .data = 3},
+	{PTRACE_SETFGREGS, .data = -1},
+	{PTRACE_SETFGREGS, .data = -2},
+	{PTRACE_SETFGREGS, .data = -3},
+	{PTRACE_SETFGREGS, .data = -4},
 #endif
 #if HAVE_DECL_PTRACE_GETSIGINFO
-	{
-	PTRACE_GETSIGINFO,.data = 0}, {
-	PTRACE_GETSIGINFO,.data = 1}, {
-	PTRACE_GETSIGINFO,.data = 2}, {
-	PTRACE_GETSIGINFO,.data = 3}, {
-	PTRACE_GETSIGINFO,.data = -1}, {
-	PTRACE_GETSIGINFO,.data = -2}, {
-	PTRACE_GETSIGINFO,.data = -3}, {
-	PTRACE_GETSIGINFO,.data = -4},
+	{PTRACE_GETSIGINFO, .data = 0},
+	{PTRACE_GETSIGINFO, .data = 1},
+	{PTRACE_GETSIGINFO, .data = 2},
+	{PTRACE_GETSIGINFO, .data = 3},
+	{PTRACE_GETSIGINFO, .data = -1},
+	{PTRACE_GETSIGINFO, .data = -2},
+	{PTRACE_GETSIGINFO, .data = -3},
+	{PTRACE_GETSIGINFO, .data = -4},
 #endif
 #if HAVE_DECL_PTRACE_SETSIGINFO
-	{
-	PTRACE_SETSIGINFO,.data = 0}, {
-	PTRACE_SETSIGINFO,.data = 1}, {
-	PTRACE_SETSIGINFO,.data = 2}, {
-	PTRACE_SETSIGINFO,.data = 3}, {
-	PTRACE_SETSIGINFO,.data = -1}, {
-	PTRACE_SETSIGINFO,.data = -2}, {
-	PTRACE_SETSIGINFO,.data = -3}, {
-	PTRACE_SETSIGINFO,.data = -4},
+	{PTRACE_SETSIGINFO, .data = 0},
+	{PTRACE_SETSIGINFO, .data = 1},
+	{PTRACE_SETSIGINFO, .data = 2},
+	{PTRACE_SETSIGINFO, .data = 3},
+	{PTRACE_SETSIGINFO, .data = -1},
+	{PTRACE_SETSIGINFO, .data = -2},
+	{PTRACE_SETSIGINFO, .data = -3},
+	{PTRACE_SETSIGINFO, .data = -4},
 #endif
 };
 
-int TST_TOTAL = ARRAY_SIZE(test_cases);
+#define SPT(x) [PTRACE_##x] = #x,
+static char *strings[] = {
+	SPT(TRACEME)
+	SPT(PEEKTEXT)
+	SPT(PEEKDATA)
+	SPT(PEEKUSER)
+	SPT(POKETEXT)
+	SPT(POKEDATA)
+	SPT(POKEUSER)
+#ifdef PTRACE_GETREGS
+	SPT(GETREGS)
+#endif
+#ifdef PTRACE_SETREGS
+	SPT(SETREGS)
+#endif
+#ifdef PTRACE_GETSIGINFO
+	SPT(GETSIGINFO)
+#endif
+#ifdef PTRACE_SETSIGINFO
+	SPT(SETSIGINFO)
+#endif
+#ifdef PTRACE_GETFGREGS
+	SPT(GETFGREGS)
+#endif
+#ifdef PTRACE_SETFGREGS
+	SPT(SETFGREGS)
+#endif
+	SPT(KILL)
+	SPT(SINGLESTEP)
+};
+
+static inline char *strptrace(int request)
+{
+	return strings[request];
+}
+
+static void child(void)
+{
 
-int main(int argc, char *argv[])
+	char path[512];
+
+	SAFE_PTRACE(PTRACE_TRACEME, 0, NULL, NULL);
+
+	if (tst_get_path("ptrace06_child", path, sizeof(path)))
+		tst_brk(TCONF, "Couldn't find ptrace06_child in $PATH");
+
+	TEST(execl(path, "ptrace06_child", "", NULL));
+	tst_brk(TFAIL | TTERRNO,
+			"Failed to execute execl01_child");
+	exit(0);
+}
+
+static void run(void)
 {
 	size_t i;
-	long ret;
-	int saved_errno;
+	int pid;
+	int status;
 
-	tst_parse_opts(argc, argv, NULL, NULL);
+	pid = SAFE_FORK();
 
-	make_a_baby(argc, argv);
+	if (!pid)
+		child();
+
+	SAFE_WAIT(&status);
+
+	if (!WIFSTOPPED(status))
+		tst_brk(TBROK, "child %d was not stopped", pid);
 
 	for (i = 0; i < ARRAY_SIZE(test_cases); ++i) {
 		struct test_case_t *tc = &test_cases[i];
 
-		errno = 0;
-		ret =
-		    ptrace(tc->request, pid, (void *)tc->addr,
-			   (void *)tc->data);
-		saved_errno = errno;
-		if (ret != -1)
-			tst_resm(TFAIL,
+		TEST(ptrace(tc->request, pid, (void *)tc->addr,
+					(void *)tc->data));
+		if (TST_RET != -1)
+			tst_brk(TFAIL | TERRNO,
 				 "ptrace(%s, ..., %li, %li) returned %li instead of -1",
 				 strptrace(tc->request), tc->addr, tc->data,
-				 ret);
-		else if (saved_errno != EIO && saved_errno != EFAULT)
-			tst_resm(TFAIL,
+				 TST_RET);
+		else if (TST_ERR != EIO && TST_ERR != EFAULT)
+			tst_brk(TFAIL | TERRNO,
 				 "ptrace(%s, ..., %li, %li) expected errno EIO or EFAULT; actual: %i (%s)",
 				 strptrace(tc->request), tc->addr, tc->data,
-				 saved_errno, strerror(saved_errno));
+				 TST_ERR, strerror(TST_ERR));
 		else
-			tst_resm(TPASS,
+			tst_res(TPASS,
 				 "ptrace(%s, ..., %li, %li) failed as expected",
 				 strptrace(tc->request), tc->addr, tc->data);
 	}
 
-	/* hopefully this worked */
-	ptrace(PTRACE_KILL, pid, NULL, NULL);
+	SAFE_PTRACE(PTRACE_CONT, pid, NULL, NULL);
 
-	tst_exit();
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.forks_child = 1,
+};
diff --git a/testcases/kernel/syscalls/ptrace/ptrace06_child.c b/testcases/kernel/syscalls/ptrace/ptrace06_child.c
new file mode 100644
index 000000000..350b0e8df
--- /dev/null
+++ b/testcases/kernel/syscalls/ptrace/ptrace06_child.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2018 Linux Test Project
+ * Copyright (C) 2015 Cyril Hrubis chrubis@suse.cz
+ */
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+
+int main(int argc, char *argv[])
+{
+
+	tst_res(TPASS, "%s executed", argv[0]);
+
+	return 0;
+}
-- 
2.35.3


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

Re: [LTP] [PATCH v1 2/2] ptrace06: Refactor the test using new LTP API

[Wei Gao via ltp]

On Tue, Nov 28, 2023 at 10:51:49AM +0100, Petr Vorel wrote:
> Hi Wei,
> 
> >  #if defined(__i386__)
> >  #define SIZEOF_USER 284
> >  #elif defined(__x86_64__)
> > @@ -35,168 +34,213 @@
> >  #define SIZEOF_USER 0x1000	/* just pick a big number */
> >  #endif
> 
> I wonder if this SIZEOF_USER is valid. I haven't found what value they mean
> (they talk about arch/*/include/asm/user*.h in kernel).
> 
Yes, i also not found SIZEOF_USER, but i suppose the risk is low since the kernel
code for struct user is quite stable and the latest update is 2008.3.

Thanks.
Regards
Gao Wei

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

Re: [LTP] [PATCH v2 1/2] ptrace05: Refactor the test using new LTP API

[Andrea Cervesato via ltp]

Hi!

On 12/1/23 01:59, Wei Gao via ltp wrote:
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
> testcases/kernel/syscalls/ptrace/ptrace05.c | 179 ++++++--------------
> 1 file changed, 50 insertions(+), 129 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/ptrace/ptrace05.c 
> b/testcases/kernel/syscalls/ptrace/ptrace05.c
> index 541018393..267d5f9d2 100644
> --- a/testcases/kernel/syscalls/ptrace/ptrace05.c
> +++ b/testcases/kernel/syscalls/ptrace/ptrace05.c
> @@ -1,178 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> /*
> - 
> ******************************************************************************
> - *
> - *   ptrace05 - an app which ptraces itself as per arbitrarily 
> specified signals,
> - *   over a user specified range.
> - *
> - *   Copyright (C) 2009, Ngie Cooper
> - *
> - *   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.
> + * Copyright (c) Linux Test Project, 2009-2019
> + * Copyright (C) 2009, Ngie Cooper
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
>  *
> - *   You should have received a copy of the GNU General Public 
> License along
> - *   with this program; if not, write to the Free Software 
> Foundation, Inc.,
> - *   51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + * This test ptraces itself as per arbitrarily specified signals,
> + * over 0 to SIGRTMAX range.
>  *
> - 
> ******************************************************************************
>  */
>
> -#include <sys/types.h>
> -#include <sys/wait.h>
> -#include <signal.h>
> -#include <errno.h>
> -#include <libgen.h>
> -#include <math.h>
> #include <stdlib.h>
> -#include <stdio.h>
> -#include <string.h>
> -#include <unistd.h>
> #include <sys/ptrace.h>
> -
> -#include "test.h"
> #include "lapi/signal.h"
> +#include "tst_test.h"
>
> -char *TCID = "ptrace05";
> -int TST_TOTAL = 0;
> -
> -int usage(const char *);
> -
> -int usage(const char *argv0)
> -{
> -    fprintf(stderr, "usage: %s [start-signum] [end-signum]\n", argv0);
> -    return 1;
> -}
> -
> -int main(int argc, char **argv)
> +static void run(void)
> {
>
new line to remove
> -    int end_signum = -1;
> -    int signum;
> -    int start_signum = -1;
> +    int signum = 0;
>     int status;
>
new line to remove
>     pid_t child;
>
> -    tst_parse_opts(argc, argv, NULL, NULL);
> -
> -    if (start_signum == -1) {
> -        start_signum = 0;
> -    }
> -    if (end_signum == -1) {
> -        end_signum = SIGRTMAX;
> -    }
> -
> -    for (signum = start_signum; signum <= end_signum; signum++) {
> -
> +    for (signum = 0; signum <= SIGRTMAX; signum++) {
>         if (signum >= __SIGRTMIN && signum < SIGRTMIN)
>             continue;
>
> -        switch (child = fork()) {
> -        case -1:
> -            tst_brkm(TBROK | TERRNO, NULL, "fork() failed");
> -        case 0:
> +        child = SAFE_FORK();
>
> -            if (ptrace(PTRACE_TRACEME, 0, NULL, NULL) != -1) {
> -                tst_resm(TINFO, "[child] Sending kill(.., %d)",
> -                     signum);
> -                if (kill(getpid(), signum) < 0) {
> -                    tst_resm(TINFO | TERRNO,
> -                         "[child] kill(.., %d) failed.",
> -                         signum);
> -                }
> +        if (child == 0) {
> +            TEST(ptrace(PTRACE_TRACEME, 0, NULL, NULL));
> +            if (TST_RET != -1) {
> +                tst_res(TINFO, "[child] Sending kill(.., %d)",
> +                        signum);
> +                SAFE_KILL(getpid(), signum);
>             } else {
> -
> -                /*
> -                 * This won't increment the TST_COUNT var.
> -                 * properly, but it'll show up as a failure
> -                 * nonetheless.
> -                 */
> -                tst_resm(TFAIL | TERRNO,
> -                     "Failed to ptrace(PTRACE_TRACEME, ...) "
> -                     "properly");
> -
> +                tst_brk(TFAIL | TERRNO,
> +                        "Failed to ptrace(PTRACE_TRACEME, ...) "
> +                        "properly");
>             }
> +
>             /* Shouldn't get here if signum == 0. */
>             exit((signum == 0 ? 0 : 2));
> -            break;
> -
> -        default:
> -
> -            waitpid(child, &status, 0);
> +        } else {

I would remove the "else" statement, because we don't really need it 
once we handle the child and it creates more indentation.

> + SAFE_WAITPID(child, &status, 0);
>
>             switch (signum) {
>             case 0:
>                 if (WIFEXITED(status)
>                     && WEXITSTATUS(status) == 0) {
> -                    tst_resm(TPASS,
> -                         "kill(.., 0) exited "
> -                         "with 0, as expected.");
> +                    tst_res(TPASS,
> +                         "kill(.., 0) exited with 0, as expected.");
>                 } else {
> -                    tst_resm(TFAIL,
> -                         "kill(.., 0) didn't exit "
> -                         "with 0.");
> +                    tst_brk(TFAIL | TERRNO,
> +                         "kill(.., 0) didn't exit with 0.");
>                 }
>                 break;
>             case SIGKILL:
>                 if (WIFSIGNALED(status)) {
>                     /* SIGKILL must be uncatchable. */
>                     if (WTERMSIG(status) == SIGKILL) {
> -                        tst_resm(TPASS,
> -                             "Killed with SIGKILL, "
> -                             "as expected.");
> +                        tst_res(TPASS,
> +                             "Killed with SIGKILL, as expected.");
>                     } else {
> -                        tst_resm(TPASS,
> -                             "Didn't die with "
> -                             "SIGKILL (?!) ");
> +                        tst_brk(TFAIL | TERRNO,
> +                             "Didn't die with SIGKILL (?!) ");
>                     }
>                 } else if (WIFEXITED(status)) {
> -                    tst_resm(TFAIL,
> -                         "Exited unexpectedly instead "
> -                         "of dying with SIGKILL.");
> +                    tst_res(TFAIL | TERRNO,
> +                         "Exited unexpectedly instead of dying with 
> SIGKILL.");
>                 } else if (WIFSTOPPED(status)) {
> -                    tst_resm(TFAIL,
> -                         "Stopped instead of dying "
> -                         "with SIGKILL.");
> +                    tst_res(TFAIL | TERRNO,
> +                         "Stopped instead of dying with SIGKILL.");
>                 }
>                 break;
>                 /* All other processes should be stopped. */
>             default:
> -                if (WIFSTOPPED(status)) {
> -                    tst_resm(TPASS, "Stopped as expected");
> -                } else {
> -                    tst_resm(TFAIL, "Didn't stop as "
> -                         "expected.");
> -                    if (kill(child, 0)) {
> -                        tst_resm(TINFO,
> -                             "Is still alive!?");
> -                    } else if (WIFEXITED(status)) {
> -                        tst_resm(TINFO,
> -                             "Exited normally");
> -                    } else if (WIFSIGNALED(status)) {
> -                        tst_resm(TINFO,
> -                             "Was signaled with "
> -                             "signum=%d",
> -                             WTERMSIG(status));
> -                    }
> -
> -                }
> -
> +                if (WIFSTOPPED(status))
> +                    tst_res(TPASS, "Stopped as expected");
> +                else
> +                    tst_res(TFAIL | TERRNO, "Didn't stop as expected.");
>                 break;
> -
>             }
>
new line to remove :-) I know it can be boring, but please keep new line 
where it's needed.
>         }
> -        /* Make sure the child dies a quick and painless death ... */
> -        kill(child, 9);
> -
new line to add
> +        if (signum != 0 && signum != SIGKILL)
> +            SAFE_PTRACE(PTRACE_CONT, child, NULL, NULL);
>     }
> -
> -    tst_exit();
> -
> }
> +
> +static struct tst_test test = {
> +    .test_all = run,
> +    .forks_child = 1,
> +};

Besides a few stylish things, it's fine.

Regards,
Andrea Cervesato


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

Re: [LTP] [PATCH v2 2/2] ptrace06: Refactor the test using new LTP API

[Andrea Cervesato via ltp]

Hi!

On 12/1/23 01:59, Wei Gao via ltp wrote:
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
> testcases/kernel/syscalls/ptrace/ptrace06.c   | 326 ++++++++++--------
> .../kernel/syscalls/ptrace/ptrace06_child.c   |  16 +
> 2 files changed, 202 insertions(+), 140 deletions(-)
> create mode 100644 testcases/kernel/syscalls/ptrace/ptrace06_child.c
>
> diff --git a/testcases/kernel/syscalls/ptrace/ptrace06.c 
> b/testcases/kernel/syscalls/ptrace/ptrace06.c
> index a1db3bab8..225d9d466 100644
> --- a/testcases/kernel/syscalls/ptrace/ptrace06.c
> +++ b/testcases/kernel/syscalls/ptrace/ptrace06.c
> @@ -1,29 +1,28 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> /*
> + * Copyright (c) 2008 Analog Devices Inc.
> + * Copyright (c) Linux Test Project, 2009-2022
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
>  * check out-of-bound/unaligned addresses given to
> + *
>  *  - {PEEK,POKE}{DATA,TEXT,USER}
>  *  - {GET,SET}{,FG}REGS
>  *  - {GET,SET}SIGINFO
> - *
> - * Copyright (c) 2008 Analog Devices Inc.
> - *
> - * Licensed under the GPL-2 or later
>  */
>
> -#define _GNU_SOURCE
> -
> -#include <errno.h>
> -#include <stdbool.h>
> -#include <stdio.h>
> #include <stdlib.h>
> -#include <unistd.h>
> #include <sys/ptrace.h>
> -
> -#include "test.h"
> -#include "spawn_ptrace_child.h"
> +#include "tst_test.h"
>
> /* this should be sizeof(struct user), but that info is only found
>  * in the kernel asm/user.h which is not exported to userspace.
>  */
> +
> #if defined(__i386__)
> #define SIZEOF_USER 284
> #elif defined(__x86_64__)
> @@ -32,168 +31,215 @@
> #define SIZEOF_USER 0x1000    /* just pick a big number */
> #endif
>
> -char *TCID = "ptrace06";
> -
> -struct test_case_t {
> +static struct test_case_t {
>     int request;
>     long addr;
>     long data;
> } test_cases[] = {
> -    {
> -    PTRACE_PEEKDATA,.addr = 0}, {
> -    PTRACE_PEEKDATA,.addr = 1}, {
> -    PTRACE_PEEKDATA,.addr = 2}, {
> -    PTRACE_PEEKDATA,.addr = 3}, {
> -    PTRACE_PEEKDATA,.addr = -1}, {
> -    PTRACE_PEEKDATA,.addr = -2}, {
> -    PTRACE_PEEKDATA,.addr = -3}, {
> -    PTRACE_PEEKDATA,.addr = -4}, {
> -    PTRACE_PEEKTEXT,.addr = 0}, {
> -    PTRACE_PEEKTEXT,.addr = 1}, {
> -    PTRACE_PEEKTEXT,.addr = 2}, {
> -    PTRACE_PEEKTEXT,.addr = 3}, {
> -    PTRACE_PEEKTEXT,.addr = -1}, {
> -    PTRACE_PEEKTEXT,.addr = -2}, {
> -    PTRACE_PEEKTEXT,.addr = -3}, {
> -    PTRACE_PEEKTEXT,.addr = -4}, {
> -    PTRACE_PEEKUSER,.addr = SIZEOF_USER + 1}, {
> -    PTRACE_PEEKUSER,.addr = SIZEOF_USER + 2}, {
> -    PTRACE_PEEKUSER,.addr = SIZEOF_USER + 3}, {
> -    PTRACE_PEEKUSER,.addr = SIZEOF_USER + 4}, {
> -    PTRACE_PEEKUSER,.addr = -1}, {
> -    PTRACE_PEEKUSER,.addr = -2}, {
> -    PTRACE_PEEKUSER,.addr = -3}, {
> -    PTRACE_PEEKUSER,.addr = -4}, {
> -    PTRACE_POKEDATA,.addr = 0}, {
> -    PTRACE_POKEDATA,.addr = 1}, {
> -    PTRACE_POKEDATA,.addr = 2}, {
> -    PTRACE_POKEDATA,.addr = 3}, {
> -    PTRACE_POKEDATA,.addr = -1}, {
> -    PTRACE_POKEDATA,.addr = -2}, {
> -    PTRACE_POKEDATA,.addr = -3}, {
> -    PTRACE_POKEDATA,.addr = -4}, {
> -    PTRACE_POKETEXT,.addr = 0}, {
> -    PTRACE_POKETEXT,.addr = 1}, {
> -    PTRACE_POKETEXT,.addr = 2}, {
> -    PTRACE_POKETEXT,.addr = 3}, {
> -    PTRACE_POKETEXT,.addr = -1}, {
> -    PTRACE_POKETEXT,.addr = -2}, {
> -    PTRACE_POKETEXT,.addr = -3}, {
> -    PTRACE_POKETEXT,.addr = -4}, {
> -    PTRACE_POKEUSER,.addr = SIZEOF_USER + 1}, {
> -    PTRACE_POKEUSER,.addr = SIZEOF_USER + 2}, {
> -    PTRACE_POKEUSER,.addr = SIZEOF_USER + 3}, {
> -    PTRACE_POKEUSER,.addr = SIZEOF_USER + 4}, {
> -    PTRACE_POKEUSER,.addr = -1}, {
> -    PTRACE_POKEUSER,.addr = -2}, {
> -    PTRACE_POKEUSER,.addr = -3}, {
> -    PTRACE_POKEUSER,.addr = -4},
> +    {PTRACE_PEEKDATA, .addr = 0},
> +    {PTRACE_PEEKDATA, .addr = 1},
> +    {PTRACE_PEEKDATA, .addr = 2},
> +    {PTRACE_PEEKDATA, .addr = 3},
> +    {PTRACE_PEEKDATA, .addr = -1},
> +    {PTRACE_PEEKDATA, .addr = -2},
> +    {PTRACE_PEEKDATA, .addr = -3},
> +    {PTRACE_PEEKDATA, .addr = -4},
> +    {PTRACE_PEEKTEXT, .addr = 0},
> +    {PTRACE_PEEKTEXT, .addr = 1},
> +    {PTRACE_PEEKTEXT, .addr = 2},
> +    {PTRACE_PEEKTEXT, .addr = 3},
> +    {PTRACE_PEEKTEXT, .addr = -1},
> +    {PTRACE_PEEKTEXT, .addr = -2},
> +    {PTRACE_PEEKTEXT, .addr = -3},
> +    {PTRACE_PEEKTEXT, .addr = -4},
> +    {PTRACE_PEEKUSER, .addr = SIZEOF_USER + 1},
> +    {PTRACE_PEEKUSER, .addr = SIZEOF_USER + 2},
> +    {PTRACE_PEEKUSER, .addr = SIZEOF_USER + 3},
> +    {PTRACE_PEEKUSER, .addr = SIZEOF_USER + 4},
> +    {PTRACE_PEEKUSER, .addr = -1},
> +    {PTRACE_PEEKUSER, .addr = -2},
> +    {PTRACE_PEEKUSER, .addr = -3},
> +    {PTRACE_PEEKUSER, .addr = -4},
> +    {PTRACE_POKEDATA, .addr = 0},
> +    {PTRACE_POKEDATA, .addr = 1},
> +    {PTRACE_POKEDATA, .addr = 2},
> +    {PTRACE_POKEDATA, .addr = 3},
> +    {PTRACE_POKEDATA, .addr = -1},
> +    {PTRACE_POKEDATA, .addr = -2},
> +    {PTRACE_POKEDATA, .addr = -3},
> +    {PTRACE_POKEDATA, .addr = -4},
> +    {PTRACE_POKETEXT, .addr = 0},
> +    {PTRACE_POKETEXT, .addr = 1},
> +    {PTRACE_POKETEXT, .addr = 2},
> +    {PTRACE_POKETEXT, .addr = 3},
> +    {PTRACE_POKETEXT, .addr = -1},
> +    {PTRACE_POKETEXT, .addr = -2},
> +    {PTRACE_POKETEXT, .addr = -3},
> +    {PTRACE_POKETEXT, .addr = -4},
> +    {PTRACE_POKEUSER, .addr = SIZEOF_USER + 1},
> +    {PTRACE_POKEUSER, .addr = SIZEOF_USER + 2},
> +    {PTRACE_POKEUSER, .addr = SIZEOF_USER + 3},
> +    {PTRACE_POKEUSER, .addr = SIZEOF_USER + 4},
> +    {PTRACE_POKEUSER, .addr = -1},
> +    {PTRACE_POKEUSER, .addr = -2},
> +    {PTRACE_POKEUSER, .addr = -3},
> +    {PTRACE_POKEUSER, .addr = -4},
> #ifdef PTRACE_GETREGS
> -    {
> -    PTRACE_GETREGS,.data = 0}, {
> -    PTRACE_GETREGS,.data = 1}, {
> -    PTRACE_GETREGS,.data = 2}, {
> -    PTRACE_GETREGS,.data = 3}, {
> -    PTRACE_GETREGS,.data = -1}, {
> -    PTRACE_GETREGS,.data = -2}, {
> -    PTRACE_GETREGS,.data = -3}, {
> -    PTRACE_GETREGS,.data = -4},
> +    {PTRACE_GETREGS, .data = 0},
> +    {PTRACE_GETREGS, .data = 1},
> +    {PTRACE_GETREGS, .data = 2},
> +    {PTRACE_GETREGS, .data = 3},
> +    {PTRACE_GETREGS, .data = -1},
> +    {PTRACE_GETREGS, .data = -2},
> +    {PTRACE_GETREGS, .data = -3},
> +    {PTRACE_GETREGS, .data = -4},
> #endif
> #ifdef PTRACE_GETFGREGS
> -    {
> -    PTRACE_GETFGREGS,.data = 0}, {
> -    PTRACE_GETFGREGS,.data = 1}, {
> -    PTRACE_GETFGREGS,.data = 2}, {
> -    PTRACE_GETFGREGS,.data = 3}, {
> -    PTRACE_GETFGREGS,.data = -1}, {
> -    PTRACE_GETFGREGS,.data = -2}, {
> -    PTRACE_GETFGREGS,.data = -3}, {
> -    PTRACE_GETFGREGS,.data = -4},
> +    {PTRACE_GETFGREGS, .data = 0},
> +    {PTRACE_GETFGREGS, .data = 1},
> +    {PTRACE_GETFGREGS, .data = 2},
> +    {PTRACE_GETFGREGS, .data = 3},
> +    {PTRACE_GETFGREGS, .data = -1},
> +    {PTRACE_GETFGREGS, .data = -2},
> +    {PTRACE_GETFGREGS, .data = -3},
> +    {PTRACE_GETFGREGS, .data = -4},
> #endif
> #ifdef PTRACE_SETREGS
> -    {
> -    PTRACE_SETREGS,.data = 0}, {
> -    PTRACE_SETREGS,.data = 1}, {
> -    PTRACE_SETREGS,.data = 2}, {
> -    PTRACE_SETREGS,.data = 3}, {
> -    PTRACE_SETREGS,.data = -1}, {
> -    PTRACE_SETREGS,.data = -2}, {
> -    PTRACE_SETREGS,.data = -3}, {
> -    PTRACE_SETREGS,.data = -4},
> +    {PTRACE_SETREGS, .data = 0},
> +    {PTRACE_SETREGS, .data = 1},
> +    {PTRACE_SETREGS, .data = 2},
> +    {PTRACE_SETREGS, .data = 3},
> +    {PTRACE_SETREGS, .data = -1},
> +    {PTRACE_SETREGS, .data = -2},
> +    {PTRACE_SETREGS, .data = -3},
> +    {PTRACE_SETREGS, .data = -4},
> #endif
> #ifdef PTRACE_SETFGREGS
> -    {
> -    PTRACE_SETFGREGS,.data = 0}, {
> -    PTRACE_SETFGREGS,.data = 1}, {
> -    PTRACE_SETFGREGS,.data = 2}, {
> -    PTRACE_SETFGREGS,.data = 3}, {
> -    PTRACE_SETFGREGS,.data = -1}, {
> -    PTRACE_SETFGREGS,.data = -2}, {
> -    PTRACE_SETFGREGS,.data = -3}, {
> -    PTRACE_SETFGREGS,.data = -4},
> +    {PTRACE_SETFGREGS, .data = 0},
> +    {PTRACE_SETFGREGS, .data = 1},
> +    {PTRACE_SETFGREGS, .data = 2},
> +    {PTRACE_SETFGREGS, .data = 3},
> +    {PTRACE_SETFGREGS, .data = -1},
> +    {PTRACE_SETFGREGS, .data = -2},
> +    {PTRACE_SETFGREGS, .data = -3},
> +    {PTRACE_SETFGREGS, .data = -4},
> #endif
> #if HAVE_DECL_PTRACE_GETSIGINFO
> -    {
> -    PTRACE_GETSIGINFO,.data = 0}, {
> -    PTRACE_GETSIGINFO,.data = 1}, {
> -    PTRACE_GETSIGINFO,.data = 2}, {
> -    PTRACE_GETSIGINFO,.data = 3}, {
> -    PTRACE_GETSIGINFO,.data = -1}, {
> -    PTRACE_GETSIGINFO,.data = -2}, {
> -    PTRACE_GETSIGINFO,.data = -3}, {
> -    PTRACE_GETSIGINFO,.data = -4},
> +    {PTRACE_GETSIGINFO, .data = 0},
> +    {PTRACE_GETSIGINFO, .data = 1},
> +    {PTRACE_GETSIGINFO, .data = 2},
> +    {PTRACE_GETSIGINFO, .data = 3},
> +    {PTRACE_GETSIGINFO, .data = -1},
> +    {PTRACE_GETSIGINFO, .data = -2},
> +    {PTRACE_GETSIGINFO, .data = -3},
> +    {PTRACE_GETSIGINFO, .data = -4},
> #endif
> #if HAVE_DECL_PTRACE_SETSIGINFO
> -    {
> -    PTRACE_SETSIGINFO,.data = 0}, {
> -    PTRACE_SETSIGINFO,.data = 1}, {
> -    PTRACE_SETSIGINFO,.data = 2}, {
> -    PTRACE_SETSIGINFO,.data = 3}, {
> -    PTRACE_SETSIGINFO,.data = -1}, {
> -    PTRACE_SETSIGINFO,.data = -2}, {
> -    PTRACE_SETSIGINFO,.data = -3}, {
> -    PTRACE_SETSIGINFO,.data = -4},
> +    {PTRACE_SETSIGINFO, .data = 0},
> +    {PTRACE_SETSIGINFO, .data = 1},
> +    {PTRACE_SETSIGINFO, .data = 2},
> +    {PTRACE_SETSIGINFO, .data = 3},
> +    {PTRACE_SETSIGINFO, .data = -1},
> +    {PTRACE_SETSIGINFO, .data = -2},
> +    {PTRACE_SETSIGINFO, .data = -3},
> +    {PTRACE_SETSIGINFO, .data = -4},
> #endif
> };
>
> -int TST_TOTAL = ARRAY_SIZE(test_cases);
> +#define SPT(x) [PTRACE_##x] = #x,
> +static char *strings[] = {
> +    SPT(TRACEME)
> +    SPT(PEEKTEXT)
> +    SPT(PEEKDATA)
> +    SPT(PEEKUSER)
> +    SPT(POKETEXT)
> +    SPT(POKEDATA)
> +    SPT(POKEUSER)
> +#ifdef PTRACE_GETREGS
> +    SPT(GETREGS)
> +#endif
> +#ifdef PTRACE_SETREGS
> +    SPT(SETREGS)
> +#endif
> +#ifdef PTRACE_GETSIGINFO
> +    SPT(GETSIGINFO)
> +#endif
> +#ifdef PTRACE_SETSIGINFO
> +    SPT(SETSIGINFO)
> +#endif
> +#ifdef PTRACE_GETFGREGS
> +    SPT(GETFGREGS)
> +#endif
> +#ifdef PTRACE_SETFGREGS
> +    SPT(SETFGREGS)
> +#endif
> +    SPT(KILL)
> +    SPT(SINGLESTEP)
> +};
> +
> +static inline char *strptrace(int request)
> +{
> +    return strings[request];
> +}
> +
> +static void child(void)
> +{
>
> -int main(int argc, char *argv[])
> +    char path[512];
> +
> +    SAFE_PTRACE(PTRACE_TRACEME, 0, NULL, NULL);
> +
> +    if (tst_get_path("ptrace06_child", path, sizeof(path)))
> +        tst_brk(TCONF, "Couldn't find ptrace06_child in $PATH");
> +
> +    TEST(execl(path, "ptrace06_child", "", NULL));
> +    tst_brk(TFAIL | TTERRNO,
> +            "Failed to execute execl01_child");
> +    exit(0);
> +}
> +
> +static void run(void)
> {
>     size_t i;
> -    long ret;
> -    int saved_errno;
> +    int pid;
> +    int status;
>
> -    tst_parse_opts(argc, argv, NULL, NULL);
> +    pid = SAFE_FORK();
>
> -    make_a_baby(argc, argv);
> +    if (!pid)
> +        child();
> +
> +    SAFE_WAIT(&status);
> +
> +    if (!WIFSTOPPED(status))
> +        tst_brk(TBROK, "child %d was not stopped", pid);
>
>     for (i = 0; i < ARRAY_SIZE(test_cases); ++i) {
>         struct test_case_t *tc = &test_cases[i];
>
> -        errno = 0;
> -        ret =
> -            ptrace(tc->request, pid, (void *)tc->addr,
> -               (void *)tc->data);
> -        saved_errno = errno;
> -        if (ret != -1)
> -            tst_resm(TFAIL,
> +        TEST(ptrace(tc->request, pid, (void *)tc->addr,
> +                    (void *)tc->data));
> +        if (TST_RET != -1)
> +            tst_brk(TFAIL | TERRNO,
>                  "ptrace(%s, ..., %li, %li) returned %li instead of -1",
>                  strptrace(tc->request), tc->addr, tc->data,
> -                 ret);
> -        else if (saved_errno != EIO && saved_errno != EFAULT)
> -            tst_resm(TFAIL,
> +                 TST_RET);
> +        else if (TST_ERR != EIO && TST_ERR != EFAULT)
> +            tst_brk(TFAIL | TERRNO,
>                  "ptrace(%s, ..., %li, %li) expected errno EIO or 
> EFAULT; actual: %i (%s)",
>                  strptrace(tc->request), tc->addr, tc->data,
> -                 saved_errno, strerror(saved_errno));
> +                 TST_ERR, strerror(TST_ERR));
>         else
> -            tst_resm(TPASS,
> +            tst_res(TPASS,
>                  "ptrace(%s, ..., %li, %li) failed as expected",
>                  strptrace(tc->request), tc->addr, tc->data);
>     }
>
> -    /* hopefully this worked */
> -    ptrace(PTRACE_KILL, pid, NULL, NULL);
> +    SAFE_PTRACE(PTRACE_CONT, pid, NULL, NULL);
>
> -    tst_exit();
> }
> +
> +static struct tst_test test = {
> +    .test_all = run,
> +    .forks_child = 1,
> +};
> diff --git a/testcases/kernel/syscalls/ptrace/ptrace06_child.c 
> b/testcases/kernel/syscalls/ptrace/ptrace06_child.c
> new file mode 100644
> index 000000000..350b0e8df
> --- /dev/null
> +++ b/testcases/kernel/syscalls/ptrace/ptrace06_child.c
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2018 Linux Test Project
> + * Copyright (C) 2015 Cyril Hrubis chrubis@suse.cz
> + */
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +
> +int main(int argc, char *argv[])
> +{
> +
> +    tst_res(TPASS, "%s executed", argv[0]);
> +
> +    return 0;
> +}

Please run "make check" before sending the patch and fix the issues. 
Otherwise LGTM.

Regards,
Andrea Cervesato


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