[LTP] [PATCH 2/2] syscalls/ptrace02: Add another EPERM error test

[LTP] [PATCH 2/2] syscalls/ptrace02: Add another EPERM error test

[Yang Xu]

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 runtest/syscalls                            |  1 +
 testcases/kernel/syscalls/ptrace/.gitignore |  1 +
 testcases/kernel/syscalls/ptrace/ptrace02.c | 66 +++++++++++++++++++++
 3 files changed, 68 insertions(+)
 create mode 100644 testcases/kernel/syscalls/ptrace/ptrace02.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 0dcd3d66d..a439267b2 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -990,6 +990,7 @@ pselect03 pselect03
 pselect03_64 pselect03_64
 
 ptrace01 ptrace01
+ptrace02 ptrace02
 ptrace03 ptrace03
 ptrace04 ptrace04
 ptrace05 ptrace05
diff --git a/testcases/kernel/syscalls/ptrace/.gitignore b/testcases/kernel/syscalls/ptrace/.gitignore
index 7ee3b3c47..34be5148f 100644
--- a/testcases/kernel/syscalls/ptrace/.gitignore
+++ b/testcases/kernel/syscalls/ptrace/.gitignore
@@ -1,4 +1,5 @@
 /ptrace01
+/ptrace02
 /ptrace03
 /ptrace04
 /ptrace05
diff --git a/testcases/kernel/syscalls/ptrace/ptrace02.c b/testcases/kernel/syscalls/ptrace/ptrace02.c
new file mode 100644
index 000000000..b87529d90
--- /dev/null
+++ b/testcases/kernel/syscalls/ptrace/ptrace02.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com
+ *
+ * ptrace() returns -1 and sets errno to EPERM if tracer doesn't have
+ * CAP_SYS_PTRACE capability for the process. Such as nobody user.
+ */
+
+#include <errno.h>
+#include <signal.h>
+#include <sys/wait.h>
+#include <pwd.h>
+#include <config.h>
+#include <stdlib.h>
+#include "ptrace.h"
+#include "tst_test.h"
+
+uid_t uid;
+
+static void verify_ptrace(void)
+{
+	int child_pid;
+
+	tst_res(TINFO, "Trace a process that don't have CAP_SYS_PTRACE capability(nobody user) for it");
+
+	child_pid = SAFE_FORK();
+	if (!child_pid)
+		pause();
+
+	if (!SAFE_FORK()) {
+		SAFE_SETUID(uid);
+		TEST(ptrace(PTRACE_ATTACH, child_pid, NULL, NULL));
+		if (TST_RET == 0) {
+			tst_res(TFAIL, "ptrace() succeeded unexpectedly");
+			TST_CHECKPOINT_WAKE(0);
+			exit(0);
+		}
+		if (TST_ERR == EPERM)
+			tst_res(TPASS | TTERRNO, "ptrace() failed as expected");
+		else
+			tst_res(TFAIL | TTERRNO, "ptrace() expected EPERM, but got");
+		TST_CHECKPOINT_WAKE(0);
+		exit(0);
+	}
+	TST_CHECKPOINT_WAIT(0);
+	SAFE_KILL(child_pid, SIGKILL);
+	SAFE_WAITPID(child_pid, NULL, 0);
+	tst_reap_children();
+}
+
+static void setup(void)
+{
+	struct passwd *pw;
+
+	pw = SAFE_GETPWNAM("nobody");
+	uid = pw->pw_uid;
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = verify_ptrace,
+	.forks_child = 1,
+	.needs_root = 1,
+	.needs_checkpoints = 1,
+};
-- 
2.23.0

[LTP] [PATCH 2/2] syscalls/ptrace02: Add another EPERM error test

[Cyril Hrubis]

