From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Theodore Ts'o" Subject: Re: [PATCH v2 5/5] fscrypt: fix race where ->lookup() marks plaintext dentry as ciphertext Date: Wed, 17 Apr 2019 10:24:35 -0400 Message-ID: <20190417142435.GE4686@mit.edu> References: <20190320183913.12686-1-ebiggers@kernel.org> <20190320183913.12686-6-ebiggers@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1hGlUk-0008EJ-AP for linux-f2fs-devel@lists.sourceforge.net; Wed, 17 Apr 2019 14:25:02 +0000 Received: from outgoing-auth-1.mit.edu ([18.9.28.11] helo=outgoing.mit.edu) by sfi-mx-3.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) id 1hGlUi-00Chh9-IM for linux-f2fs-devel@lists.sourceforge.net; Wed, 17 Apr 2019 14:25:02 +0000 Content-Disposition: inline In-Reply-To: <20190320183913.12686-6-ebiggers@kernel.org> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net To: Eric Biggers Cc: linux-unionfs@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-fscrypt@vger.kernel.org, linux-mtd@lists.infradead.org, Sarthak Kukreti , linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org On Wed, Mar 20, 2019 at 11:39:13AM -0700, Eric Biggers wrote: > From: Eric Biggers > > ->lookup() in an encrypted directory begins as follows: > > 1. fscrypt_prepare_lookup(): > a. Try to load the directory's encryption key. > b. If the key is unavailable, mark the dentry as a ciphertext name > via d_flags. > 2. fscrypt_setup_filename(): > a. Try to load the directory's encryption key. > b. If the key is available, encrypt the name (treated as a plaintext > name) to get the on-disk name. Otherwise decode the name > (treated as a ciphertext name) to get the on-disk name. > > But if the key is concurrently added, it may be found at (2a) but not at > (1a). In this case, the dentry will be wrongly marked as a ciphertext > name even though it was actually treated as plaintext. > > This will cause the dentry to be wrongly invalidated on the next lookup, > potentially causing problems. For example, if the racy ->lookup() was > part of sys_mount(), then the new mount will be detached when anything > tries to access it. This is despite the mountpoint having a plaintext > path, which should remain valid now that the key was added. > > Of course, this is only possible if there's a userspace race. Still, > the additional kernel-side race is confusing and unexpected. > > Close the kernel-side race by changing fscrypt_prepare_lookup() to also > set the on-disk filename (step 2b), consistent with the d_flags update. > > Fixes: 28b4c263961c ("ext4 crypto: revalidate dentry after adding or removing the key") > Signed-off-by: Eric Biggers Looks good, applied. - Ted