All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Bernd Edlinger <bernd.edlinger@hotmail.de>,
	Waiman Long <longman@redhat.com>, Ingo Molnar <mingo@kernel.org>,
	Will Deacon <will@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Alexey Gladkov <gladkov.alexey@gmail.com>,
	Oleg Nesterov <oleg@redhat.com>
Subject: Re: [GIT PULL] Please pull proc and exec work for 5.7-rc1
Date: Thu, 9 Apr 2020 17:30:41 -0700	[thread overview]
Message-ID: <CAHk-=whKHpERyVv2-C+kxq9KV_mJPW3hkGDpn6f4yOvs+au8SA@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wgh4zts+3hdkGzHLJ6pBGumcJ=23gRbMfubDrLstis2Bg@mail.gmail.com>

On Thu, Apr 9, 2020 at 2:17 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> So I'm not just good at "spotting odd corner cases". I told you why
> that bogus deadlock of yours failed - the execve was pointlessly
> waiting for a dead thread that had marked itself ptraced, and nobody
> was reaping it.

Side note: this does seem to be the only special case at least in this
particular area for execve().

The thing is, for actual _live_ threads that end up waiting for
ptracers, the SIGKILL that zap_other_threads() does should punch
through everything - it should not just break out anybody waiting for
the mutex, it should also break any "wait for tracer" ptrace waiting
conditions.

But already dead threads are special. Unlike SIGKILL'ed other threads,
they can stay around almost forever.

