Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1614764

Re: [RFC][PATCH] exec: Don't wait for ptraced threads to be reaped.

Show all headers | View raw

On 03/30, Eric W. Biederman wrote:
>
> Oleg Nesterov <oleg@redhat.com> writes:
>
> > Very nice. So de_thread() returns as soon as all other threads decrement
> > signal->live in do_exit(). Before they do, say, exit_mm(). This is already
> > wrong, for example this breaks OOM. Plus a lot more problems afaics,  but
> > lets ignore this.
>
> Which means that we need to keep sig->notify_count.

Yes, although we need to make it less ugly.

> > Note that de_thread() also unshares ->sighand before return. So in the
> > case of mt exec it will likely see oldsighand->count != 1 and alloc the
> > new sighand_struct and this breaks the locking.
> >
> > Because the execing thread will use newsighand->siglock to protect its
> > signal_struct while the zombie threads will use oldsighand->siglock to
> > protect the same signal struct. Yes, tasklist_lock + the fact irq_disable
> > implies rcu_lock mostly save us but not entirely, say, a foreign process
> > doing __send_signal() can take the right or the wrong lock depending on
> > /dev/random.
>
> Which leads to the question how can we get back tot he 2.4 behavior
> of freeing sighand_struct in do_exit?
>
> At which point as soon as we free sighand_struct if we are the last
> to dying thread notify de_thread and everything works.

I was thinking about the similar option, see below, but decided that we
should not do this at least right now.

> For what __ptrace_unlink is doing we should just be able to skip
> acquiring of siglock if PF_EXITING is set.

We can even remove it from release_task() path, this is simple.

> __exit_signal is a little more interesting but half of what it is
> doing looks like it was pulled out of do_exit and just needs to
> be put back.

That is. I think we should actually unhash the exiting sub-thread even
if it is traced. IOW, remove it from thread/pid/parent/etc lists and
nullify its ->sighand. IMO, whatever we do thread_group_empty(current)
should be true after exec.

So the exiting sub-trace should look almost a EXIT_DEAD task except it
still should report to debugger.

But this is dangerous. Say, wait4(upid <= 0) becomes unsafe because
task_pid_type(PIDTYPE_PGID) won't work.

> Which probably adds up to 4 or 5 small carefully written patches to sort
> out that part of the exit path,

Perhaps I am wrong, but I think you underestimate the problems, and it is
not clear to me if we really want this.

=========================================================================
Anyway, Eric, even if we can and want to do this, why we can't do this on
top of my fix?

I simply fail to understand why you dislike it that much. Yes it is not
pretty, I said this many times, but it is safe in that it doesn't really
change the current behaviour.

I am much more worried about 2/2 you didn't argue with, this patch _can_
break something and this is obviously not good even if PTRACE_EVENT_EXIT
was always broken.

