From patchwork Tue Nov 3 20:35:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg KH X-Patchwork-Id: 11879041 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B50A7139F for ; Tue, 3 Nov 2020 21:25:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9865B22450 for ; Tue, 3 Nov 2020 21:25:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604438751; bh=mnO+NWe+VUlFb0OX4jGXnxmON27sUidwJ6J5ZEA+yQM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=2X7qoqLmIvUWg6ubEnzaR180re6c0dFuN/WqrDXhcyLqJ+3tF96+WDATXOt6HeMgq tuyusFfcCW9m0Y4R2/n+TJRzC4g5rsodVI2FKjSSHCQAlGN84rsSHgstqfpiKn30K0 GLIZwLAisiSIqoOYs2p78dzrgvC3yo0FNBicDbMI= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387594AbgKCVCR (ORCPT ); Tue, 3 Nov 2020 16:02:17 -0500 Received: from mail.kernel.org ([198.145.29.99]:39146 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387562AbgKCVCM (ORCPT ); Tue, 3 Nov 2020 16:02:12 -0500 Received: from localhost (83-86-74-64.cable.dynamic.v4.ziggo.nl [83.86.74.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 6545B205ED; Tue, 3 Nov 2020 21:02:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604437330; bh=mnO+NWe+VUlFb0OX4jGXnxmON27sUidwJ6J5ZEA+yQM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=EOZekDwU3sx8mST22OnDQSOu0IHuWspuB3dBkRWynAlOjdUtIsi1ASrJ/MLZCf8EB bqJqo/030Tw9vaM+KuA4dlM3hIbm9QbhGWb0nqZxHCQkVAL/vR07w5x9xc2MhOFE2C ZCLW5Frgvq/H90U77pY/G+3BWf4xLHnsu20DZzIQ= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Greg Kroah-Hartman , "linux-fscrypt@vger.kernel.org, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-mtd@lists.infradead.org, Theodore Tso" , Eric Biggers , Theodore Ts'o Subject: [PATCH 4.19 030/191] fscrypt: clean up and improve dentry revalidation Date: Tue, 3 Nov 2020 21:35:22 +0100 Message-Id: <20201103203236.626034349@linuxfoundation.org> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20201103203232.656475008@linuxfoundation.org> References: <20201103203232.656475008@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-fscrypt@vger.kernel.org From: Eric Biggers commit 6cc248684d3d23bbd073ae2fa73d3416c0558909 upstream. Make various improvements to fscrypt dentry revalidation: - Don't try to handle the case where the per-directory key is removed, as this can't happen without the inode (and dentries) being evicted. - Flag ciphertext dentries rather than plaintext dentries, since it's ciphertext dentries that need the special handling. - Avoid doing unnecessary work for non-ciphertext dentries. - When revalidating ciphertext dentries, try to set up the directory's i_crypt_info to make sure the key is really still absent, rather than invalidating all negative dentries as the previous code did. An old comment suggested we can't do this for locking reasons, but AFAICT this comment was outdated and it actually works fine. Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o Signed-off-by: Greg Kroah-Hartman --- fs/crypto/crypto.c | 58 ++++++++++++++++++++++++------------------------ fs/crypto/hooks.c | 4 +-- include/linux/dcache.h | 2 - include/linux/fscrypt.h | 6 +--- 4 files changed, 35 insertions(+), 35 deletions(-) --- a/fs/crypto/crypto.c +++ b/fs/crypto/crypto.c @@ -314,45 +314,47 @@ int fscrypt_decrypt_page(const struct in EXPORT_SYMBOL(fscrypt_decrypt_page); /* - * Validate dentries for encrypted directories to make sure we aren't - * potentially caching stale data after a key has been added or - * removed. + * Validate dentries in encrypted directories to make sure we aren't potentially + * caching stale dentries after a key has been added. */ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags) { struct dentry *dir; - int dir_has_key, cached_with_key; + int err; + int valid; + + /* + * Plaintext names are always valid, since fscrypt doesn't support + * reverting to ciphertext names without evicting the directory's inode + * -- which implies eviction of the dentries in the directory. + */ + if (!(dentry->d_flags & DCACHE_ENCRYPTED_NAME)) + return 1; + + /* + * Ciphertext name; valid if the directory's key is still unavailable. + * + * Although fscrypt forbids rename() on ciphertext names, we still must + * use dget_parent() here rather than use ->d_parent directly. That's + * because a corrupted fs image may contain directory hard links, which + * the VFS handles by moving the directory's dentry tree in the dcache + * each time ->lookup() finds the directory and it already has a dentry + * elsewhere. Thus ->d_parent can be changing, and we must safely grab + * a reference to some ->d_parent to prevent it from being freed. + */ if (flags & LOOKUP_RCU) return -ECHILD; dir = dget_parent(dentry); - if (!IS_ENCRYPTED(d_inode(dir))) { - dput(dir); - return 0; - } - - spin_lock(&dentry->d_lock); - cached_with_key = dentry->d_flags & DCACHE_ENCRYPTED_WITH_KEY; - spin_unlock(&dentry->d_lock); - dir_has_key = (d_inode(dir)->i_crypt_info != NULL); + err = fscrypt_get_encryption_info(d_inode(dir)); + valid = !fscrypt_has_encryption_key(d_inode(dir)); dput(dir); - /* - * If the dentry was cached without the key, and it is a - * negative dentry, it might be a valid name. We can't check - * if the key has since been made available due to locking - * reasons, so we fail the validation so ext4_lookup() can do - * this check. - * - * We also fail the validation if the dentry was created with - * the key present, but we no longer have the key, or vice versa. - */ - if ((!cached_with_key && d_is_negative(dentry)) || - (!cached_with_key && dir_has_key) || - (cached_with_key && !dir_has_key)) - return 0; - return 1; + if (err < 0) + return err; + + return valid; } const struct dentry_operations fscrypt_d_ops = { --- a/fs/crypto/hooks.c +++ b/fs/crypto/hooks.c @@ -101,9 +101,9 @@ int __fscrypt_prepare_lookup(struct inod if (err) return err; - if (fscrypt_has_encryption_key(dir)) { + if (!fscrypt_has_encryption_key(dir)) { spin_lock(&dentry->d_lock); - dentry->d_flags |= DCACHE_ENCRYPTED_WITH_KEY; + dentry->d_flags |= DCACHE_ENCRYPTED_NAME; spin_unlock(&dentry->d_lock); } --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -210,7 +210,7 @@ struct dentry_operations { #define DCACHE_MAY_FREE 0x00800000 #define DCACHE_FALLTHRU 0x01000000 /* Fall through to lower layer */ -#define DCACHE_ENCRYPTED_WITH_KEY 0x02000000 /* dir is encrypted with a valid key */ +#define DCACHE_ENCRYPTED_NAME 0x02000000 /* Encrypted name (dir key was unavailable) */ #define DCACHE_OP_REAL 0x04000000 #define DCACHE_PAR_LOOKUP 0x10000000 /* being looked up (with parent locked shared) */ --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -145,10 +145,8 @@ static inline int fscrypt_prepare_rename * filenames are presented in encrypted form. Therefore, we'll try to set up * the directory's encryption key, but even without it the lookup can continue. * - * To allow invalidating stale dentries if the directory's encryption key is - * added later, we also install a custom ->d_revalidate() method and use the - * DCACHE_ENCRYPTED_WITH_KEY flag to indicate whether a given dentry is a - * plaintext name (flag set) or a ciphertext name (flag cleared). + * This also installs a custom ->d_revalidate() method which will invalidate the + * dentry if it was created without the key and the key is later added. * * Return: 0 on success, -errno if a problem occurred while setting up the * encryption key From patchwork Tue Nov 3 20:35:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg KH X-Patchwork-Id: 11879043 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 4C5C7174A for ; Tue, 3 Nov 2020 21:25:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2D0572242B for ; Tue, 3 Nov 2020 21:25:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604438752; bh=VKbr2do/Xb70jJsR2z/OGL/3FZyisH4oRDFQUWCYkn8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=AEkjq9P/Q7WkB6bwkZU9P2ZRC9ui/8k+SOm8b+np0dmpJiI2UcB5GOU3d8JNwlwjB GkeFbnf9bje++7OPzYLInt75mBTuJPfJPhcH8IECNUZjvOYBI1yD7T+xKsU2YBrjW6 q1tvyBg3PK2nHCd4Op6Kz0P6bZ9IacaWakNwPzaY= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387599AbgKCVCQ (ORCPT ); Tue, 3 Nov 2020 16:02:16 -0500 Received: from mail.kernel.org ([198.145.29.99]:39218 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387577AbgKCVCO (ORCPT ); Tue, 3 Nov 2020 16:02:14 -0500 Received: from localhost (83-86-74-64.cable.dynamic.v4.ziggo.nl [83.86.74.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id C7B0C206B5; Tue, 3 Nov 2020 21:02:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604437333; bh=VKbr2do/Xb70jJsR2z/OGL/3FZyisH4oRDFQUWCYkn8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Ir2GkHdFgEIx5I27Myjaaa689KDiGibqx+lzwKifYxdPwoCMFAs3hsaf9EsPfYRbb 0oYqQKL9CVxmoCEjpNZiS7rv9tnqMiVH17rX/efwZ8M833UALmoJL0nHAGF+r0zot/ oGLVQI+Ncjze0I653aCAB/qJoj6AscDC0xY+cQ9Q= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Greg Kroah-Hartman , "linux-fscrypt@vger.kernel.org, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-mtd@lists.infradead.org, Theodore Tso" , Eric Biggers , Theodore Ts'o Subject: [PATCH 4.19 031/191] fscrypt: fix race allowing rename() and link() of ciphertext dentries Date: Tue, 3 Nov 2020 21:35:23 +0100 Message-Id: <20201103203236.750207039@linuxfoundation.org> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20201103203232.656475008@linuxfoundation.org> References: <20201103203232.656475008@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-fscrypt@vger.kernel.org From: Eric Biggers commit 968dd6d0c6d6b6a989c6ddb9e2584a031b83e7b5 upstream. Close some race conditions where fscrypt allowed rename() and link() on ciphertext dentries that had been looked up just prior to the key being concurrently added. It's better to return -ENOKEY in this case. This avoids doing the nonsensical thing of encrypting the names a second time when searching for the actual on-disk dir entries. It also guarantees that DCACHE_ENCRYPTED_NAME dentries are never rename()d, so the dcache won't have support all possible combinations of moving DCACHE_ENCRYPTED_NAME around during __d_move(). Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o Signed-off-by: Greg Kroah-Hartman --- fs/crypto/hooks.c | 12 +++++++++++- include/linux/fscrypt.h | 2 +- include/linux/fscrypt_notsupp.h | 4 ++-- include/linux/fscrypt_supp.h | 3 ++- 4 files changed, 16 insertions(+), 5 deletions(-) --- a/fs/crypto/hooks.c +++ b/fs/crypto/hooks.c @@ -49,7 +49,8 @@ int fscrypt_file_open(struct inode *inod } EXPORT_SYMBOL_GPL(fscrypt_file_open); -int __fscrypt_prepare_link(struct inode *inode, struct inode *dir) +int __fscrypt_prepare_link(struct inode *inode, struct inode *dir, + struct dentry *dentry) { int err; @@ -57,6 +58,10 @@ int __fscrypt_prepare_link(struct inode if (err) return err; + /* ... in case we looked up ciphertext name before key was added */ + if (dentry->d_flags & DCACHE_ENCRYPTED_NAME) + return -ENOKEY; + if (!fscrypt_has_permitted_context(dir, inode)) return -EXDEV; @@ -78,6 +83,11 @@ int __fscrypt_prepare_rename(struct inod if (err) return err; + /* ... in case we looked up ciphertext name(s) before key was added */ + if ((old_dentry->d_flags | new_dentry->d_flags) & + DCACHE_ENCRYPTED_NAME) + return -ENOKEY; + if (old_dir != new_dir) { if (IS_ENCRYPTED(new_dir) && !fscrypt_has_permitted_context(new_dir, --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -97,7 +97,7 @@ static inline int fscrypt_prepare_link(s struct dentry *dentry) { if (IS_ENCRYPTED(dir)) - return __fscrypt_prepare_link(d_inode(old_dentry), dir); + return __fscrypt_prepare_link(d_inode(old_dentry), dir, dentry); return 0; } --- a/include/linux/fscrypt_notsupp.h +++ b/include/linux/fscrypt_notsupp.h @@ -183,8 +183,8 @@ static inline int fscrypt_file_open(stru return 0; } -static inline int __fscrypt_prepare_link(struct inode *inode, - struct inode *dir) +static inline int __fscrypt_prepare_link(struct inode *inode, struct inode *dir, + struct dentry *dentry) { return -EOPNOTSUPP; } --- a/include/linux/fscrypt_supp.h +++ b/include/linux/fscrypt_supp.h @@ -184,7 +184,8 @@ extern int fscrypt_zeroout_range(const s /* hooks.c */ extern int fscrypt_file_open(struct inode *inode, struct file *filp); -extern int __fscrypt_prepare_link(struct inode *inode, struct inode *dir); +extern int __fscrypt_prepare_link(struct inode *inode, struct inode *dir, + struct dentry *dentry); extern int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry, struct inode *new_dir, From patchwork Tue Nov 3 20:35:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg KH X-Patchwork-Id: 11879039 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 158AE92C for ; Tue, 3 Nov 2020 21:25:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E7FCD22456 for ; Tue, 3 Nov 2020 21:25:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604438744; bh=NCgEeXgIGtG26G81EP7Rjb5PupTf60OODIdNFZgVuYM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=VhIAntPWkruMVhCtMLIkDbeLGt5YR9e9/iXU+mHZhMMA8IdKqquwY0+HE5bYjDLm6 m0Ngb6ZIQ3taBQ9rPE9pfFtoKoHIndtktyPAwpVsBTsCU6iGZD+DVZRzgjW9MnoGjF OqNQp8p6DX20XKpCVGZbnIoOLNe1ADBmCt8RmpPw= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731639AbgKCVZm (ORCPT ); Tue, 3 Nov 2020 16:25:42 -0500 Received: from mail.kernel.org ([198.145.29.99]:39380 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387614AbgKCVCU (ORCPT ); Tue, 3 Nov 2020 16:02:20 -0500 Received: from localhost (83-86-74-64.cable.dynamic.v4.ziggo.nl [83.86.74.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 9762220658; Tue, 3 Nov 2020 21:02:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604437340; bh=NCgEeXgIGtG26G81EP7Rjb5PupTf60OODIdNFZgVuYM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ke4pVpumifRFuJA06vdJ+TMgX7AfGmt1vdggeQZt7lVQP4OU2/dPXL+hBC0uhiZjk nS3fFIteQ2GaAknP9gk0GMLl/sbgc4/z7s53LkpyhZlrL+QTNq+LxufidPQfi+xlAQ yJxUCO+MlB6X0SbZC4qnUvlB7UMmjdNS+wilAhX4= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Greg Kroah-Hartman , "linux-fscrypt@vger.kernel.org, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-mtd@lists.infradead.org, Theodore Tso" , Eric Biggers , Theodore Ts'o Subject: [PATCH 4.19 033/191] fscrypt: only set dentry_operations on ciphertext dentries Date: Tue, 3 Nov 2020 21:35:25 +0100 Message-Id: <20201103203237.007655514@linuxfoundation.org> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20201103203232.656475008@linuxfoundation.org> References: <20201103203232.656475008@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-fscrypt@vger.kernel.org From: Eric Biggers commit d456a33f041af4b54f3ce495a86d00c246165032 upstream. Plaintext dentries are always valid, so only set fscrypt_d_ops on ciphertext dentries. Besides marginally improved performance, this allows overlayfs to use an fscrypt-encrypted upperdir, provided that all the following are true: (1) The fscrypt encryption key is placed in the keyring before mounting overlayfs, and remains while the overlayfs is mounted. (2) The overlayfs workdir uses the same encryption policy. (3) No dentries for the ciphertext names of subdirectories have been created in the upperdir or workdir yet. (Since otherwise d_splice_alias() will reuse the old dentry with ->d_op set.) One potential use case is using an ephemeral encryption key to encrypt all files created or changed by a container, so that they can be securely erased ("crypto-shredded") after the container stops. Signed-off-by: Eric Biggers Signed-off-by: Theodore Ts'o Signed-off-by: Greg Kroah-Hartman --- fs/crypto/hooks.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) --- a/fs/crypto/hooks.c +++ b/fs/crypto/hooks.c @@ -115,9 +115,8 @@ int __fscrypt_prepare_lookup(struct inod spin_lock(&dentry->d_lock); dentry->d_flags |= DCACHE_ENCRYPTED_NAME; spin_unlock(&dentry->d_lock); + d_set_d_op(dentry, &fscrypt_d_ops); } - - d_set_d_op(dentry, &fscrypt_d_ops); return 0; } EXPORT_SYMBOL_GPL(__fscrypt_prepare_lookup); From patchwork Tue Nov 3 20:35:26 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg KH X-Patchwork-Id: 11879035 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 277BE921 for ; Tue, 3 Nov 2020 21:02:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id F337722226 for ; Tue, 3 Nov 2020 21:02:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604437345; bh=ajXfxJDXISAfGEtCDQjBjEiNDvRH0r32O42QDCvKPVM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=I8O0qkJS+ynzzv5+DqHVCAVwil3E1fQVwTcpF9670ZCLjpkTf1SzgUtcupO/arThw Ks3oiTTI2SG8QrVfuxUZN0/T4J5Y6Xh/y2kSD5Mupnqdas3HHHtHJrZawzyG4DvIOD MQCgfxVBCSo4e6kE8i+40HVo1H2nPO4FPy771tGA= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387631AbgKCVCY (ORCPT ); Tue, 3 Nov 2020 16:02:24 -0500 Received: from mail.kernel.org ([198.145.29.99]:39404 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387624AbgKCVCX (ORCPT ); Tue, 3 Nov 2020 16:02:23 -0500 Received: from localhost (83-86-74-64.cable.dynamic.v4.ziggo.nl [83.86.74.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id D474C205ED; Tue, 3 Nov 2020 21:02:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604437342; bh=ajXfxJDXISAfGEtCDQjBjEiNDvRH0r32O42QDCvKPVM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=eQMR600HYuus6mw7ZORzoCCHNoZoNZHMbtPpVlyQTic4v5HTwFGDE3MDqoyt5wYQ5 Y+jy8RFGi5MgOJWbjWpheCQBtLd8lRAjH+AtPP52FoEot1rCc/TzLo3wbse1zZ2eLD jWEFa4Qb4fEjTqHKc376ENhDT8OQXvXeNNb0d1CY= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Greg Kroah-Hartman , "linux-fscrypt@vger.kernel.org, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-mtd@lists.infradead.org, Theodore Tso" , Eric Biggers , Theodore Ts'o Subject: [PATCH 4.19 034/191] fscrypt: fix race where ->lookup() marks plaintext dentry as ciphertext Date: Tue, 3 Nov 2020 21:35:26 +0100 Message-Id: <20201103203237.143516712@linuxfoundation.org> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20201103203232.656475008@linuxfoundation.org> References: <20201103203232.656475008@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-fscrypt@vger.kernel.org From: Eric Biggers commit b01531db6cec2aa330dbc91bfbfaaef4a0d387a4 upstream. ->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 Signed-off-by: Theodore Ts'o Signed-off-by: Greg Kroah-Hartman --- fs/crypto/fname.c | 1 fs/crypto/hooks.c | 11 +++-- fs/ext4/ext4.h | 62 ++++++++++++++++++++++++-------- fs/ext4/namei.c | 76 +++++++++++++++++++++++++++------------- fs/f2fs/namei.c | 17 +++++--- fs/ubifs/dir.c | 8 +--- include/linux/fscrypt.h | 22 +++++++---- include/linux/fscrypt_notsupp.h | 5 +- include/linux/fscrypt_supp.h | 3 + 9 files changed, 139 insertions(+), 66 deletions(-) --- a/fs/crypto/fname.c +++ b/fs/crypto/fname.c @@ -354,6 +354,7 @@ int fscrypt_setup_filename(struct inode } if (!lookup) return -ENOKEY; + fname->is_ciphertext_name = true; /* * We don't have the key and we are doing a lookup; decode the --- a/fs/crypto/hooks.c +++ b/fs/crypto/hooks.c @@ -104,20 +104,21 @@ int __fscrypt_prepare_rename(struct inod } EXPORT_SYMBOL_GPL(__fscrypt_prepare_rename); -int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry) +int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry, + struct fscrypt_name *fname) { - int err = fscrypt_get_encryption_info(dir); + int err = fscrypt_setup_filename(dir, &dentry->d_name, 1, fname); - if (err) + if (err && err != -ENOENT) return err; - if (!fscrypt_has_encryption_key(dir)) { + if (fname->is_ciphertext_name) { spin_lock(&dentry->d_lock); dentry->d_flags |= DCACHE_ENCRYPTED_NAME; spin_unlock(&dentry->d_lock); d_set_d_op(dentry, &fscrypt_d_ops); } - return 0; + return err; } EXPORT_SYMBOL_GPL(__fscrypt_prepare_lookup); --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -2326,23 +2326,47 @@ static inline bool ext4_encrypted_inode( } #ifdef CONFIG_EXT4_FS_ENCRYPTION +static inline void ext4_fname_from_fscrypt_name(struct ext4_filename *dst, + const struct fscrypt_name *src) +{ + memset(dst, 0, sizeof(*dst)); + + dst->usr_fname = src->usr_fname; + dst->disk_name = src->disk_name; + dst->hinfo.hash = src->hash; + dst->hinfo.minor_hash = src->minor_hash; + dst->crypto_buf = src->crypto_buf; +} + static inline int ext4_fname_setup_filename(struct inode *dir, - const struct qstr *iname, - int lookup, struct ext4_filename *fname) + const struct qstr *iname, + int lookup, + struct ext4_filename *fname) { struct fscrypt_name name; int err; - memset(fname, 0, sizeof(struct ext4_filename)); - err = fscrypt_setup_filename(dir, iname, lookup, &name); + if (err) + return err; - fname->usr_fname = name.usr_fname; - fname->disk_name = name.disk_name; - fname->hinfo.hash = name.hash; - fname->hinfo.minor_hash = name.minor_hash; - fname->crypto_buf = name.crypto_buf; - return err; + ext4_fname_from_fscrypt_name(fname, &name); + return 0; +} + +static inline int ext4_fname_prepare_lookup(struct inode *dir, + struct dentry *dentry, + struct ext4_filename *fname) +{ + struct fscrypt_name name; + int err; + + err = fscrypt_prepare_lookup(dir, dentry, &name); + if (err) + return err; + + ext4_fname_from_fscrypt_name(fname, &name); + return 0; } static inline void ext4_fname_free_filename(struct ext4_filename *fname) @@ -2356,19 +2380,27 @@ static inline void ext4_fname_free_filen fname->usr_fname = NULL; fname->disk_name.name = NULL; } -#else +#else /* !CONFIG_EXT4_FS_ENCRYPTION */ static inline int ext4_fname_setup_filename(struct inode *dir, - const struct qstr *iname, - int lookup, struct ext4_filename *fname) + const struct qstr *iname, + int lookup, + struct ext4_filename *fname) { fname->usr_fname = iname; fname->disk_name.name = (unsigned char *) iname->name; fname->disk_name.len = iname->len; return 0; } -static inline void ext4_fname_free_filename(struct ext4_filename *fname) { } -#endif +static inline int ext4_fname_prepare_lookup(struct inode *dir, + struct dentry *dentry, + struct ext4_filename *fname) +{ + return ext4_fname_setup_filename(dir, &dentry->d_name, 1, fname); +} + +static inline void ext4_fname_free_filename(struct ext4_filename *fname) { } +#endif /* !CONFIG_EXT4_FS_ENCRYPTION */ /* dir.c */ extern int __ext4_check_dir_entry(const char *, unsigned int, struct inode *, --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -1343,7 +1343,7 @@ static int is_dx_internal_node(struct in } /* - * ext4_find_entry() + * __ext4_find_entry() * * finds an entry in the specified directory with the wanted name. It * returns the cache buffer in which the entry was found, and the entry @@ -1353,39 +1353,32 @@ static int is_dx_internal_node(struct in * The returned buffer_head has ->b_count elevated. The caller is expected * to brelse() it when appropriate. */ -static struct buffer_head * ext4_find_entry (struct inode *dir, - const struct qstr *d_name, - struct ext4_dir_entry_2 **res_dir, - int *inlined) +static struct buffer_head *__ext4_find_entry(struct inode *dir, + struct ext4_filename *fname, + struct ext4_dir_entry_2 **res_dir, + int *inlined) { struct super_block *sb; struct buffer_head *bh_use[NAMEI_RA_SIZE]; struct buffer_head *bh, *ret = NULL; ext4_lblk_t start, block; - const u8 *name = d_name->name; + const u8 *name = fname->usr_fname->name; size_t ra_max = 0; /* Number of bh's in the readahead buffer, bh_use[] */ size_t ra_ptr = 0; /* Current index into readahead buffer */ ext4_lblk_t nblocks; int i, namelen, retval; - struct ext4_filename fname; *res_dir = NULL; sb = dir->i_sb; - namelen = d_name->len; + namelen = fname->usr_fname->len; if (namelen > EXT4_NAME_LEN) return NULL; - retval = ext4_fname_setup_filename(dir, d_name, 1, &fname); - if (retval == -ENOENT) - return NULL; - if (retval) - return ERR_PTR(retval); - if (ext4_has_inline_data(dir)) { int has_inline_data = 1; - ret = ext4_find_inline_entry(dir, &fname, res_dir, + ret = ext4_find_inline_entry(dir, fname, res_dir, &has_inline_data); if (has_inline_data) { if (inlined) @@ -1405,7 +1398,7 @@ static struct buffer_head * ext4_find_en goto restart; } if (is_dx(dir)) { - ret = ext4_dx_find_entry(dir, &fname, res_dir); + ret = ext4_dx_find_entry(dir, fname, res_dir); /* * On success, or if the error was file not found, * return. Otherwise, fall back to doing a search the @@ -1470,7 +1463,7 @@ restart: goto cleanup_and_exit; } set_buffer_verified(bh); - i = search_dirblock(bh, dir, &fname, + i = search_dirblock(bh, dir, fname, block << EXT4_BLOCK_SIZE_BITS(sb), res_dir); if (i == 1) { EXT4_I(dir)->i_dir_start_lookup = block; @@ -1501,10 +1494,50 @@ cleanup_and_exit: /* Clean up the read-ahead blocks */ for (; ra_ptr < ra_max; ra_ptr++) brelse(bh_use[ra_ptr]); - ext4_fname_free_filename(&fname); return ret; } +static struct buffer_head *ext4_find_entry(struct inode *dir, + const struct qstr *d_name, + struct ext4_dir_entry_2 **res_dir, + int *inlined) +{ + int err; + struct ext4_filename fname; + struct buffer_head *bh; + + err = ext4_fname_setup_filename(dir, d_name, 1, &fname); + if (err == -ENOENT) + return NULL; + if (err) + return ERR_PTR(err); + + bh = __ext4_find_entry(dir, &fname, res_dir, inlined); + + ext4_fname_free_filename(&fname); + return bh; +} + +static struct buffer_head *ext4_lookup_entry(struct inode *dir, + struct dentry *dentry, + struct ext4_dir_entry_2 **res_dir) +{ + int err; + struct ext4_filename fname; + struct buffer_head *bh; + + err = ext4_fname_prepare_lookup(dir, dentry, &fname); + if (err == -ENOENT) + return NULL; + if (err) + return ERR_PTR(err); + + bh = __ext4_find_entry(dir, &fname, res_dir, NULL); + + ext4_fname_free_filename(&fname); + return bh; +} + static struct buffer_head * ext4_dx_find_entry(struct inode *dir, struct ext4_filename *fname, struct ext4_dir_entry_2 **res_dir) @@ -1563,16 +1596,11 @@ static struct dentry *ext4_lookup(struct struct inode *inode; struct ext4_dir_entry_2 *de; struct buffer_head *bh; - int err; - - err = fscrypt_prepare_lookup(dir, dentry, flags); - if (err) - return ERR_PTR(err); if (dentry->d_name.len > EXT4_NAME_LEN) return ERR_PTR(-ENAMETOOLONG); - bh = ext4_find_entry(dir, &dentry->d_name, &de, NULL); + bh = ext4_lookup_entry(dir, dentry, &de); if (IS_ERR(bh)) return (struct dentry *) bh; inode = NULL; --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -432,19 +432,23 @@ static struct dentry *f2fs_lookup(struct nid_t ino = -1; int err = 0; unsigned int root_ino = F2FS_ROOT_INO(F2FS_I_SB(dir)); + struct fscrypt_name fname; trace_f2fs_lookup_start(dir, dentry, flags); - err = fscrypt_prepare_lookup(dir, dentry, flags); - if (err) - goto out; - if (dentry->d_name.len > F2FS_NAME_LEN) { err = -ENAMETOOLONG; goto out; } - de = f2fs_find_entry(dir, &dentry->d_name, &page); + err = fscrypt_prepare_lookup(dir, dentry, &fname); + if (err == -ENOENT) + goto out_splice; + if (err) + goto out; + de = __f2fs_find_entry(dir, &fname, &page); + fscrypt_free_filename(&fname); + if (!de) { if (IS_ERR(page)) { err = PTR_ERR(page); @@ -484,8 +488,7 @@ static struct dentry *f2fs_lookup(struct } out_splice: new = d_splice_alias(inode, dentry); - if (IS_ERR(new)) - err = PTR_ERR(new); + err = PTR_ERR_OR_ZERO(new); trace_f2fs_lookup_end(dir, dentry, ino, err); return new; out_iput: --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -220,11 +220,9 @@ static struct dentry *ubifs_lookup(struc dbg_gen("'%pd' in dir ino %lu", dentry, dir->i_ino); - err = fscrypt_prepare_lookup(dir, dentry, flags); - if (err) - return ERR_PTR(err); - - err = fscrypt_setup_filename(dir, &dentry->d_name, 1, &nm); + err = fscrypt_prepare_lookup(dir, dentry, &nm); + if (err == -ENOENT) + return d_splice_alias(NULL, dentry); if (err) return ERR_PTR(err); --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -32,6 +32,7 @@ struct fscrypt_name { u32 hash; u32 minor_hash; struct fscrypt_str crypto_buf; + bool is_ciphertext_name; }; #define FSTR_INIT(n, l) { .name = n, .len = l } @@ -138,25 +139,32 @@ static inline int fscrypt_prepare_rename * fscrypt_prepare_lookup - prepare to lookup a name in a possibly-encrypted directory * @dir: directory being searched * @dentry: filename being looked up - * @flags: lookup flags + * @fname: (output) the name to use to search the on-disk directory * - * Prepare for ->lookup() in a directory which may be encrypted. Lookups can be - * done with or without the directory's encryption key; without the key, + * Prepare for ->lookup() in a directory which may be encrypted by determining + * the name that will actually be used to search the directory on-disk. Lookups + * can be done with or without the directory's encryption key; without the key, * filenames are presented in encrypted form. Therefore, we'll try to set up * the directory's encryption key, but even without it the lookup can continue. * * This also installs a custom ->d_revalidate() method which will invalidate the * dentry if it was created without the key and the key is later added. * - * Return: 0 on success, -errno if a problem occurred while setting up the - * encryption key + * Return: 0 on success; -ENOENT if key is unavailable but the filename isn't a + * correctly formed encoded ciphertext name, so a negative dentry should be + * created; or another -errno code. */ static inline int fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry, - unsigned int flags) + struct fscrypt_name *fname) { if (IS_ENCRYPTED(dir)) - return __fscrypt_prepare_lookup(dir, dentry); + return __fscrypt_prepare_lookup(dir, dentry, fname); + + memset(fname, 0, sizeof(*fname)); + fname->usr_fname = &dentry->d_name; + fname->disk_name.name = (unsigned char *)dentry->d_name.name; + fname->disk_name.len = dentry->d_name.len; return 0; } --- a/include/linux/fscrypt_notsupp.h +++ b/include/linux/fscrypt_notsupp.h @@ -112,7 +112,7 @@ static inline int fscrypt_setup_filename if (IS_ENCRYPTED(dir)) return -EOPNOTSUPP; - memset(fname, 0, sizeof(struct fscrypt_name)); + memset(fname, 0, sizeof(*fname)); fname->usr_fname = iname; fname->disk_name.name = (unsigned char *)iname->name; fname->disk_name.len = iname->len; @@ -199,7 +199,8 @@ static inline int __fscrypt_prepare_rena } static inline int __fscrypt_prepare_lookup(struct inode *dir, - struct dentry *dentry) + struct dentry *dentry, + struct fscrypt_name *fname) { return -EOPNOTSUPP; } --- a/include/linux/fscrypt_supp.h +++ b/include/linux/fscrypt_supp.h @@ -191,7 +191,8 @@ extern int __fscrypt_prepare_rename(stru struct inode *new_dir, struct dentry *new_dentry, unsigned int flags); -extern int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry); +extern int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry, + struct fscrypt_name *fname); extern int __fscrypt_prepare_symlink(struct inode *dir, unsigned int len, unsigned int max_len, struct fscrypt_str *disk_link);