Received: by 2002:a25:c205:0:0:0:0:0 with SMTP id s5csp2442995ybf; Mon, 2 Mar 2020 08:43:57 -0800 (PST) X-Google-Smtp-Source: ADFU+vvqAb+J4UF1MzZZPv3iGfTDWkF499DJlJhwgOOmBrfx+8H05SEBIk7AAHemr1tCMtqiuMkl X-Received: by 2002:a05:6830:3148:: with SMTP id c8mr26301ots.359.1583167437811; Mon, 02 Mar 2020 08:43:57 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1583167437; cv=none; d=google.com; s=arc-20160816; b=JLd4Y4nbYU3gFsRW3oIKmA308rZvJPtLh2nxI7WrRTkAOpbmiY4Zg6+YdW/OPAipib K6Z2Sr/B8P1pxWpPIATXBnk+K09FtybFoGp6hOTVv9D5eFeeO7jr3PUBlIt77Igc7DgH 6KoUQHvHhtNZezW98fehIynSaFjxmna//GmLdgXYooOSZx2kvP7gwDVoeEJdQP9kEIen XEgg+0zxL1DgMCneMHwHR8nh90T6MMQNZsDNzECKcsscb7qlb4yUK5HFhatWHXQ6C+dq nxAIe0A+lxBrRsMQCghYWBHNZ8U0iEGgP9sPzrmntw8XxTlJfMJ2VkF8Sj0N1XXSVLU7 hNdQ== 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=7kNUg6jrLRYXBxRwxs+v6id6LKMnX/sXkRxwLGsubN8=; b=tzTLwglSHQf0ocXMZ+A2cEnPxReumwO96YvgOMdAIWi8MBLptujWkq6gGJii5jNFFl W0EmowgxjL29HVvyVYa3dZCytbTTCL0TYWLwbldxuQCCZksrY1JG9nzI6bXnrJkdPjf2 XFHI77lxIJ6XcX5K8dCqs44ZV4jOVVdyupVjO2J4ER+tybNKoU52iY0aW3QTxGedLYpx 3leaBBdgyP3IqZOtNuCPX0ePPvctBdeSgWOyVKwjVqRjFcA3O0Fh2TfzLyNqK3iq+Xmg CSM6e3UIbppyg4ew3lHl+62rVTIZ/u7JSnsWo0sjdXFGyP1RPTBZjUU67aFPYbO0mTuM lzdA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=G85budQa; 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 i20si6497267ota.82.2020.03.02.08.43.45; Mon, 02 Mar 2020 08:43:57 -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=G85budQa; 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 S1727306AbgCBQne (ORCPT + 99 others); Mon, 2 Mar 2020 11:43:34 -0500 Received: from mail-oi1-f193.google.com ([209.85.167.193]:35659 "EHLO mail-oi1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726997AbgCBQne (ORCPT ); Mon, 2 Mar 2020 11:43:34 -0500 Received: by mail-oi1-f193.google.com with SMTP id b18so10969870oie.2 for ; Mon, 02 Mar 2020 08:43:32 -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=7kNUg6jrLRYXBxRwxs+v6id6LKMnX/sXkRxwLGsubN8=; b=G85budQaXfEQvDNWIalbDXyWPGbmN1VgzYIvybMkE6FjzEE9OQ06bKVGmxbSFZLDeZ NCG1SEAjzWEvnPaBKjkeXOiCbefYDCeZuZT/wMi0k0Z8lRMg1ptUkwT9yyguQl1/2S+o jfJxI20+jwJY/A2UlJ9u9Qil4ApszAgT38+jjcvSvG9xoofpBq921uBU19jjtE1+amxO hwunfbFnfCp0nj6pojNjexPVGNkT+8IQLC8KySny/q03clG9aJQnNSB7rlhWMZOdoDcs Tlc1qt9VorvqgYNwFtqnCnSE57RGtJfTiH7WPMYBRl0+W5TnUTvBhMda5uGFAqYj+Yii VzMw== 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=7kNUg6jrLRYXBxRwxs+v6id6LKMnX/sXkRxwLGsubN8=; b=Xub7RrtAEpNEd4HSl2aGZSP8CK7Hrpq4O/c7D4PA87Do8x7+aqe0e9zjYrGX7zlV2U m6mN4SBpbbJcz1YDQ4N8j8cNmR7Za9we08OSbsiSvRQMUtgwXNeyonmtqSGzk6EtP54e XrUrd5r/Zb2tzAOIPhrNt0qUiHMf+N0GCiZgLSyUW1vNeutvmIRWWsGw0RMF/XL1nNmv +KUJBd+DPA2wmPGOUXMKOuj3GCRe/kxLy+fJ+3h+yOSj84wz+FUSiPAEpMfiP+8JpYRh FNOm9UoZv64xtlZN/pyW0WY1KfpyTVPl6XuDj7qp7ZdY7cLq6nU+1P1REIkEbf1xKSC8 1pMw== X-Gm-Message-State: ANhLgQ0EYGqDrynW/UA6fE5W0tOP82zSRz8Im95dCl1rqtv9c4S5e2AE Nx/uDt0fLG4arkB2kns6LxGZXHr+prorEdTsxmYgFw== X-Received: by 2002:a05:6808:8d0:: with SMTP id k16mr12043oij.68.1583167412109; Mon, 02 Mar 2020 08:43:32 -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: <875zfmloir.fsf@x220.int.ebiederm.org> From: Jann Horn Date: Mon, 2 Mar 2020 17:43:05 +0100 Message-ID: Subject: Re: [PATCHv2] exec: Fix a deadlock in ptrace To: "Eric W. Biederman" , James Morris Cc: Bernd Edlinger , 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 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.