Because they're in Zombie state, and no amount of SIGKILL will do
anything to them (and zap_other_threads() didn't even try), and
they'll stay around until they are reaped by some entirely independent
party (which just happened to be the same one that was doing the
ptrace, but doesn't really have to be)

(Of course, who knows what other special cases there might be - I'm
not saying this is the _only_ special case, but in the particular area
of 'zap other threads and wait for them', already dead threads do seem
to be special).

So the fact that "zap_threads()" counts dead threads - but cannot do
anything about them - is fundamentally different, and that's why that
particular test-case has that odd behavior.

So I think we have basically three choices:

 (1) have execve() not wait for dead threads while holding the cred
mutex (that's effectively what that zap_threads() hacky patch does,
but it's not really correct because it can cause notify_count
underflows)

 (2) have zap_other_threads() force-reap Zombie threads

 (3) say that it's a user space bug, and if you're doing PTRACE_ATTACH
you need to make sure there are no dead threads of the target that
might be blocking an execve().

For an example of that (3) approach: making the test-case just do a
waitpid() before the PTRACE_ATTACH will unhang it, because it reaps
that thread that did the PTRACE_TRACEME.

So option (3) is basically saying "that test-case is buggy, exactly
like the two readers on a pipe".

But I continued to look at (1), but trying to deal with the fact that
"notify_count" will get decremented not just by live threads, but by
the ones that already died too.

Instead of trying to change how notify_count gets decremented, we
could do something like the attached patch: wait for it to go down to
zero, yes, but go back and re-check until you don't have to wait any
more. That should fix the underflow situation. The comment says it
all.

This patch is 100% untested. It probably compiles, that's all I'll
say. I'm not proud of it. And I may be missing some important thing
(and I'm not happy about the magical "am I not the
thread_group_leader?" special case etc).

It's worth noting that this code is the only one that cares about the
return value of zap_other_threads(), so changing the semantics to only
count non-dead threads is safe in that sense.

Whether it's safe to then share the signal structure ever the rest of
exevbe() - even if it's only with dead threads - I didn't check.

I think Oleg might be the only person alive who understands all of our
process exit code.

Oleg? Comments?

              Linus

  parent reply	other threads:[~2020-04-10  0:31 UTC|newest]

Thread overview: 127+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <87blobnq02.fsf@x220.int.ebiederm.org>
2020-04-02 19:04 ` [GIT PULL] Please pull proc and exec work for 5.7-rc1 Linus Torvalds
2020-04-02 19:31   ` Bernd Edlinger
2020-04-02 19:52     ` Linus Torvalds
2020-04-02 20:59       ` Bernd Edlinger
2020-04-02 21:46         ` Linus Torvalds
2020-04-02 23:01           ` Eric W. Biederman
2020-04-02 23:42             ` Bernd Edlinger
2020-04-02 23:45               ` Eric W. Biederman
2020-04-02 23:49                 ` Bernd Edlinger
2020-04-02 23:45               ` Linus Torvalds
2020-04-02 23:44             ` Linus Torvalds
2020-04-03  0:05               ` Eric W. Biederman
2020-04-07  1:29               ` [RFC][PATCH 0/3] exec_update_mutex related cleanups Eric W. Biederman
2020-04-07  1:31                 ` [PATCH 1/3] binfmt: Move install_exec_creds after setup_new_exec to match binfmt_elf Eric W. Biederman
2020-04-07 15:58                   ` Kees Cook
2020-04-07 16:11                   ` Christian Brauner
2020-04-08 17:25                   ` Linus Torvalds
2020-04-08 19:51                     ` Eric W. Biederman
2020-04-07  1:31                 ` [PATCH 2/3] exec: Make unlocking exec_update_mutex explict Eric W. Biederman
2020-04-07 16:02                   ` Kees Cook
2020-04-07 16:17                   ` Christian Brauner
2020-04-07 16:21                     ` Eric W. Biederman
2020-04-07  1:32                 ` [PATCH 3/3] exec: Rename the flag called_exec_mmap point_of_no_return Eric W. Biederman
2020-04-07 16:03                   ` Kees Cook
2020-04-07 16:21                   ` Christian Brauner
2020-04-07 16:22                 ` [RFC][PATCH 0/3] exec_update_mutex related cleanups Christian Brauner
2020-04-08 17:26                 ` Linus Torvalds
2020-04-03  5:09             ` [GIT PULL] Please pull proc and exec work for 5.7-rc1 Bernd Edlinger
2020-04-03 19:26             ` Linus Torvalds
2020-04-03 20:41               ` Waiman Long
2020-04-03 20:59                 ` Linus Torvalds
2020-04-03 23:16                   ` Waiman Long
2020-04-03 23:23                     ` Waiman Long
2020-04-04  1:30                       ` Linus Torvalds
2020-04-04  2:02                         ` Waiman Long
2020-04-04  2:28                           ` Linus Torvalds
2020-04-04  6:34                             ` Bernd Edlinger
2020-04-05  6:34                               ` Bernd Edlinger
2020-04-05 19:35                                 ` Linus Torvalds
2020-04-05  2:42                             ` Waiman Long
2020-04-05  3:35                               ` Bernd Edlinger
2020-04-05  3:45                                 ` Waiman Long
2020-04-06 13:13                             ` Will Deacon
2020-04-04  4:23                     ` Bernd Edlinger
2020-04-06 22:17               ` Eric W. Biederman
2020-04-07 19:50                 ` Linus Torvalds
2020-04-07 20:29                   ` Bernd Edlinger
2020-04-07 20:47                     ` Linus Torvalds
2020-04-08 15:14                   ` Eric W. Biederman
2020-04-08 15:21                     ` Bernd Edlinger
2020-04-08 16:34                     ` Linus Torvalds
2020-04-09 14:58                       ` Eric W. Biederman
2020-04-09 15:15                         ` Bernd Edlinger
2020-04-09 16:15                         ` Linus Torvalds
2020-04-09 16:24                           ` Linus Torvalds
2020-04-09 17:03                             ` Eric W. Biederman
2020-04-09 17:17                               ` Bernd Edlinger
2020-04-09 17:37                                 ` Linus Torvalds
2020-04-09 17:46                                   ` Bernd Edlinger
2020-04-09 18:36                                     ` Linus Torvalds
2020-04-09 19:42                                       ` Linus Torvalds
2020-04-09 19:57                                         ` Bernd Edlinger
2020-04-09 20:04                                           ` Linus Torvalds
2020-04-09 20:36                                             ` Bernd Edlinger
2020-04-09 21:00                                             ` Eric W. Biederman
2020-04-09 21:17                                               ` Linus Torvalds
2020-04-09 23:52                                                 ` Bernd Edlinger
2020-04-10  0:30                                                 ` Linus Torvalds [this message]
2020-04-10  0:32                                                   ` Linus Torvalds
2020-04-11  4:07                                                     ` Bernd Edlinger
2020-04-11 18:20                                                   ` Oleg Nesterov
2020-04-11 18:29                                                     ` Linus Torvalds
2020-04-11 18:31                                                       ` Linus Torvalds
2020-04-11 19:15                                                       ` Bernd Edlinger
2020-04-11 20:07                                                         ` Linus Torvalds
2020-04-11 21:16                                                           ` Bernd Edlinger
     [not found]                                                             ` <CAHk-=wgWHkBzFazWJj57emHPd3Dg9SZHaZqoO7-AD+UbBTJgig@mail.gmail.com>
2020-04-11 21:57                                                               ` Linus Torvalds
2020-04-12  6:01                                                                 ` Bernd Edlinger
2020-04-12 19:50                                                       ` Oleg Nesterov
2020-04-12 20:14                                                         ` Linus Torvalds
2020-04-28  2:56                                                           ` Bernd Edlinger
2020-04-28 17:07                                                             ` Linus Torvalds
2020-04-28 19:08                                                               ` Oleg Nesterov
2020-04-28 20:35                                                                 ` Linus Torvalds
2020-04-28 21:06                                                                   ` Jann Horn
2020-04-28 21:36                                                                     ` Linus Torvalds
2020-04-28 21:53                                                                       ` Jann Horn
2020-04-28 22:14                                                                         ` Linus Torvalds
2020-04-28 23:36                                                                           ` Jann Horn
2020-04-29 17:58                                                                             ` Linus Torvalds
2020-04-29 18:33                                                                               ` Jann Horn
2020-04-29 18:57                                                                                 ` Linus Torvalds
2020-04-29 19:23                                                                               ` Bernd Edlinger
2020-04-29 19:26                                                                                 ` Jann Horn
2020-04-29 20:19                                                                                   ` Bernd Edlinger
2020-04-29 21:06                                                                                     ` Jann Horn
2020-04-29 22:38                                                                                 ` Linus Torvalds
2020-04-29 23:22                                                                                   ` Linus Torvalds
2020-04-29 23:59                                                                                     ` Jann Horn
2020-04-30  1:08                                                                                       ` Bernd Edlinger
2020-04-30  2:20                                                                                         ` Linus Torvalds
2020-04-30  3:00                                                                                           ` Jann Horn
2020-04-30  3:25                                                                                             ` Linus Torvalds
2020-04-30  3:41                                                                                               ` Jann Horn
2020-04-30  3:50                                                                                                 ` Linus Torvalds
2020-04-30 13:37                                                                                               ` Linus Torvalds
2020-04-30  2:16                                                                                       ` Linus Torvalds
2020-04-30 13:39                                                                                         ` Bernd Edlinger
2020-04-30 13:47                                                                                           ` Linus Torvalds
2020-04-30 14:29                                                                                             ` Bernd Edlinger
2020-04-30 16:40                                                                                               ` Linus Torvalds
2020-05-02  4:11                                                                                                 ` Bernd Edlinger
2020-04-09 17:36                               ` Linus Torvalds
2020-04-09 20:34                                 ` Eric W. Biederman
2020-04-09 20:56                                   ` Linus Torvalds
2020-04-02 23:02           ` Bernd Edlinger
2020-04-02 23:22           ` Bernd Edlinger
2020-04-03  7:38           ` Bernd Edlinger
2020-04-03 16:00       ` Bernd Edlinger
2020-04-03 15:09   ` Bernd Edlinger
2020-04-03 16:23     ` Linus Torvalds
2020-04-03 16:36       ` Bernd Edlinger
2020-04-04  5:43       ` Bernd Edlinger
2020-04-04  5:48         ` Bernd Edlinger
2020-04-06  6:41           ` Bernd Edlinger
2020-04-10 13:03 ` [GIT PULL] proc fix " Eric W. Biederman
2020-04-10 20:40   ` pr-tracker-bot

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='CAHk-=whKHpERyVv2-C+kxq9KV_mJPW3hkGDpn6f4yOvs+au8SA@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=bernd.edlinger@hotmail.de \
    --cc=ebiederm@xmission.com \
    --cc=gladkov.alexey@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=will@kernel.org \
    /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.