From: Bernd Edlinger <bernd.edlinger@hotmail.de>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Alexey Dobriyan <adobriyan@gmail.com>,
Oleg Nesterov <oleg@redhat.com>,
Kees Cook <keescook@chromium.org>,
Andy Lutomirski <luto@amacapital.net>,
Will Drewry <wad@chromium.org>, Shuah Khan <shuah@kernel.org>,
Christian Brauner <christian.brauner@ubuntu.com>,
Michal Hocko <mhocko@suse.com>, Serge Hallyn <serge@hallyn.com>,
James Morris <jamorris@linux.microsoft.com>,
Charles Haithcock <chaithco@redhat.com>,
Suren Baghdasaryan <surenb@google.com>,
Yafang Shao <laoar.shao@gmail.com>, Helge Deller <deller@gmx.de>,
YiFei Zhu <yifeifz2@illinois.edu>,
Adrian Reber <areber@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
Jens Axboe <axboe@kernel.dk>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
linux-kselftest@vger.kernel.org,
"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH v9] exec: Fix dead-lock in de_thread with ptrace_attach
Date: Tue, 22 Jun 2021 07:10:20 +0200 [thread overview]
Message-ID: <DB9PR10MB47134C036A2DADB4FD53F16AE4099@DB9PR10MB4713.EURPRD10.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <AM8PR10MB47083E11E2B39ACBDF396954E40F9@AM8PR10MB4708.EURPRD10.PROD.OUTLOOK.COM>
On 6/16/21 11:31 PM, Bernd Edlinger wrote:
> On 6/15/21 4:26 PM, Bernd Edlinger wrote:
>> The first phase of de_thread needs co-operation from a user task,
>> if and only if any task t except the thread leader has t->ptrace.
>> Taking tasks from RUNNING->EXIT_ZOMBIE only needs co-operation from kernel code,
>
>
> Aehm, sorry, that is not correct, what I said here.
>
> I totally overlooked ptrace(PTRACE_SEIZE, pid, 0L, PTRACE_O_TRACEEXIT)
>
> and unfortunately this also prevents even the thread leader to enter the
> EXIT_ZOMBIE state because do_exit does:
>
> ptrace_event(PTRACE_EVENT_EXIT, code);
>
> unfortunately this sends an event to the tracer, and waits not only for
> the tracer to call waitpid, but also needs a PTRACE_CONT before do_exit
> can call exit_notify which does tsk->exit_state = EXIT_ZOMBIE.
>
P.S:
I think there is something really odd in ptrace_stop().
If it is intentional (which I believe to be the case) to wait here after a
SIGKILL until the process enters the exit_state == EXIT_ZOMBIE, then aborting the
pending ptrace_stop() via sigkill_pending() is questionable, especially because
arch_ptrace_stop_needed() is defined as (0) in most architectures, only sparc and
ia64 do something here.
static void ptrace_stop(int exit_code, int why, int clear_code, kernel_siginfo_t *info)
__releases(¤t->sighand->siglock)
__acquires(¤t->sighand->siglock)
{
bool gstop_done = false;
if (arch_ptrace_stop_needed(exit_code, info)) {
/*
* The arch code has something special to do before a
* ptrace stop. This is allowed to block, e.g. for faults
* on user stack pages. We can't keep the siglock while
* calling arch_ptrace_stop, so we must release it now.
* To preserve proper semantics, we must do this before
* any signal bookkeeping like checking group_stop_count.
* Meanwhile, a SIGKILL could come in before we retake the
* siglock. That must prevent us from sleeping in TASK_TRACED.
* So after regaining the lock, we must check for SIGKILL.
*/
spin_unlock_irq(¤t->sighand->siglock);
arch_ptrace_stop(exit_code, info);
spin_lock_irq(¤t->sighand->siglock);
if (sigkill_pending(current))
return;
}
set_special_state(TASK_TRACED);
After this point there is no sigkill_pending() or fatal_signal_pending(), just
a single freezable_schedule() which explains why this can even wait with a fatal
signal pending. But if the code executes the if block above the sigkill can
only be ignored if it happens immediately before the set_special_state(TASK_TRACED).
What do you think?
Bernd.
next prev parent reply other threads:[~2021-06-22 5:10 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-10 7:31 [PATCHv8] exec: Fix dead-lock in de_thread with ptrace_attach Bernd Edlinger
2021-06-10 21:36 ` Andrew Morton
2021-06-11 4:42 ` Bernd Edlinger
2021-06-11 15:55 ` [PATCH v9] " Bernd Edlinger
2021-06-11 23:16 ` Andrew Morton
2021-06-12 5:22 ` Bernd Edlinger
2021-06-12 7:02 ` Greg KH
2021-06-14 16:42 ` Eric W. Biederman
2021-06-15 14:26 ` Bernd Edlinger
2021-06-16 21:31 ` Bernd Edlinger
2021-06-22 5:10 ` Bernd Edlinger [this message]
2021-06-12 19:44 ` [PATCHv8] " Eric W. Biederman
2021-06-11 7:54 ` Bernd Edlinger
2021-06-11 23:15 ` Andrew Morton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DB9PR10MB47134C036A2DADB4FD53F16AE4099@DB9PR10MB4713.EURPRD10.PROD.OUTLOOK.COM \
--to=bernd.edlinger@hotmail.de \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=areber@redhat.com \
--cc=axboe@kernel.dk \
--cc=chaithco@redhat.com \
--cc=christian.brauner@ubuntu.com \
--cc=deller@gmx.de \
--cc=ebiederm@xmission.com \
--cc=jamorris@linux.microsoft.com \
--cc=keescook@chromium.org \
--cc=laoar.shao@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mhocko@suse.com \
--cc=oleg@redhat.com \
--cc=serge@hallyn.com \
--cc=shuah@kernel.org \
--cc=stable@vger.kernel.org \
--cc=surenb@google.com \
--cc=tglx@linutronix.de \
--cc=viro@zeniv.linux.org.uk \
--cc=wad@chromium.org \
--cc=yifeifz2@illinois.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.