From patchwork Wed Mar 20 18:39:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Biggers X-Patchwork-Id: 1059479 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.infradead.org (client-ip=2607:7c80:54:e::133; helo=bombadil.infradead.org; envelope-from=linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="FKwuIcXW"; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="Em8aMCDX"; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:e::133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44Pdyl0FQfz9sPb for ; Thu, 21 Mar 2019 05:40:31 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Ht0DnKse35SR0gRsfWz/DCKzPUuKpOtKRRrkF6bU3XU=; b=FKwuIcXWFeeDv0 oEC1YGYDUy0j3JL5Sgxmk1Z10ivr9JVa0W91ahULp+ZwgtShrg1DgcZ+VxgrCDlFZ2aYtW4QSEhXd Zffns35GW1CRzioVAMFsYXj9RhpoddPOI0Cybn2QKwo7Q81A8VCmufAMmPkUhcuiijyprfzihBhKk E6szOHVFWH7eNNhOpx8Yhd2tNfQb7MGO+bCz83mX3troc1/f5LqE8ciOboKUg8EASpZ66ZrWH9QIL tYL18LIRv8CEPgIb4mY65fcAgisqVTnwIewTtLxgLdZWleLjIwy+US8EONnwBMIZYcq5gn2+Yg+9h x1zU0mjISZAjtM5mNjTg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h6g8W-0000E2-QS; Wed, 20 Mar 2019 18:40:24 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h6g8N-0008Uu-7O for linux-mtd@lists.infradead.org; Wed, 20 Mar 2019 18:40:16 +0000 Received: from ebiggers-linuxstation.mtv.corp.google.com (unknown [104.132.1.77]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 1173F2175B; Wed, 20 Mar 2019 18:40:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1553107214; bh=+vU7rSc55LpXe0KhhD1OnU9UyCaQvdmdHhdtI2vPxAI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Em8aMCDX+SQHXQkctoEz2RFiEK6yg6BTx1PR+EC02wPpxsPhDWfyJrKUliE4IrUty ViCRdEbiv3evrzQFa1ZOagJn1SuWQ4wyy1m+yOs+WjqMXe5kPRijkcT3wpkwNJgYI7 PLp86dUKUd1Q5xupnOM2CBJuPttxXtCZjQKPf6z8= From: Eric Biggers To: linux-fscrypt@vger.kernel.org Subject: [PATCH v2 1/5] fscrypt: clean up and improve dentry revalidation Date: Wed, 20 Mar 2019 11:39:09 -0700 Message-Id: <20190320183913.12686-2-ebiggers@kernel.org> X-Mailer: git-send-email 2.21.0.225.g810b269d1ac-goog In-Reply-To: <20190320183913.12686-1-ebiggers@kernel.org> References: <20190320183913.12686-1-ebiggers@kernel.org> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190320_114015_385187_2B424E97 X-CRM114-Status: GOOD ( 22.49 ) X-Spam-Score: -5.2 (-----) X-Spam-Report: SpamAssassin version 3.4.2 on bombadil.infradead.org summary: Content analysis details: (-5.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at https://www.dnswl.org/, high trust [198.145.29.99 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.0 DKIMWL_WL_HIGH DKIMwl.org - Whitelisted High sender X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Gao Xiang , linux-unionfs@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-mtd@lists.infradead.org, Sarthak Kukreti , linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org Sender: "linux-mtd" Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org From: Eric Biggers 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 --- 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(-) diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c index 4dc788e3bc96b..3fc84bf2b1e52 100644 --- a/fs/crypto/crypto.c +++ b/fs/crypto/crypto.c @@ -313,45 +313,47 @@ int fscrypt_decrypt_page(const struct inode *inode, struct page *page, 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 = (d_inode(dir)->i_crypt_info == NULL); 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 = { diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c index 56debb1fcf5eb..a9492f75bbe13 100644 --- a/fs/crypto/hooks.c +++ b/fs/crypto/hooks.c @@ -101,9 +101,9 @@ int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry) 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); } diff --git a/include/linux/dcache.h b/include/linux/dcache.h index 60996e64c5798..9b3b75d3bd211 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -212,7 +212,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) */ diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h index e5194fc3983e9..5a0c3fee1ea2b 100644 --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -545,10 +545,8 @@ static inline int fscrypt_prepare_rename(struct inode *old_dir, * 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