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

Groups > linux.kernel > #1615605

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

Show all headers | View raw

Oleg Nesterov <oleg@redhat.com> writes:

> Eric,
>
> I see another series from you, but I simply failed to force myself to read
> it carefully. Because at first glance it makes me really sad, I do dislike
> it even if it is correct. Yes, yes, sure, I can be wrong. Will try
> tomorrow.

Yes.  I needed to get my thoughts concrete.  I missed fixing the race in
zap_other_threads.  But overall I think things are moving in a good
direction.

>>
>> I don't know who actually useses PTRACE_O_TRACEEXIT so I don't actually
>> know what the implications of changing it are.  Let's see...
>
> And nobody knows ;) This is the problem, even the clear ptrace bugfix can
> break something, this happened before and we had to revert the obviously-
> correct patches; the bug was already used as feature.

Yes that is the challenge of changing userspace.   Which is why it helps
to test as much of a userspace change as possible.  Or to get very
clever, and figure out how to avoid the userspace change.

So I think it is worth knowing the lldb actually uses
PTRACE_O_TRACEEXIT.  So we can test at least some programs to verify
that all is well.

I don't see any way around cleaning up PTRACE_O_TRACEEXIT.  As
we fundamentally have the non-thread-group-leader exec problem.
We have to reap that previous leader thread with release_task.
Which means we can't stop for a PTRACE_O_TRACEEXIT.


>> If delivering a second SIGKILL
> ...
>> So userspace can absolutely kill a processes in PTRACE_EVENT_EXIT
>> before the tracers find it.
>>
>> Therefore we are only talking a quality of implementation issue
>> if we actually stop and wait for the tracer or not.
>
> Oh, this is another story, needs another discussion. We really need some
> changes in this area, we need to distinguish SIGKILL sent from user-space
> and (say) from group-exit, and we need to decide when should we stop.
>
> But at least I think the tracee should never stop if SIGKILL comes from
> user space. And yes ptrace_stop() is ugly and wrong, just look at the
> arch_ptrace_stop_needed() check. The problem, again, is that any fix will
> be user-visible.

The only issue I see is that arch_ptrace_stop() may sleep (sparc and
ia64 do as they flush the register stack to memory).  As the
code may sleep it means we can't set TASK_TRACED until after calling
arch_ptrace_stop().

My inclination is to just solve that by saying:
if (!sigkill_pending(current))
	set_current_task(TASK_TRACED);

That removes the special case.  We have to handle SIGKILL being
delivered immediately after set_current_state in any event.  And as we
are talking about something that happens on rare architecutres I don't
see any problem with tweaking that code at all.

It is closely enough related I will fold that into the next version of
my patch.

Eric

Back to linux.kernel | Previous | Next — Previous 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