Hi!
> +static void verify_ptrace(void)
> +{
> +	int child_pid;
> +
> +	tst_res(TINFO, "Trace a process that don't have CAP_SYS_PTRACE capability(nobody user) for it");

I wouldn't be printing this verbose info here, anyone who will have to
debug the test failures will look into the source code and at the test
description in the top level comment.

> +	child_pid = SAFE_FORK();
> +	if (!child_pid)
> +		pause();
> +
> +	if (!SAFE_FORK()) {
> +		SAFE_SETUID(uid);
> +		TEST(ptrace(PTRACE_ATTACH, child_pid, NULL, NULL));
> +		if (TST_RET == 0) {
> +			tst_res(TFAIL, "ptrace() succeeded unexpectedly");
> +			TST_CHECKPOINT_WAKE(0);
> +			exit(0);
> +		}
> +		if (TST_ERR == EPERM)
> +			tst_res(TPASS | TTERRNO, "ptrace() failed as expected");
> +		else
> +			tst_res(TFAIL | TTERRNO, "ptrace() expected EPERM, but got");
> +		TST_CHECKPOINT_WAKE(0);
> +		exit(0);
> +	}
> +	TST_CHECKPOINT_WAIT(0);
> +	SAFE_KILL(child_pid, SIGKILL);
> +	SAFE_WAITPID(child_pid, NULL, 0);

We do not need the checkpoints here at all, we just need to waitpid for
the second child before we kill the first one.

> +	tst_reap_children();
> +}
> +
> +static void setup(void)
> +{
> +	struct passwd *pw;
> +
> +	pw = SAFE_GETPWNAM("nobody");
> +	uid = pw->pw_uid;
> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.test_all = verify_ptrace,
> +	.forks_child = 1,
> +	.needs_root = 1,
> +	.needs_checkpoints = 1,
> +};
> -- 
> 2.23.0
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/2] syscalls/ptrace02: Add another EPERM error test

[Yang Xu]

Hi Cyril
> Hi!
>> +static void verify_ptrace(void)
>> +{
>> +	int child_pid;
>> +
>> +	tst_res(TINFO, "Trace a process that don't have CAP_SYS_PTRACE capability(nobody user) for it");
>
> I wouldn't be printing this verbose info here, anyone who will have to
> debug the test failures will look into the source code and at the test
> description in the top level comment.
Will remove it.
>
>> +	child_pid = SAFE_FORK();
>> +	if (!child_pid)
>> +		pause();
>> +
>> +	if (!SAFE_FORK()) {
>> +		SAFE_SETUID(uid);
>> +		TEST(ptrace(PTRACE_ATTACH, child_pid, NULL, NULL));
>> +		if (TST_RET == 0) {
>> +			tst_res(TFAIL, "ptrace() succeeded unexpectedly");
>> +			TST_CHECKPOINT_WAKE(0);
>> +			exit(0);
>> +		}
>> +		if (TST_ERR == EPERM)
>> +			tst_res(TPASS | TTERRNO, "ptrace() failed as expected");
>> +		else
>> +			tst_res(TFAIL | TTERRNO, "ptrace() expected EPERM, but got");
>> +		TST_CHECKPOINT_WAKE(0);
>> +		exit(0);
>> +	}
>> +	TST_CHECKPOINT_WAIT(0);
>> +	SAFE_KILL(child_pid, SIGKILL);
>> +	SAFE_WAITPID(child_pid, NULL, 0);
>
> We do not need the checkpoints here at all, we just need to waitpid for
> the second child before we kill the first one.
Yes, Will fix it in v2.
>
>> +	tst_reap_children();
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	struct passwd *pw;
>> +
>> +	pw = SAFE_GETPWNAM("nobody");
>> +	uid = pw->pw_uid;
>> +}
>> +
>> +static struct tst_test test = {
>> +	.setup = setup,
>> +	.test_all = verify_ptrace,
>> +	.forks_child = 1,
>> +	.needs_root = 1,
>> +	.needs_checkpoints = 1,
>> +};
>> --
>> 2.23.0
>>
>>
>>
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
>

[LTP] [PATCH v2 1/2] syscalls/ptrace02: Add another EPERM error test

[Yang Xu]

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 runtest/syscalls                            |  1 +
 testcases/kernel/syscalls/ptrace/.gitignore |  1 +
 testcases/kernel/syscalls/ptrace/ptrace02.c | 61 +++++++++++++++++++++
 3 files changed, 63 insertions(+)
 create mode 100644 testcases/kernel/syscalls/ptrace/ptrace02.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 3ddf70c54..aeacb8bc8 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -990,6 +990,7 @@ pselect03 pselect03
 pselect03_64 pselect03_64
 
 ptrace01 ptrace01
