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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 44086C83F10 for ; Thu, 31 Aug 2023 12:36:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B84BC8D000D; Thu, 31 Aug 2023 08:36:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B34F18D0001; Thu, 31 Aug 2023 08:36:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A243A8D000D; Thu, 31 Aug 2023 08:36:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 9267F8D0001 for ; Thu, 31 Aug 2023 08:36:19 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 6B9541C96EB for ; Thu, 31 Aug 2023 12:36:19 +0000 (UTC) X-FDA: 81184347678.08.C2A433F Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf21.hostedemail.com (Postfix) with ESMTP id 820F71C0004 for ; Thu, 31 Aug 2023 12:36:17 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=R7qpQa4M; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf21.hostedemail.com: domain of brauner@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=brauner@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1693485377; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=neUHtkfXFnL3Og1q48F0weQM/KSL7WzrNQMdkudcGK0=; b=wV3CIh5H4C3M3HfPB9O8GKeRVwvdZnwz7RLelE7dqW8IUW41tbIXLEyyPYwTfIlRE30Uww znQvqUOoes06HK6wE/9BDQ/P/YN1x9ugKDcjG83wJMq9QL4Ox6gvqwYykYSB9wbC2VGI/q BxgDlNZHczz1deCksq/C1RBnPNW+AOc= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=R7qpQa4M; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf21.hostedemail.com: domain of brauner@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=brauner@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1693485377; a=rsa-sha256; cv=none; b=CZD2KrItBndLE8QG2fQ1qh+8+cUBli6FW2iA/Eh0V1yntRHoZJ/8jloxzatAXPuA/jMm9b ejiUUv1vy8by5Irw76LVUrGsgUN/VTsfJa9382HDJ2O0svJV8f0mmYwjE5pN2GvLkERdas rf9LA2tcA53NzNyGfRAzyqUGHQ6ff/w= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id A12E5B82171; Thu, 31 Aug 2023 12:36:15 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3D009C433C7; Thu, 31 Aug 2023 12:36:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1693485374; bh=YTE6VXGsNhwwnE1WRR+jiKmCFv8IQTGmHPP7b83zi08=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=R7qpQa4MA07HylLZFu3oc5ztNx14fNsvCYu1KK6a5nAmk+n7tAMXgnUofERjdizTO DfIiK2nFNw9zwW/bbS/+/He6ohZkSi3aFOf1XOzFIv0HcgfnpHmyGFDucR80Q8Vmrc SMonwuIHUUMXY197yN9LbfDK/Wc8A6dyCqqJ70Cg575G1MvBi6w0THyIvafx+RRz/j mU51da11VPhIt4oao2mtzHaXXNHTyax47nDwfUcJupUNwQvEGgGq8yiMl8nBV83Vh9 wfqzDlfl/iWJ09heVj+tEElA8ghwIR3eSW59EyUBlcNuL6bUsuT4wcl9FgvU/QdwZQ Nn6VrGHvTAULw== Date: Thu, 31 Aug 2023 14:36:09 +0200 From: Christian Brauner To: Hugh Dickins Cc: Paul Moore , Mimi Zohar , Al Viro , Andrew Morton , linux-security-module@vger.kernel.org, linux-fsdevel@vger.kernel.org, selinux@vger.kernel.org, linux-mm@kvack.org, linux-integrity@vger.kernel.org Subject: Re: LSM hook ordering in shmem_mknod() and shmem_tmpfile()? Message-ID: <20230831-nachverfolgen-meditation-dcde56b10df7@brauner> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 820F71C0004 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: 1r7h5emwy4o49zscm1pa45i855x8ujb6 X-HE-Tag: 1693485377-778150 X-HE-Meta: U2FsdGVkX1/wIbWaR6g6o2qq6Hj1MbH/pcB4QP7yhzZL0KSedjxiF5h4kpzDbjYbPaioJQOWpWPJ9eIWWq8pkQbj1rGAmKrFc48PCV7Xizm+dejkBPzHUGaNz4KXTiOWY57KfqJA9XsxzK8buSqMYCEMbALCGefwa5Bx7Xj0h4sMI6zlvcsqve+zuN/Ch012p7NntVh6eJuNIPuNsk4PKr9i7eTmPtfA/aYHc1TrlIgG7TliM8naGwd0fbxLlRYobxQ2685rq8kr0aCXY4N8psyyAttYFzOlJwdD5CancokmsR66XKuDhSiaCfpjJvWDj5GdFCDATNeLytaXhuJPD9ZRS2ELIsqnvxyegO2UGzQCz62NiZrAV9mN5W1LAZCNdBI7A40lAnTvrU4k3c1F480+ntnXFtypVbUIedeN2fG8r4Ym//oQ2f0GfRuzK3jlCBNELEUTfawACOfCqg5NoGbfy5QdjO3rYX/oNFRw1AlNXAb4zKDsXuVzqwp6Mx80E/bNzBjVBuHJ095F2UawZG87ohskV6o8uYShUJS0vgr0swTgjiH+aYvHbmBNXXTVdJJ9i+ezlf+d8G96A4W74zaK2raMjO3NMVVorq0aFyyMpaHs5dkEepoDeSL0yFbsrVpXKq94ics4kk/1MU8HDOQSXgTIOweGXyN2ovL6VcvTMj9vLyE3pB8EpFh6Rlv0zxR25KYl9v+Awc55eqezu1eCdN85Aur+hgfmM27o/wRSEiVso8XmX1IoHKCj0jyQ88EYeIEIXYoqV016w+GR1srwUgXEXXMPxAYQOuZVJSM1yOki8Hs7oetjc/a1C9U4bj1dPFvLeEsicMneRBmMLcXy9ty+32vsJUzVsnlVyUud6WTWzjg8zUEgOM1ISZa1wJm6lgEwOBF01vbDqz90lfmeLeoV3bEJBjDIV5Aw37vf29PSKoKzoe4Ez5JgSW8CuXZ3G/ukh3RHPbyARYm 8GAZDfjf zcRzyUM8iN+c26xULAYvmrcmdbr/G8RqkZpcUXFBQMA+GAe2zAyyXBQ1M/OBGP/cuHHwoxEndmxO9OuhtgLiC39zfSb25DQCpJ7AkcykiOWwRrmFXayoL/4OrO9ala0i0qicFO8B9+o1/2BLXeh8WbfJAToM529hcxHrWgZjXFDJFWRLfmDOj7fYngmCdE5IZHZlvQsh9EICBu88IXci+kfpFfKiKVnlfxtTla58y38pD3Fz1CII6dknri0+7kTUUdNLEbWWsdjnpsbhAftES36s9IkRVS8WTB/O5WvlJkYQcoqwOvcOd4Qb/V2uG833lKLlc1C26hA7remGunM+ZBaymzkoIUWdSe8J1mnWxGXZTbMt3I0gTGWOyShDPw5N9KH6J 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 Thu, Aug 31, 2023 at 02:19:20AM -0700, Hugh Dickins wrote: > On Wed, 30 Aug 2023, Paul Moore wrote: > > > Hello all, > > > > While looking at some recent changes in mm/shmem.c I noticed that the > > ordering between simple_acl_create() and > > security_inode_init_security() is different between shmem_mknod() and > > shmem_tmpfile(). In shmem_mknod() the ACL call comes before the LSM > > hook, and in shmem_tmpfile() the LSM call comes before the ACL call. > > > > Perhaps this is correct, but it seemed a little odd to me so I wanted > > to check with all of you to make sure there is a good reason for the > > difference between the two functions. Looking back to when > > shmem_tmpfile() was created ~2013 I don't see any explicit mention as > > to why the ordering is different so I'm looking for a bit of a sanity > > check to see if I'm missing something obvious. > > > > My initial thinking this morning is that the > > security_inode_init_security() call should come before > > simple_acl_create() in both cases, but I'm open to different opinions > > on this. > > Good eye. The crucial commit here appears to be Mimi's 3.11 commit > 37ec43cdc4c7 "evm: calculate HMAC after initializing posix acl on tmpfs" > which intentionally moved shmem_mknod()'s generic_acl_init() up before > the security_inode_init_security(), around the same time as Al was > copying shmem_mknod() to introduce shmem_tmpfile(). > > I'd have agreed with you, Paul, until reading Mimi's commit: > now it looks more like shmem_tmpfile() is the one to be changed, > except (I'm out of my depth) maybe it's irrelevant on tmpfiles. POSIX ACLs generally need to be set first as they are may change inode properties that security_inode_init_security() may rely on to be stable. That specifically incudes inode->i_mode: * If the filesystem doesn't support POSIX ACLs then the umask is stripped in the VFS before it ever gets to the filesystems. For such cases the order of *_init_security() and setting POSIX ACLs doesn't matter. * If the filesystem does support POSIX ACLs and the directory of the resulting file does have default POSIX ACLs with mode settings then the inode->i_mode will be updated. * If the filesystem does support POSIX ACLs but the directory doesn't have default POSIX ACLs the umask will be stripped. (roughly from memory) If tmpfs is compiled with POSIX ACL support the mode might change and if anything in *_init_security() relies on inode->i_mode being stable it needs to be called after they have been set. EVM hashes do use the mode and the hash gets updated when POSIX ACLs are changed - which caused me immense pain when I redid these codepaths last year. IMHO, the easiest fix really is to lump all this together for all creation paths. This is what most filesystems do. For examples, see xfs_generic_create() -> posix_acl_create(&mode) -> xfs_create{_tmpfile}(mode) -> xfs_inode_init_security() or __ext4_new_inode() -> ext4_init_acl() -> ext4_init_security()