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=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 A0BE7C18E5C for ; Tue, 10 Mar 2020 21:27:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 849D224649 for ; Tue, 10 Mar 2020 21:27:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727661AbgCJV1b (ORCPT ); Tue, 10 Mar 2020 17:27:31 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:40600 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726265AbgCJV1a (ORCPT ); Tue, 10 Mar 2020 17:27:30 -0400 Received: from in01.mta.xmission.com ([166.70.13.51]) by out01.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jBmPQ-0008Ng-W8; Tue, 10 Mar 2020 15:27:29 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jBmPQ-0003ht-8O; Tue, 10 Mar 2020 15:27:28 -0600 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: <87v9ne5y4y.fsf_-_@x220.int.ebiederm.org> <87zhcq4jdj.fsf_-_@x220.int.ebiederm.org> <878sk94eay.fsf@x220.int.ebiederm.org> <87r1y12yc7.fsf@x220.int.ebiederm.org> <87k13t2xpd.fsf@x220.int.ebiederm.org> <87d09l2x5n.fsf@x220.int.ebiederm.org> <871rq12vxu.fsf@x220.int.ebiederm.org> <877dzt1fnf.fsf@x220.int.ebiederm.org> <875zfcxlwy.fsf@x220.int.ebiederm.org> <874kuwvxkz.fsf@x220.int.ebiederm.org> Date: Tue, 10 Mar 2020 16:25:09 -0500 In-Reply-To: (Bernd Edlinger's message of "Tue, 10 Mar 2020 21:19:05 +0100") Message-ID: <878sk7q4wa.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=1jBmPQ-0003ht-8O;;;mid=<878sk7q4wa.fsf@x220.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+R1YozAbyKQHiEXFA7wXLQCvRAwXr7aX0= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH 3/4] proc: io_accounting: Use new infrastructure to fix deadlocks in execve X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.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: > On 3/10/20 8:06 PM, Eric W. Biederman wrote: >> Bernd Edlinger writes: >> >>> This changes do_io_accounting to use the new exec_update_mutex >>> instead of cred_guard_mutex. >>> >>> This fixes possible deadlocks when the trace is accessing >>> /proc/$pid/io for instance. >>> >>> This should be safe, as the credentials are only used for reading. >> >> This is an improvement. >> >> We probably want to do this just as an incremental step in making things >> better but perhaps I am blind but I am not finding the reason for >> guarding this with the cred_guard_mutex to be at all persuasive. >> >> I think moving the ptrace_may_access check down to after the >> unlock_task_sighand would be just as effective at addressing the >> concerns raised in the original commit. I think the task_lock provides >> all of the barrier we need to make it safe to move the ptrace_may_access >> checks safe. >> >> The reason I say this is I don't see exec changing ->ioac. Just >> performing some I/O which would update the io accounting statistics. >> > > Maybe the suid executable is starting up and doing io or not, > and what the program does immediately at startup is a secret, > that we want to keep secret but evil eve want to find out. > eve is using /proc/alice/io to do that. > > It is a bit constructed, but seems like a security concern. > when we keep the exec_update_mutex while collecting the data, we > cannot see any io of the new process when the new credentials > don't allow that. Jann Horn has convinced me we should just convert these to the exec_change_mutex today. Because while not 100% correct in theory, the only really interesting case is exec. So the code does something interesting and worth while, and mostly correct. The last thing I want to do is to cause an unnecessary regression. Eric