+ptrace02 ptrace02
 ptrace03 ptrace03
 ptrace04 ptrace04
 ptrace05 ptrace05
diff --git a/testcases/kernel/syscalls/ptrace/.gitignore b/testcases/kernel/syscalls/ptrace/.gitignore
index 7ee3b3c47..34be5148f 100644
--- a/testcases/kernel/syscalls/ptrace/.gitignore
+++ b/testcases/kernel/syscalls/ptrace/.gitignore
@@ -1,4 +1,5 @@
 /ptrace01
+/ptrace02
 /ptrace03
 /ptrace04
 /ptrace05
diff --git a/testcases/kernel/syscalls/ptrace/ptrace02.c b/testcases/kernel/syscalls/ptrace/ptrace02.c
new file mode 100644
index 000000000..cacaeeb96
--- /dev/null
+++ b/testcases/kernel/syscalls/ptrace/ptrace02.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com
+ *
+ * ptrace() returns -1 and sets errno to EPERM if tracer doesn't have
+ * CAP_SYS_PTRACE capability for the process. Such as nobody user.
+ */
+
+#include <errno.h>
+#include <signal.h>
+#include <sys/wait.h>
+#include <pwd.h>
+#include <config.h>
+#include <stdlib.h>
+#include "ptrace.h"
+#include "tst_test.h"
+
+uid_t uid;
+
+static void verify_ptrace(void)
+{
+	int child_pid[2];
+
+	child_pid[0] = SAFE_FORK();
+	if (!child_pid[0])
+		pause();
+
+	child_pid[1] = SAFE_FORK();
+	if (!child_pid[1]) {
+		SAFE_SETUID(uid);
+		TEST(ptrace(PTRACE_ATTACH, child_pid[0], NULL, NULL));
+		if (TST_RET == 0) {
+			tst_res(TFAIL, "ptrace() succeeded unexpectedly");
+			exit(0);
+		}
+		if (TST_ERR == EPERM)
+			tst_res(TPASS | TTERRNO, "ptrace() failed as expected");
+		else
+			tst_res(TFAIL | TTERRNO, "ptrace() expected EPERM, but got");
+		exit(0);
+	}
+	SAFE_WAITPID(child_pid[1], NULL, 0);
+	SAFE_KILL(child_pid[0], SIGKILL);
+	SAFE_WAITPID(child_pid[0], NULL, 0);
+}
+
+static void setup(void)
+{
+	struct passwd *pw;
+
+	pw = SAFE_GETPWNAM("nobody");
+	uid = pw->pw_uid;
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.test_all = verify_ptrace,
+	.forks_child = 1,
+	.needs_root = 1,
+};
-- 
2.23.0

[LTP] [PATCH v2 2/2] syscalls/ptrace11: Add test for tracing init process

[Yang Xu]

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 runtest/syscalls                            |  1 +
 testcases/kernel/syscalls/ptrace/.gitignore |  1 +
 testcases/kernel/syscalls/ptrace/ptrace11.c | 39 +++++++++++++++++++++
 3 files changed, 41 insertions(+)
 create mode 100644 testcases/kernel/syscalls/ptrace/ptrace11.c

diff --git a/runtest/syscalls b/runtest/syscalls
index aeacb8bc8..6b5908662 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1000,6 +1000,7 @@ ptrace07 ptrace07
 ptrace08 ptrace08
 ptrace09 ptrace09
 ptrace10 ptrace10
+ptrace11 ptrace11
 
 pwrite01 pwrite01
 pwrite02 pwrite02
diff --git a/testcases/kernel/syscalls/ptrace/.gitignore b/testcases/kernel/syscalls/ptrace/.gitignore
index 34be5148f..01cbc6072 100644
--- a/testcases/kernel/syscalls/ptrace/.gitignore
+++ b/testcases/kernel/syscalls/ptrace/.gitignore
@@ -7,3 +7,4 @@
 /ptrace08
 /ptrace09
 /ptrace10
