Hi, thanks for cleaning this up. Just for completenes: I have actually two patches submitted a while ago, but did not get any response so far, one that would make the test case work as it is: [PATCH v10] exec: Fix dead-lock in de_thread with ptrace_attach https://lore.kernel.org/lkml/AM8PR10MB470801D01A0CF24BC32C25E7E40E9@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ and my favorite one, that would fix the dead-lock altogether (and adjust the test case accordingly): [PATCH v11] exec: Fix dead-lock in de_thread with ptrace_attach https://lore.kernel.org/lkml/AM8PR10MB470875B22B4C08BEAEC3F77FE4169@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ Note: I got actually one response from an automatic checker to the v11 patch, (that I just discovered today, when I searched in my in-box:) https://lore.kernel.org/lkml/202107121344.wu68hEPF-lkp@xxxxxxxxx/ which is complaining about: >> kernel/ptrace.c:425:26: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct cred const *old_cred @@ got struct cred const [noderef] __rcu *real_cred @@ 417 struct linux_binprm *bprm = task->signal->exec_bprm; 418 const struct cred *old_cred; 419 struct mm_struct *old_mm; 420 421 retval = down_write_killable(&task->signal->exec_update_lock); 422 if (retval) 423 goto unlock_creds; 424 task_lock(task); > 425 old_cred = task->real_cred; probably no big issue, I guess I would just have to add a __rcu to the declaration of old_cred. But similar warnings appear to be everywhere. Thanks Bernd. On 11/28/22 8:04 AM, limin wrote: > That is an open issue > Bernd Edlinger wrote the test case in anticipation that all of > patch series got accepted,but the last patch was not picked up > for inclusion in the linux kernel. > How to reproduce warning: > $ make -C tools/testing/selftests TARGETS=ptrace run_tests > Example vmaccess from 6.1.0-next source tree run fail on bare metal > RUN global.attach ... > attach: Test terminated by timeout > FAIL global.attach > > Link:https://lore.kernel.org/all/AM8PR10MB4708E6FF0E155261455064C2E4209@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ > Fixes: 2de4e82318c7 ("selftests/ptrace: add test cases for dead-locks") > Signed-off-by: limin <limin100@xxxxxxxxxx> > --- > tools/testing/selftests/ptrace/vmaccess.c | 37 ++++++++--------------- > 1 file changed, 13 insertions(+), 24 deletions(-) > > diff --git a/tools/testing/selftests/ptrace/vmaccess.c b/tools/testing/selftests/ptrace/vmaccess.c > index 4db327b44586..751a41f1163c 100644 > --- a/tools/testing/selftests/ptrace/vmaccess.c > +++ b/tools/testing/selftests/ptrace/vmaccess.c > @@ -45,42 +45,31 @@ TEST(vmaccess) > > TEST(attach) > { > - int s, k, pid = fork(); > + int k; > + int s; > > + pid_t pid = fork(); > if (!pid) { > - pthread_t pt; > - > - pthread_create(&pt, NULL, thread, NULL); > - pthread_join(pt, NULL); > + ptrace(PTRACE_TRACEME, 0, NULL, NULL); > execlp("sleep", "sleep", "2", NULL); > } > > sleep(1); > k = ptrace(PTRACE_ATTACH, pid, 0L, 0L); > - ASSERT_EQ(errno, EAGAIN); > + printf("k1:%d\n", k); > + ASSERT_EQ(k, -1); > + waitpid(pid, &s, WNOHANG); > ASSERT_EQ(k, -1); > - k = waitpid(-1, &s, WNOHANG); > - ASSERT_NE(k, -1); > ASSERT_NE(k, 0); > ASSERT_NE(k, pid); > - ASSERT_EQ(WIFEXITED(s), 1); > - ASSERT_EQ(WEXITSTATUS(s), 0); > - sleep(1); > - k = ptrace(PTRACE_ATTACH, pid, 0L, 0L); > - ASSERT_EQ(k, 0); > - k = waitpid(-1, &s, 0); > - ASSERT_EQ(k, pid); > + if (WIFEXITED(s)) > + ASSERT_EQ(WEXITSTATUS(s), 0); > + if (WIFSTOPPED(s)) > + ASSERT_EQ(WSTOPSIG(s), SIGTRAP); > ASSERT_EQ(WIFSTOPPED(s), 1); > - ASSERT_EQ(WSTOPSIG(s), SIGSTOP); > - k = ptrace(PTRACE_DETACH, pid, 0L, 0L); > - ASSERT_EQ(k, 0); > - k = waitpid(-1, &s, 0); > - ASSERT_EQ(k, pid); > - ASSERT_EQ(WIFEXITED(s), 1); > - ASSERT_EQ(WEXITSTATUS(s), 0); > - k = waitpid(-1, NULL, 0); > + sleep(1); > + ptrace(PTRACE_CONT, pid, NULL, NULL); > ASSERT_EQ(k, -1); > - ASSERT_EQ(errno, ECHILD); > } > > TEST_HARNESS_MAIN >
