From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1EB58C10F26 for ; Fri, 6 Mar 2020 21:21:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E9B9420705 for ; Fri, 6 Mar 2020 21:21:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726733AbgCFVVF (ORCPT ); Fri, 6 Mar 2020 16:21:05 -0500 Received: from out01.mta.xmission.com ([166.70.13.231]:51586 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726194AbgCFVVD (ORCPT ); Fri, 6 Mar 2020 16:21:03 -0500 Received: from in02.mta.xmission.com ([166.70.13.52]) by out01.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jAKOf-0001bO-Qo; Fri, 06 Mar 2020 14:20:41 -0700 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jAKOe-0007Ua-SE; Fri, 06 Mar 2020 14:20:41 -0700 From: ebiederm@xmission.com (Eric W. Biederman) To: Bernd Edlinger Cc: Christian Brauner , Kees Cook , Jann Horn , Jonathan Corbet , Alexander Viro , Andrew Morton , Alexey Dobriyan , Thomas Gleixner , Oleg Nesterov , Frederic Weisbecker , Andrei Vagin , Ingo Molnar , "Peter Zijlstra \(Intel\)" , Yuyang Du , David Hildenbrand , Sebastian Andrzej Siewior , Anshuman Khandual , David Howells , James Morris , Greg Kroah-Hartman , Shakeel Butt , Jason Gunthorpe , Christian Kellner , Andrea Arcangeli , Aleksa Sarai , "Dmitry V. Levin" , "linux-doc\@vger.kernel.org" , "linux-kernel\@vger.kernel.org" , "linux-fsdevel\@vger.kernel.org" , "linux-mm\@kvack.org" , "stable\@vger.kernel.org" , "linux-api\@vger.kernel.org" References: <202003021531.C77EF10@keescook> <20200303085802.eqn6jbhwxtmz4j2x@wittgenstein> <87v9nlii0b.fsf@x220.int.ebiederm.org> <87a74xi4kz.fsf@x220.int.ebiederm.org> <87r1y8dqqz.fsf@x220.int.ebiederm.org> <87tv32cxmf.fsf_-_@x220.int.ebiederm.org> <87imjicxjw.fsf_-_@x220.int.ebiederm.org> <87k13yawpp.fsf@x220.int.ebiederm.org> Date: Fri, 06 Mar 2020 15:18:20 -0600 In-Reply-To: (Bernd Edlinger's message of "Fri, 6 Mar 2020 11:46:01 +0000") Message-ID: <877dzx9o83.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1jAKOe-0007Ua-SE;;;mid=<877dzx9o83.fsf@x220.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1/H7zIdc5Xdrawq/Ho8JIA/FNvc5wTe1Zg= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH 2/2] exec: Add a exec_update_mutex to replace cred_guard_mutex X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Bernd Edlinger writes: > Am 06.03.20 um 06:17 schrieb Eric W. Biederman: >> Bernd Edlinger writes: >> >>> On 3/5/20 10:16 PM, Eric W. Biederman wrote: >>>> >>>> The cred_guard_mutex is problematic. The cred_guard_mutex is held >>>> over the userspace accesses as the arguments from userspace are read. >>>> The cred_guard_mutex is held of PTRACE_EVENT_EXIT as the the other >>>> threads are killed. The cred_guard_mutex is held over >>>> "put_user(0, tsk->clear_child_tid)" in exit_mm(). >>>> >>>> Any of those can result in deadlock, as the cred_guard_mutex is held >>>> over a possible indefinite userspace waits for userspace. >>>> >>>> Add exec_update_mutex that is only held over exec updating process >>>> with the new contents of exec, so that code that needs not to be >>>> confused by exec changing the mm and the cred in ways that can not >>>> happen during ordinary execution of a process can take. >>>> >>>> The plan is to switch the users of cred_guard_mutex to >>>> exed_udpate_mutex one by one. This lets us move forward while still >>>> being careful and not introducing any regressions. >>>> >>>> Link: https://lore.kernel.org/lkml/20160921152946.GA24210@dhcp22.suse.cz/ >>>> Link: https://lore.kernel.org/lkml/AM6PR03MB5170B06F3A2B75EFB98D071AE4E60@AM6PR03MB5170.eurprd03.prod.outlook.com/ >>>> Link: https://lore.kernel.org/linux-fsdevel/20161102181806.GB1112@redhat.com/ >>>> Link: https://lore.kernel.org/lkml/20160923095031.GA14923@redhat.com/ >>>> Link: https://lore.kernel.org/lkml/20170213141452.GA30203@redhat.com/ >>>> Ref: 45c1a159b85b ("Add PTRACE_O_TRACEVFORKDONE and PTRACE_O_TRACEEXIT facilities.") >>>> Ref: 456f17cd1a28 ("[PATCH] user-vm-unlock-2.5.31-A2") >>>> Signed-off-by: "Eric W. Biederman" >>>> --- >>>> fs/exec.c | 4 ++++ >>>> include/linux/sched/signal.h | 9 ++++++++- >>>> kernel/fork.c | 1 + >>>> 3 files changed, 13 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/fs/exec.c b/fs/exec.c >>>> index c243f9660d46..ad7b518f906d 100644 >>>> --- a/fs/exec.c >>>> +++ b/fs/exec.c >>>> @@ -1182,6 +1182,7 @@ static int de_thread(struct linux_binprm *bprm, struct task_struct *tsk) >>>> release_task(leader); >>>> } >>>> >>>> + mutex_lock(¤t->signal->exec_update_mutex); > > And by the way, could you make this mutex_lock_killable? For some reason when I first read this suggestion I thought making this mutex_lock_killable would cause me to rework the logic of when I set unrecoverable and when I unlock the mutex. I blame a tired brain. If a process has received a fatal signal none of that matters. So yes I will do that just to make things robust in case we miss something that would still make it possible to deadlock in with the new mutex. I am a little worried that the new mutex might still cover a little too much. But past a certain point I we are not being able to make this code perfect in the first change. The best we can do is to be careful and avoid regressions. Whatever slips through we can fix when we spot the problem. Eric