From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753540AbbKWMzH (ORCPT ); Mon, 23 Nov 2015 07:55:07 -0500 Received: from out02.mta.xmission.com ([166.70.13.232]:57675 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752291AbbKWMzB (ORCPT ); Mon, 23 Nov 2015 07:55:01 -0500 From: ebiederm@xmission.com (Eric W. Biederman) To: Jan Kara Cc: Kees Cook , linux-kernel@vger.kernel.org, Andrew Morton , Dave Chinner , Andy Lutomirski , "Kirill A. Shutemov" , Mel Gorman , Johannes Weiner , Rik van Riel , Matthew Wilcox , Shachar Raindel , Boaz Harrosh , Michal Hocko , Haggai Eran , Theodore Tso , Willy Tarreau , Dirk Steinmetz , Michael Kerrisk-manpages , Serge Hallyn , Seth Forshee , Alexander Viro , Linux FS Devel , Serge Hallyn , linux-mm@kvack.org References: <20151120001043.GA28204@www.outflux.net> <20151123122624.GI23418@quack.suse.cz> Date: Mon, 23 Nov 2015 06:34:06 -0600 In-Reply-To: <20151123122624.GI23418@quack.suse.cz> (Jan Kara's message of "Mon, 23 Nov 2015 13:26:24 +0100") Message-ID: <87lh9odhdt.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-AID: U2FsdGVkX1+59g1sQo/o3hr74J6by8Z7/aU7BduP+Ds= X-SA-Exim-Connect-IP: 67.3.201.231 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 1.5 TR_Symld_Words too many words that have symbols inside * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 1397; Body=1 Fuz1=1 Fuz2=1] * 0.1 XMSolicitRefs_0 Weightloss drug X-Spam-DCC: XMission; sa06 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: *;Jan Kara X-Spam-Relay-Country: X-Spam-Timing: total 890 ms - load_scoreonly_sql: 0.05 (0.0%), signal_user_changed: 3.7 (0.4%), b_tie_ro: 2.5 (0.3%), parse: 0.99 (0.1%), extract_message_metadata: 22 (2.4%), get_uri_detail_list: 2.1 (0.2%), tests_pri_-1000: 7 (0.8%), tests_pri_-950: 1.24 (0.1%), tests_pri_-900: 1.10 (0.1%), tests_pri_-400: 32 (3.6%), check_bayes: 31 (3.5%), b_tokenize: 13 (1.5%), b_tok_get_all: 8 (0.9%), b_comp_prob: 3.5 (0.4%), b_tok_touch_all: 3.6 (0.4%), b_finish: 0.73 (0.1%), tests_pri_0: 812 (91.3%), tests_pri_500: 4.6 (0.5%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH] fs: clear file set[ug]id when writing via mmap X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Wed, 24 Sep 2014 11:00:52 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Jan Kara writes: > On Thu 19-11-15 16:10:43, Kees Cook wrote: >> Normally, when a user can modify a file that has setuid or setgid bits, >> those bits are cleared when they are not the file owner or a member of the >> group. This is enforced when using write() directly but not when writing >> to a shared mmap on the file. This could allow the file writer to gain >> privileges by changing the binary without losing the setuid/setgid bits. >> >> Signed-off-by: Kees Cook >> Cc: stable@vger.kernel.org > > So I had another look at this and now I understand why we didn't do it from > the start: > > To call file_remove_privs() safely, we need to hold inode->i_mutex since > that operations is going to modify file mode / extended attributes and > i_mutex protects those. However we cannot get i_mutex in the page fault > path as that ranks above mmap_sem which we hold during the whole page > fault. > > So calling file_remove_privs() when opening the file is probably as good as > it can get. It doesn't catch the case when suid bits / IMA attrs are set > while the file is already open but I don't see easy way around this. Could we perhaps do this on mmap MAP_WRITE instead of open, and simply deny adding these attributes if a file is mapped for write? That would seem to be a little more compatible with what we already do, and guards against the races you mention as well. Eric