+/ptrace11
diff --git a/testcases/kernel/syscalls/ptrace/ptrace11.c b/testcases/kernel/syscalls/ptrace/ptrace11.c
new file mode 100644
index 000000000..6375964f9
--- /dev/null
+++ b/testcases/kernel/syscalls/ptrace/ptrace11.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com
+ *
+ * This case just check whether we can trace init(1) process and
+ * doesn't trigger error.
+ */
+
+#include <errno.h>
+#include <signal.h>
+#include <sys/wait.h>
+#include <pwd.h>
+#include <config.h>
+#include <stdlib.h>
+#include "ptrace.h"
+#include "tst_test.h"
+
+static void verify_ptrace(void)
+{
+	TEST(ptrace(PTRACE_ATTACH, 1, NULL, NULL));
+	if (TST_RET == 0)
+		tst_res(TPASS, "ptrace() traces init process succeeded");
+	else
+		tst_res(TFAIL | TTERRNO,
+			"ptrace() returns %ld, failed unexpectedly", TST_RET);
+
+	/*
+	 * Running attach/detach more times will trigger a ESRCH error because
+	 * ptrace_check_attach function in kernel will report it if its process
+	 * stats is not __TASK_TRACED.
+	 */
+	TST_RETRY_FUNC(ptrace(PTRACE_DETACH, 1, NULL, NULL), TST_RETVAL_EQ0);
+}
+
+static struct tst_test test = {
+	.test_all = verify_ptrace,
+	.needs_root = 1,
+};
-- 
2.23.0

[LTP] [PATCH v2 1/2] syscalls/ptrace02: Add another EPERM error test

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 1/2] syscalls/ptrace02: Add another EPERM error test

[Cyril Hrubis]

Hi!
> Pushed, thanks.

I've pushed a follow up patch that adds an explicit test for return value.

Please be strict in checking the return values in your testcases from
now on.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 2/2] syscalls/ptrace11: Add test for tracing init process

[Cyril Hrubis]

Hi!
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com
> + *
> + * This case just check whether we can trace init(1) process and
> + * doesn't trigger error.
> + */

Why is init(1) special here? Is this a regression test?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 2/2] syscalls/ptrace11: Add test for tracing init process

[Cyril Hrubis]

Hi!
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
> > + * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com
> > + *
> > + * This case just check whether we can trace init(1) process and
> > + * doesn't trigger error.
> > + */
> 
> Why is init(1) special here? Is this a regression test?

Looking into the manual page this wasn't supported until 2.6.26. I guess
that we should mention that here in the test description.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 1/2] syscalls/ptrace02: Add another EPERM error test

[Yang Xu]

Hi Cyril
> Hi!
>> Pushed, thanks.
>
> I've pushed a follow up patch that adds an explicit test for return value.
>
> Please be strict in checking the return values in your testcases from
> now on.
Ok. I Will notice this.

Best Regards
Yang Xu
>

[LTP] [PATCH v2 2/2] syscalls/ptrace11: Add test for tracing init process

[Yang Xu]

Hi Cyril
> Hi!
>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>> +/*
>>> + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
>>> + * Author: Yang Xu<xuyang2018.jy@cn.fujitsu.com
>>> + *
>>> + * This case just check whether we can trace init(1) process and
>>> + * doesn't trigger error.
>>> + */
>>
>> Why is init(1) special here? Is this a regression test?
>
> Looking into the manual page this wasn't supported until 2.6.26. I guess
> that we should mention that here in the test description.
Yes, it should be added into the test description. Sorry for not 
explaining why testing init process.  Will add it in v2.
>

[LTP] [PATCH v3] syscalls/ptrace11: Add test for tracing init process

[Yang Xu]

Before kernel 2.6.26, ptrace can't trace init process and will get
EPERM error. Now, it should succeed when we have enough privileges.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 runtest/syscalls                            |  1 +
 testcases/kernel/syscalls/ptrace/.gitignore |  1 +
 testcases/kernel/syscalls/ptrace/ptrace11.c | 40 +++++++++++++++++++++
 3 files changed, 42 insertions(+)
 create mode 100644 testcases/kernel/syscalls/ptrace/ptrace11.c

