[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AS8P193MB1285BCEDA342A074F6A837FDE4139@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM>
Date: Mon, 28 Nov 2022 08:56:09 +0100
From: Bernd Edlinger <bernd.edlinger@...mail.de>
To: limin <limin100@...wei.com>, shuah@...nel.org,
keescook@...omium.org, ebiederm@...ssion.com,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: songmuchun@...edance.com, lizefan.x@...edance.com
Subject: Re: [PATCH -next] selftests/ptrace: Fix Test terminated by timeout in
ptrace_attach
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@AM8PR10MB4708.EURPRD10.PROD.OUTLOOK.COM/
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@AM8PR10MB4708.EURPRD10.PROD.OUTLOOK.COM/
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@intel.com/
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@AM8PR10MB4708.EURPRD10.PROD.OUTLOOK.COM/
> Fixes: 2de4e82318c7 ("selftests/ptrace: add test cases for dead-locks")
> Signed-off-by: limin <limin100@...wei.com>
> ---
> 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
>
Powered by blists - more mailing lists