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=-8.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no 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 515F6C3F2D5 for ; Sun, 1 Mar 2020 20:00:52 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id F075D246C3 for ; Sun, 1 Mar 2020 20:00:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="geEqLlC1" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F075D246C3 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 563346B0005; Sun, 1 Mar 2020 15:00:51 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 514C06B0006; Sun, 1 Mar 2020 15:00:51 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3DB196B0007; Sun, 1 Mar 2020 15:00:51 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0169.hostedemail.com [216.40.44.169]) by kanga.kvack.org (Postfix) with ESMTP id 22B4F6B0005 for ; Sun, 1 Mar 2020 15:00:51 -0500 (EST) Received: from smtpin20.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id DF0178245578 for ; Sun, 1 Mar 2020 20:00:50 +0000 (UTC) X-FDA: 76547861460.20.verse34_7bc3b4a73d049 X-HE-Tag: verse34_7bc3b4a73d049 X-Filterd-Recvd-Size: 6240 Received: from mail-ot1-f68.google.com (mail-ot1-f68.google.com [209.85.210.68]) by imf19.hostedemail.com (Postfix) with ESMTP for ; Sun, 1 Mar 2020 20:00:50 +0000 (UTC) Received: by mail-ot1-f68.google.com with SMTP id g96so7598128otb.13 for ; Sun, 01 Mar 2020 12:00:50 -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=eSJCogFW+5WEfqSVoqYuW5WPipQ09ya4rXn8XpVI9lk=; b=geEqLlC1SOE3TSZPMu4eY2C7kEQ3iAEu8+MjDZth1yJJEz3aYIl8RZzNGqXKo+fdBK L3piqmGBZ+R6u2jb71FG+otXTb5dexaFNTIMqfdFtBwyUv8J83uWWiWpJL700olDe1KU jdQ95KSTEviW1p++P8rmN10buW8MUjs1Ce5bxgvibleP8FVy2pIqdkf+iVbKv0OxIfep R3lnoXwspaouzFJKvOL2TU0Ri+Z9X4sJEOZJahs/nnjs5Uj7mCy6y8dFFjwiRQtUui/3 PYeYJCJjLDd5jpik81hu7laW8Arud8MEejgRzp/E94E5OSXDgeoA9nO6m2RJMQ1ob9wA F/aQ== 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=eSJCogFW+5WEfqSVoqYuW5WPipQ09ya4rXn8XpVI9lk=; b=b+z0kmx14SKUkIeWnG4wyW6DarY/wKNG7BSbU966NYQ5DGK1KYv6cLHgu13/5iBkGR Q1YWi5JyjU8+TBN/k1kriyGgckMqXMCKhJWuWiWBsc/OVV1s9H3Vwp4Tz4TCR9H7OsNC UJLB1gIqjvzqJl5JfwOc2KwyFlbKSxwTRThOsabO+uuBiniaPQWM+Tqnz6wirQVIz8Ln fPbB4rRpGeqJnSAaD9SKM9Z0kbLQJGfINiQedDzLupeqayYn++HRU7KP6fjgb5CLhzzo aib+vPw0j6AzIP1s5n4cZCYTb1965i6DI30cKXPwIvbvQgvS4NxIvFS7I/maFd8Pscyo ytvA== X-Gm-Message-State: APjAAAU4BIeLJbUzFllMyDhEhSpdqiYA2BNf68aVpPr6kp0IXPr30Gri LtNewTvx7xpn27Tslj9VDR+i9aLdlv9y/FkDdDUUSQ== X-Google-Smtp-Source: APXvYqxlPmGGj6lqZ/+U20/sqfXaMGOTkoLMW6nyRqaPvY8us5RwnrwLaHiGmuMsLhOUI/kEd8nfooddFe4/2Iz0+l8= X-Received: by 2002:a9d:5e8b:: with SMTP id f11mr10976903otl.110.1583092849244; Sun, 01 Mar 2020 12:00:49 -0800 (PST) MIME-Version: 1.0 References: <20200301185244.zkofjus6xtgkx4s3@wittgenstein> In-Reply-To: <20200301185244.zkofjus6xtgkx4s3@wittgenstein> From: Jann Horn Date: Sun, 1 Mar 2020 21:00:22 +0100 Message-ID: Subject: Re: [PATCH] exec: Fix a deadlock in ptrace To: Christian Brauner Cc: Bernd Edlinger , Jonathan Corbet , Alexander Viro , Andrew Morton , Alexey Dobriyan , "Eric W. Biederman" , 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 , 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" Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sun, Mar 1, 2020 at 7:52 PM Christian Brauner wrote: > On Sun, Mar 01, 2020 at 07:21:03PM +0100, Jann Horn wrote: > > On Sun, Mar 1, 2020 at 12:27 PM Bernd Edlinger > > wrote: > > > The proposed solution is to have a second mutex that is > > > used in mm_access, so it is allowed to continue while the > > > dying threads are not yet terminated. > > > > Just for context: When I proposed something similar back in 2016, > > https://lore.kernel.org/linux-fsdevel/20161102181806.GB1112@redhat.com/ > > was the resulting discussion thread. At least back then, I looked > > through the various existing users of cred_guard_mutex, and the only > > places that couldn't be converted to the new second mutex were > > PTRACE_ATTACH and SECCOMP_FILTER_FLAG_TSYNC. > > > > > > The ideal solution would IMO be something like this: Decide what the > > new task's credentials should be *before* reaching de_thread(), > > install them into a second cred* on the task (together with the new > > dumpability), drop the cred_guard_mutex, and let ptrace_may_access() > > check against both. After that, some further restructuring might even > > Hm, so essentially a private ptrace_access_cred member in task_struct? And a second dumpability field, because that changes together with the creds during execve. (Btw, currently the dumpability is in the mm_struct, but that's kinda wrong. The mm_struct is removed from a task on exit while access checks can still be performed against it, and currently ptrace_may_access() just lets the access go through in that case, which weakens the protection offered by PR_SET_DUMPABLE when used for security purposes. I think it ought to be moved over into the task_struct.) > That would presumably also involve altering various LSM hooks to look at > ptrace_access_cred. When I tried to implement this in the past, I changed the LSM hook to take the target task's cred* as an argument, and then called the LSM hook twice from ptrace_may_access(). IIRC having the target task's creds as an argument works for almost all the LSMs, with the exception of Yama, which doesn't really care about the target task's creds, so you have to pass in both the task_struct* and the cred*.