diff --git a/runtest/syscalls b/runtest/syscalls
index aeacb8bc8..6b5908662 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1000,6 +1000,7 @@ ptrace07 ptrace07
 ptrace08 ptrace08
 ptrace09 ptrace09
 ptrace10 ptrace10
+ptrace11 ptrace11
 
 pwrite01 pwrite01
 pwrite02 pwrite02
diff --git a/testcases/kernel/syscalls/ptrace/.gitignore b/testcases/kernel/syscalls/ptrace/.gitignore
index 34be5148f..01cbc6072 100644
--- a/testcases/kernel/syscalls/ptrace/.gitignore
+++ b/testcases/kernel/syscalls/ptrace/.gitignore
@@ -7,3 +7,4 @@
 /ptrace08
 /ptrace09
 /ptrace10
+/ptrace11
diff --git a/testcases/kernel/syscalls/ptrace/ptrace11.c b/testcases/kernel/syscalls/ptrace/ptrace11.c
new file mode 100644
index 000000000..28a08f69d
--- /dev/null
+++ b/testcases/kernel/syscalls/ptrace/ptrace11.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
+ *
+ * Before kernel 2.6.26, we can't trace init(1) process and ptrace() will
+ * get EPERM error. This case just check whether we can trace init(1)
+ * process and doesn't trigger error.
+ */
+
+#include <errno.h>
+#include <signal.h>
+#include <sys/wait.h>
+#include <pwd.h>
+#include <config.h>
+#include <stdlib.h>
+#include "ptrace.h"
+#include "tst_test.h"
+
+static void verify_ptrace(void)
+{
+	TEST(ptrace(PTRACE_ATTACH, 1, NULL, NULL));
+	if (TST_RET == 0)
+		tst_res(TPASS, "ptrace() traces init process successfully");
+	else
+		tst_res(TFAIL | TTERRNO,
+			"ptrace() returns %ld, failed unexpectedly", TST_RET);
+
+	/*
+	 * Running attach/detach more times will trigger a ESRCH error because
+	 * ptrace_check_attach function in kernel will report it if its process
+	 * stats is not __TASK_TRACED.
+	 */
+	TST_RETRY_FUNC(ptrace(PTRACE_DETACH, 1, NULL, NULL), TST_RETVAL_EQ0);
+}
+
+static struct tst_test test = {
+	.test_all = verify_ptrace,
+	.needs_root = 1,
+};
-- 
2.23.0

[LTP] [PATCH v3] syscalls/ptrace11: Add test for tracing init process

[Cyril Hrubis]

Hi!
> +	/*
> +	 * Running attach/detach more times will trigger a ESRCH error because
> +	 * ptrace_check_attach function in kernel will report it if its process
> +	 * stats is not __TASK_TRACED.
> +	 */
> +	TST_RETRY_FUNC(ptrace(PTRACE_DETACH, 1, NULL, NULL), TST_RETVAL_EQ0);

Why do we have to retry the detach here?

Other than that the rest looks fine now.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3] syscalls/ptrace11: Add test for tracing init process

[Yang Xu]

Hi Cyril
> Hi!
>> +	/*
>> +	 * Running attach/detach more times will trigger a ESRCH error because
>> +	 * ptrace_check_attach function in kernel will report it if its process
>> +	 * stats is not __TASK_TRACED.
>> +	 */
>> +	TST_RETRY_FUNC(ptrace(PTRACE_DETACH, 1, NULL, NULL), TST_RETVAL_EQ0);
>
> Why do we have to retry the detach here?

I add a retry here because running attach/detach serval times may make 
init process isn't traced status . Even we have do attach action, detach 
will get ESRCH error .

