From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:48750 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932299AbcKDRqx (ORCPT ); Fri, 4 Nov 2016 13:46:53 -0400 Date: Fri, 4 Nov 2016 19:45:05 +0100 From: Oleg Nesterov To: "Eric W. Biederman" Cc: Jann Horn , Alexander Viro , Roland McGrath , John Johansen , James Morris , "Serge E. Hallyn" , Paul Moore , Stephen Smalley , Eric Paris , Casey Schaufler , Kees Cook , Andrew Morton , Janis Danisevskis , Seth Forshee , Thomas Gleixner , Benjamin LaHaise , Ben Hutchings , Andy Lutomirski , Linus Torvalds , Krister Johansen , linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, security@kernel.org Subject: Re: [PATCH v3 1/8] exec: introduce cred_guard_light Message-ID: <20161104184505.GA21320@redhat.com> References: <1477863998-3298-1-git-send-email-jann@thejh.net> <1477863998-3298-2-git-send-email-jann@thejh.net> <20161102181806.GB1112@redhat.com> <20161102205011.GF8196@pc.thejh.net> <20161103181225.GA11212@redhat.com> <87k2cj2x6j.fsf@xmission.com> <87k2cjuw6h.fsf@xmission.com> <20161104180416.GA19221@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161104180416.GA19221@redhat.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 11/04, Oleg Nesterov wrote: > > On 11/04, Eric W. Biederman wrote: > > > > The following mostly correct patch modifies zap_other_threads in > > the case of a de_thread to not wait for zombies to be reaped. The only > > case that cares is ptrace (as threads are self reaping). So I don't > > think this will cause any problems except removing the strace -f race. > > From my previous email: > > So the only plan I currently have is change de_thread() to wait until > other threads pass exit_notify() or even exit_signals(), but I don't > like this. > > And yes, I don't like this, but perhaps this is what we should do. > > The patch is incomplete and racy (afaics), and the SIGNAL_GROUP_EXIT > checks doesn't look right, but off course technically this change should > be simple enough. > > But not that simple. Just for example, the exiting sub-threads should > not run with ->group_leader pointing to nowhere, in case it was reaped > by de_thread. Not to mention other potential problems outside of ptrace/exec. For example userns_install() can fail after mt-exec even without ptrace, simply because thread_group_empty() can be false. Sure, easy to fix, and probably _install() should use signal->live anyway, but still. And I didn't mention the fun with sighand unsharing. We simply can't do this until all sub-threads go away. IOW, your patch breaks the usage of ->siglock. The execing thread and the zombie threads will use different locks to, say, remove the task from thread-group. Again, this is fixable, but not that simple. > And we have another problem with PTRACE_EVENT_EXIT which can lead to the > same deadlock. Unfortunately, the semantics of PTRACE_EVENT_EXIT was never > defined. But this change will add the user-visible change. > > And if we add the user-visible changes, then perhaps we could simply untrace > the traced sub-threads on exec. This change is simple, we do not even need > to touch exec/de_thread, we could just change exit_notify() to ignore ->ptrace > if exec is in progress. But I'm afraid we can't do this. Eric, I hope you see my emails, I got the "Undelivered Mail Returned to Sender" ... This is the mail system at host mail.kernel.org. ... (expanded from ): host mx.xmission.com[166.70.12.20] said: 550-XM-RJCT16: SPF Failure (ip=198.145.29.136, frm=oleg@redhat.com, 550 result=fail) (in reply to RCPT TO command) right now I have no idea what does this mean. Oleg.