From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753951AbdBTPvm (ORCPT ); Mon, 20 Feb 2017 10:51:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42022 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753617AbdBTPvk (ORCPT ); Mon, 20 Feb 2017 10:51:40 -0500 Date: Mon, 20 Feb 2017 16:50:06 +0100 From: Oleg Nesterov To: "Eric W. Biederman" Cc: Andrew Morton , Mika =?iso-8859-1?Q?Penttil=E4?= , Aleksa Sarai , Andy Lutomirski , Attila Fazekas , Jann Horn , Kees Cook , Michal Hocko , Ulrich Obergfell , linux-kernel@vger.kernel.org, David Howells Subject: Re: [PATCH V2 1/2] exec: don't wait for zombie threads with cred_guard_mutex held Message-ID: <20170220155005.GA15036@redhat.com> References: <20170213141452.GA30203@redhat.com> <20170213141516.GA30233@redhat.com> <20170213180454.GA2858@redhat.com> <87k28po2ce.fsf@xmission.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87k28po2ce.fsf@xmission.com> User-Agent: Mutt/1.5.18 (2008-05-17) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Mon, 20 Feb 2017 15:51:41 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/17, Eric W. Biederman wrote: > > Oleg Nesterov writes: > > > de_thread() waits for other threads with ->cred_guard_mutex held and this > > is really bad because the time is not bounded, debugger can delay the exit > > and this lock has a lot of users (mostly abusers imo) in fs/proc and more. > > And this leads to deadlock if debugger tries to take the same mutex: > > Oleg. I looked at the history in proc of users of cred_guard_mutex > and the proc users are grabbing cred_guard_mutex for the proper > semantic reasons. To avoid races with setuid exec that could result in > an information disclosure. This is clear. However I really think it is over-used. For example, I do think that lock_trace() should die. OK, of course I can be wrong. And in any case this is almost off-topic right now, lets discuss this separately. > On that score I believe we can incrementally approach that point and > only grab the cred_guard_mutex in exec if we are performing an exec that > changes the processes credentials. May be... but afaics this is more complicated because of LSM hooks. To me one the main problems is that lsm hook can fail if the task is traced, that is why we can't simply take cred_guard_mutex and check ->ptrace after de_thread(). But again, lets discuss this later. > Right now I don't think it introduces any new security information > disclosures but the moving of flush_signal_handlers outside of > cred_guard_mutex feels wrong. Why? Just in case, we can do flush_signal_handlers() only after we unshare ->sighand, that is why it was moved outside of cred_guard_mutex. But why this is bad? collect_sigign_sigcatch() is called without this mutex, who else can look at sa.sa_handler? Oleg.