In kernel/ptrace.c code, it has the following calltrace
SYSCALL_DEFINE4(ptrace...
    =>ptrace_check_attach   //if we don't use PTRACE_ATTACH/PTRACE_SEIZE
      =>ptrace_freeze_traced

/* Ensure that nothing can wake it up, even SIGKILL */
static bool ptrace_freeze_traced(struct task_struct *task)
{
         bool ret = false;

         /* Lockless, nobody but us can set this flag */
         if (task->jobctl & JOBCTL_LISTENING)
                 return ret;
         spin_lock_irq(&task->sighand->siglock);
         if (task_is_traced(task) && !__fatal_signal_pending(task)) {
                 task->state = __TASK_TRACED;
                 ret = true;
         }
         spin_unlock_irq(&task->sighand->siglock);

         return ret;
}

ptrace_freeze_traced() may returns false when we run attach/detach 
serval times, so ptrace_check_attach returns ESRCH error .

To be honset, I don't figure out why task->stat is not task_traced 
status after attaching. I am looking into this.

Best Regards
Yang Xu
>
> Other than that the rest looks fine now.
>

[LTP] [PATCH v3] syscalls/ptrace11: Add test for tracing init process

[Cyril Hrubis]

Hi!
> >> +	/*
> >> +	 * Running attach/detach more times will trigger a ESRCH error because
> >> +	 * ptrace_check_attach function in kernel will report it if its process
> >> +	 * stats is not __TASK_TRACED.
> >> +	 */
> >> +	TST_RETRY_FUNC(ptrace(PTRACE_DETACH, 1, NULL, NULL), TST_RETVAL_EQ0);
> >
> > Why do we have to retry the detach here?
> 
> I add a retry here because running attach/detach serval times may make 
> init process isn't traced status . Even we have do attach action, detach 
> will get ESRCH error .

Looking at the manual page it says:

PTRACE_ATTACH

...
The tracee is sent a SIGSTOP, but will not necessarily have stopped by
the completion of this call; use waitpid(2) to wait for the tracee to
stop.
...

So my guess is that if we call PTRACE_ATTACH followed by the
PTRACE_DETACH we may end up in a state where the SIGSTOP for the traced
process haven't arrived yet and in this case we should get ESCRCH.

So the correct solution is waitpid() for the traced process before we
detach it.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3] syscalls/ptrace11: Add test for tracing init process

[Yang Xu]

Hi Cyril
> Hi!
>>>> +	/*
>>>> +	 * Running attach/detach more times will trigger a ESRCH error because
>>>> +	 * ptrace_check_attach function in kernel will report it if its process
>>>> +	 * stats is not __TASK_TRACED.
>>>> +	 */
>>>> +	TST_RETRY_FUNC(ptrace(PTRACE_DETACH, 1, NULL, NULL), TST_RETVAL_EQ0);
>>>
>>> Why do we have to retry the detach here?
>>
>> I add a retry here because running attach/detach serval times may make
>> init process isn't traced status . Even we have do attach action, detach
>> will get ESRCH error .
>
> Looking at the manual page it says:
>
> PTRACE_ATTACH
>
> ...
> The tracee is sent a SIGSTOP, but will not necessarily have stopped by
> the completion of this call; use waitpid(2) to wait for the tracee to
> stop.
Oh, I ignored it. Sorry.
> ...
>
> So my guess is that if we call PTRACE_ATTACH followed by the
> PTRACE_DETACH we may end up in a state where the SIGSTOP for the traced
> process haven't arrived yet and in this case we should get ESCRCH.
>
> So the correct solution is waitpid() for the traced process before we
> detach it.
Yes, using waitpid() is ok. I Will send a v4 soon.
>

[LTP] [PATCH v4] syscalls/ptrace11: Add test for tracing init process

[Yang Xu]

Before kernel 2.6.26, ptrace can't trace init process and will get
EPERM error. Now, it should succeed when we have enough privileges.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 runtest/syscalls                            |  1 +
 testcases/kernel/syscalls/ptrace/.gitignore |  1 +
 testcases/kernel/syscalls/ptrace/ptrace11.c | 46 +++++++++++++++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 testcases/kernel/syscalls/ptrace/ptrace11.c

diff --git a/runtest/syscalls b/runtest/syscalls
index a5363277f..be5e1e76c 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1000,6 +1000,7 @@ ptrace07 ptrace07
 ptrace08 ptrace08
 ptrace09 ptrace09
 ptrace10 ptrace10