Oleg.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [RFC][PATCH] exec: Don't wait for ptraced threads to be reaped. ebiederm@xmission.com (Eric W. Biederman) - 2017-03-30 10:20 +0200
  [RFC][PATCH 0/2] exec: Fixing ptrace'd mulit-threaded hang ebiederm@xmission.com (Eric W. Biederman) - 2017-04-01 07:20 +0200
    [RFC][PATCH 1/2] sighand: Count each thread group once in sighand_struct ebiederm@xmission.com (Eric W. Biederman) - 2017-04-01 07:20 +0200
    [RFC][PATCH 2/2] exec: If possible don't wait for ptraced threads to be reaped ebiederm@xmission.com (Eric W. Biederman) - 2017-04-01 07:30 +0200
      Re: [RFC][PATCH 2/2] exec: If possible don't wait for ptraced  threads to be reaped Oleg Nesterov <oleg@redhat.com> - 2017-04-02 17:40 +0200
        Re: [RFC][PATCH 2/2] exec: If possible don't wait for ptraced threads to be reaped ebiederm@xmission.com (Eric W. Biederman) - 2017-04-02 21:00 +0200
          Re: [RFC][PATCH 2/2] exec: If possible don't wait for ptraced  threads to be reaped Oleg Nesterov <oleg@redhat.com> - 2017-04-03 20:20 +0200
            Re: [RFC][PATCH 2/2] exec: If possible don't wait for ptraced threads to be reaped ebiederm@xmission.com (Eric W. Biederman) - 2017-04-03 23:20 +0200
              Re: [RFC][PATCH 2/2] exec: If possible don't wait for ptraced         threads to be reaped Oleg Nesterov <oleg@redhat.com> - 2017-04-05 18:50 +0200
    Re: [RFC][PATCH 0/2] exec: Fixing ptrace'd mulit-threaded hang Oleg Nesterov <oleg@redhat.com> - 2017-04-02 17:40 +0200
    [RFC][PATCH v2 4/5] exec: If possible don't wait for ptraced threads to be reaped ebiederm@xmission.com (Eric W. Biederman) - 2017-04-03 01:00 +0200
      Re: [RFC][PATCH v2 4/5] exec: If possible don't wait for ptraced         threads to be reaped Oleg Nesterov <oleg@redhat.com> - 2017-04-05 18:20 +0200
    [RFC][PATCH v2 1/5] ptrace: Don't wait in PTRACE_O_TRACEEXIT for exec or coredump ebiederm@xmission.com (Eric W. Biederman) - 2017-04-03 01:00 +0200
      Re: [RFC][PATCH v2 1/5] ptrace: Don't wait in PTRACE_O_TRACEEXIT         for exec or coredump Oleg Nesterov <oleg@redhat.com> - 2017-04-05 18:30 +0200
    [RFC][PATCH v2 0/5] exec: Fixing ptrace'd mulit-threaded hang ebiederm@xmission.com (Eric W. Biederman) - 2017-04-03 01:00 +0200
      [RFC][PATCH v2 3/5] clone: Disallown CLONE_THREAD with a shared sighand_struct ebiederm@xmission.com (Eric W. Biederman) - 2017-04-03 01:00 +0200
        Re: [RFC][PATCH v2 3/5] clone: Disallown CLONE_THREAD with a         shared sighand_struct Oleg Nesterov <oleg@redhat.com> - 2017-04-05 18:30 +0200
          Re: [RFC][PATCH v2 3/5] clone: Disallown CLONE_THREAD with a shared sighand_struct ebiederm@xmission.com (Eric W. Biederman) - 2017-04-05 19:50 +0200
            Re: [RFC][PATCH v2 3/5] clone: Disallown CLONE_THREAD with a         shared sighand_struct Oleg Nesterov <oleg@redhat.com> - 2017-04-05 20:20 +0200
      [RFC][PATCH v2 2/5] sighand: Count each thread group once in sighand_struct ebiederm@xmission.com (Eric W. Biederman) - 2017-04-03 01:00 +0200
      [RFC][PATCH v2 5/5] signal: Don't allow accessing signal_struct by old threads after exec ebiederm@xmission.com (Eric W. Biederman) - 2017-04-03 01:10 +0200
        Re: [RFC][PATCH v2 5/5] signal: Don't allow accessing         signal_struct by old threads after exec Oleg Nesterov <oleg@redhat.com> - 2017-04-05 18:20 +0200
          Re: [RFC][PATCH v2 5/5] signal: Don't allow accessing signal_struct by old threads after exec ebiederm@xmission.com (Eric W. Biederman) - 2017-04-05 20:30 +0200
            Re: [RFC][PATCH v2 5/5] signal: Don't allow accessing signal_struct  by old threads after exec Oleg Nesterov <oleg@redhat.com> - 2017-04-06 18:00 +0200
  Re: [RFC][PATCH] exec: Don't wait for ptraced threads to be reaped. Oleg Nesterov <oleg@redhat.com> - 2017-04-02 18:20 +0200
    Re: [RFC][PATCH] exec: Don't wait for ptraced threads to be reaped. ebiederm@xmission.com (Eric W. Biederman) - 2017-04-02 23:20 +0200
      Re: [RFC][PATCH] exec: Don't wait for ptraced threads to be reaped. Oleg Nesterov <oleg@redhat.com> - 2017-04-03 20:40 +0200
        scope of cred_guard_mutex. ebiederm@xmission.com (Eric W. Biederman) - 2017-04-04 01:00 +0200
          Re: scope of cred_guard_mutex. Oleg Nesterov <oleg@redhat.com> - 2017-04-05 18:10 +0200
            Re: scope of cred_guard_mutex. Kees Cook <keescook@chromium.org> - 2017-04-05 18:20 +0200
            Re: scope of cred_guard_mutex. ebiederm@xmission.com (Eric W. Biederman) - 2017-04-05 20:10 +0200
              Re: scope of cred_guard_mutex. Oleg Nesterov <oleg@redhat.com> - 2017-04-05 20:20 +0200
          Re: scope of cred_guard_mutex. Oleg Nesterov <oleg@redhat.com> - 2017-04-06 18:00 +0200
            Re: scope of cred_guard_mutex. Kees Cook <keescook@chromium.org> - 2017-04-08 00:10 +0200
        Re: [RFC][PATCH] exec: Don't wait for ptraced threads to be reaped. ebiederm@xmission.com (Eric W. Biederman) - 2017-04-04 01:00 +0200

csiph-web