Received: by 2002:a25:c205:0:0:0:0:0 with SMTP id s5csp2489184ybf; Mon, 2 Mar 2020 09:38:15 -0800 (PST) X-Google-Smtp-Source: ADFU+vtsFvRuQsyL2+Tca3EF3b9VslCiHrvRY3/Ef2pnWGSgO1FAafbmPNUjFo7SPit+Op8nA2ZN X-Received: by 2002:a9d:69ce:: with SMTP id v14mr244922oto.248.1583170694870; Mon, 02 Mar 2020 09:38:14 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1583170694; cv=none; d=google.com; s=arc-20160816; b=psw8E9FbWL0hgDGzhR6doBj14xddzFvq9NbWJIyafJgppsUeF+dPJlRGdniEEjJbFZ N44eRdJX8WKXs0z3fQWd8yS9Nz+d2WnYd5yM288k6riTN8yKvHpbBpcdTqQ35gnksuDn qtRwG4ejzwyDx4A6y3Y5janTjIpJolkwdv5YI8arAr5FmhxIz09ARgm2yn/EB+m7SLad bUuP19YEmtbo1KqcqJzuSXHTOAm3wlCt5eUzRQBScGRO8DaX/yX4yQdc7AiQYMjsI8KE 2OSt/IdPhC5bywlw2q6PFZrCjHKRi1Y9lETPVVZpfcRBpp9V0pPbAoEaew9d0xYnBKqO MK1w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=/ZAKVb1y956pqysbIHLXaqWd9Ys3FR5AQaat5ayTW2U=; b=uTcgsBsQHVBYlqaEDxOVgorwYAcxi3kqG9YELPbjGt7vQzWBpndBDzzt1m93r/INv8 9fSDxZ7WcV8KLIh5L9hC23ufLtflOp1BoCiyDNv0pVfjUW2C1d0xohA4nj6Y+5DTYlW4 yoK0hEiaBLGTRx84OTGl/woK1xUb1xZdC6wzGG5meRICnUGsri78p+uNGt6C03t1aTDb B+6JHEdpqfFaSZuqlzYaU8DzC0w+OThkPmLKlCm+v7cdcsuIa/tOu3dmv4pYdmTSYfJX jmPtA++bunPMuBmadBCULZQIRl4bGSRktT40wIM2F4KTTkU0884hqQvOnsxpxxQRRuot 1Yww== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=tvXmHHsd; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id s6si6885701otq.115.2020.03.02.09.38.01; Mon, 02 Mar 2020 09:38:14 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=tvXmHHsd; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727209AbgCBRhz (ORCPT + 99 others); Mon, 2 Mar 2020 12:37:55 -0500 Received: from mail-ot1-f67.google.com ([209.85.210.67]:45057 "EHLO mail-ot1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726997AbgCBRhz (ORCPT ); Mon, 2 Mar 2020 12:37:55 -0500 Received: by mail-ot1-f67.google.com with SMTP id f21so14529otp.12 for ; Mon, 02 Mar 2020 09:37:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=/ZAKVb1y956pqysbIHLXaqWd9Ys3FR5AQaat5ayTW2U=; b=tvXmHHsdwC2ZHXs+RdxnZLvNNTOIQtXQo8+c4NIG9SwSrx3X2qfM4PFM54/x5xFwJp cfFlhvQc0zBKaRaTBUYJdf4weoUwAZE/jWkqSkDz7QQA2gBwflEvwBaARHqOO3jfoZWe H3HzZ3d8YIZfEn42qsIaSNuiAQiMlVeuHwsGHfdJ91w4d3C9kQ6guQceh2DHy3g/ptV8 GD9luNMoxl/wmt0z1ZAyx7TT3Evb8ThhnGs8bzJ/crBzhPs02h3kMtaJVJfzx4+AOqh4 0/3m1NcszFU4y0wqzn15zw+AD7m87pgt2c5Z3r/ZdcO5TYFDuZe7cntLaqgpzosYUE3y +q/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=/ZAKVb1y956pqysbIHLXaqWd9Ys3FR5AQaat5ayTW2U=; b=KunwKsiZnDFx2x0xXovqsPhhxNoUtA1g4t7dDZQve2bOitFxYWL9j1hMr6PbRF11WO JahZdEjA+990War+5tr89grYcCJWHFFfIKafiCBTQSt4TvDIC0U6gGC7x5TrivuZbs8p ZGQJTifXbv6l9438wAHLdB1ypfKFZCHOWEShpfzNPl2Zv9GcJxBwnNh/azIMseiS5O9U pUCo54ehyv8l/fcS/zZ9+Qmtg+UVy1KjC7Is1UfDzS7gTr8mxldZiuG+m7ac4nDxcteM 2/KkbUCqq0mzytuy7ohz6Ub2xTg5Zu/m7z6MGGp2R/r5SUbidTEKFEB1AzLwR2/vQi1V KgcQ== X-Gm-Message-State: ANhLgQ06vomMz2YhDD0kEyvCjHngoAgFlyQ5AgO+FCf5FsIX81IFhFr9 TVYE27I4r0vPQIMXMeQqfa8Q7e+QhJB0mZb74NtpPA== X-Received: by 2002:a05:6830:1d6e:: with SMTP id l14mr256675oti.32.1583170673984; Mon, 02 Mar 2020 09:37:53 -0800 (PST) MIME-Version: 1.0 References: <20200301185244.zkofjus6xtgkx4s3@wittgenstein> <87a74zmfc9.fsf@x220.int.ebiederm.org> <87k142lpfz.fsf@x220.int.ebiederm.org> <875zfmloir.fsf@x220.int.ebiederm.org> In-Reply-To: From: Jann Horn Date: Mon, 2 Mar 2020 18:37:27 +0100 Message-ID: Subject: Re: [PATCHv2] exec: Fix a deadlock in ptrace To: Bernd Edlinger Cc: "Eric W. Biederman" , James Morris , Christian Brauner , 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 , Kees Cook , 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-security-module Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Mar 2, 2020 at 6:01 PM Bernd Edlinger wrote: > On 3/2/20 5:43 PM, Jann Horn wrote: > > On Mon, Mar 2, 2020 at 5:19 PM Eric W. Biederman wrote: > >> > >> Bernd Edlinger writes: > >> > >>> On 3/2/20 4:57 PM, Eric W. Biederman wrote: > >>>> Bernd Edlinger writes: > >>>> > >>>>> > >>>>> I tried this with s/EACCESS/EACCES/. > >>>>> > >>>>> The test case in this patch is not fixed, but strace does not freeze, > >>>>> at least with my setup where it did freeze repeatable. > >>>> > >>>> Thanks, That is what I was aiming at. > >>>> > >>>> So we have one method we can pursue to fix this in practice. > >>>> > >>>>> That is > >>>>> obviously because it bypasses the cred_guard_mutex. But all other > >>>>> process that access this file still freeze, and cannot be > >>>>> interrupted except with kill -9. > >>>>> > >>>>> However that smells like a denial of service, that this > >>>>> simple test case which can be executed by guest, creates a /proc/$pid/mem > >>>>> that freezes any process, even root, when it looks at it. > >>>>> I mean: "ln -s README /proc/$pid/mem" would be a nice bomb. > >>>> > >>>> Yes. Your the test case in your patch a variant of the original > >>>> problem. > >>>> > >>>> > >>>> I have been staring at this trying to understand the fundamentals of the > >>>> original deeper problem. > >>>> > >>>> The current scope of cred_guard_mutex in exec is because being ptraced > >>>> causes suid exec to act differently. So we need to know early if we are > >>>> ptraced. > >>>> > >>> > >>> It has a second use, that it prevents two threads entering execve, > >>> which would probably result in disaster. > >> > >> Exec can fail with an error code up until de_thread. de_thread causes > >> exec to fail with the error code -EAGAIN for the second thread to get > >> into de_thread. > >> > >> So no. The cred_guard_mutex is not needed for that case at all. > >> > >>>> If that case did not exist we could reduce the scope of the > >>>> cred_guard_mutex in exec to where your patch puts the cred_change_mutex. > >>>> > >>>> I am starting to think reworking how we deal with ptrace and exec is the > >>>> way to solve this problem. > >> > >> > >> I am 99% convinced that the fix is to move cred_guard_mutex down. > > > > "move cred_guard_mutex down" as in "take it once we've already set up > > the new process, past the point of no return"? > > > >> Then right after we take cred_guard_mutex do: > >> if (ptraced) { > >> use_original_creds(); > >> } > >> > >> And call it a day. > >> > >> The details suck but I am 99% certain that would solve everyones > >> problems, and not be too bad to audit either. > > > > Ah, hmm, that sounds like it'll work fine at least when no LSMs are involved. > > > > SELinux normally doesn't do the execution-degrading thing, it just > > blocks the execution completely - see their selinux_bprm_set_creds() > > hook. So I think they'd still need to set some state on the task that > > says "we're currently in the middle of an execution where the target > > task will run in context X", and then check against that in the > > ptrace_may_access hook. Or I suppose they could just kill the task > > near the end of execve, although that'd be kinda ugly. > > > > We have current->in_execve for that, right? > I think when the cred_guard_mutex is taken only in the critical section, > then PTRACE_ATTACH could take the guard_mutex, and look at current->in_execve, > and just return -EAGAIN in that case, right, everybody happy :) It's probably going to mean that things like strace will just randomly fail to attach to processes if they happen to be in the middle of execve... but I guess that works?