+ptrace11 ptrace11
 
 pwrite01 pwrite01
 pwrite02 pwrite02
diff --git a/testcases/kernel/syscalls/ptrace/.gitignore b/testcases/kernel/syscalls/ptrace/.gitignore
index 34be5148f..01cbc6072 100644
--- a/testcases/kernel/syscalls/ptrace/.gitignore
+++ b/testcases/kernel/syscalls/ptrace/.gitignore
@@ -7,3 +7,4 @@
 /ptrace08
 /ptrace09
 /ptrace10
+/ptrace11
diff --git a/testcases/kernel/syscalls/ptrace/ptrace11.c b/testcases/kernel/syscalls/ptrace/ptrace11.c
new file mode 100644
index 000000000..daaadcd3a
--- /dev/null
+++ b/testcases/kernel/syscalls/ptrace/ptrace11.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
+ *
+ * Before kernel 2.6.26, we can't trace init(1) process and ptrace() will
+ * get EPERM error. This case just check whether we can trace init(1)
+ * process and doesn't trigger error.
+ */
+
+#include <errno.h>
+#include <signal.h>
+#include <sys/wait.h>
+#include <pwd.h>
+#include <config.h>
+#include <stdlib.h>
+#include "ptrace.h"
+#include "tst_test.h"
+
+static void verify_ptrace(void)
+{
+	TEST(ptrace(PTRACE_ATTACH, 1, NULL, NULL));
+	if (TST_RET == 0)
+		tst_res(TPASS, "ptrace() traces init process successfully");
+	else
+		tst_res(TFAIL | TTERRNO,
+			"ptrace() returns %ld, failed unexpectedly", TST_RET);
+
+	/*
+	 * As ptrace(2) man-page said, when using PTRACE_ATTACH option, the
+	 * tracee is sent a SIGSTOP, but will not necessarily have stopped
+	 * by the completion of this call. Use waitpid(2) to wait for the
+	 * tracee into stop. Otherwise it may get ESRCH error.
+	 * As waitpid(2) man-pages said, status for traced children which have
+	 * stopped is provided even if WUNTRACED option is not specified.
+	 * So using 0 option is enough.
+	 */
+	SAFE_WAITPID(1, NULL, 0);
+
+	SAFE_PTRACE(PTRACE_DETACH, 1, NULL, NULL);
+}
+
+static struct tst_test test = {
+	.test_all = verify_ptrace,
+	.needs_root = 1,
+};
-- 
2.23.0

[LTP] [PATCH v4] syscalls/ptrace11: Add test for tracing init process

[Cyril Hrubis]

Hi!
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> + *
> + * Before kernel 2.6.26, we can't trace init(1) process and ptrace() will
> + * get EPERM error. This case just check whether we can trace init(1)
> + * process and doesn't trigger error.
> + */

I've reformatted this comment so that it's picked up by documentation
parser.

> +#include <errno.h>
> +#include <signal.h>
> +#include <sys/wait.h>
> +#include <pwd.h>
> +#include <config.h>
> +#include <stdlib.h>
> +#include "ptrace.h"
> +#include "tst_test.h"
> +
> +static void verify_ptrace(void)
> +{
> +	TEST(ptrace(PTRACE_ATTACH, 1, NULL, NULL));
> +	if (TST_RET == 0)
> +		tst_res(TPASS, "ptrace() traces init process successfully");
> +	else
> +		tst_res(TFAIL | TTERRNO,
> +			"ptrace() returns %ld, failed unexpectedly", TST_RET);
> +
> +	/*
> +	 * As ptrace(2) man-page said, when using PTRACE_ATTACH option, the
> +	 * tracee is sent a SIGSTOP, but will not necessarily have stopped
> +	 * by the completion of this call. Use waitpid(2) to wait for the
> +	 * tracee into stop. Otherwise it may get ESRCH error.
> +	 * As waitpid(2) man-pages said, status for traced children which have
> +	 * stopped is provided even if WUNTRACED option is not specified.
> +	 * So using 0 option is enough.
> +	 */

Simplified this comment.


And pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz