linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Gabriel Krisman Bertazi <krisman@suse.de>
To: ebiggers@kernel.org, viro@zeniv.linux.org.uk, jaegeuk@kernel.org
Cc: tytso@mit.edu, amir73il@gmail.com, linux-ext4@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	linux-fsdevel@vger.kernel.org, brauner@kernel.org,
	Gabriel Krisman Bertazi <krisman@suse.de>
Subject: [PATCH v7 04/10] fscrypt: Drop d_revalidate once the key is added
Date: Wed, 21 Feb 2024 12:14:06 -0500	[thread overview]
Message-ID: <20240221171412.10710-5-krisman@suse.de> (raw)
In-Reply-To: <20240221171412.10710-1-krisman@suse.de>

When a key is added, existing directory dentries in the
DCACHE_NOKEY_NAME form are moved by the VFS to the plaintext version.
But, since they have the DCACHE_OP_REVALIDATE flag set, revalidation
will be done at each lookup only to return immediately, since plaintext
dentries can't go stale until eviction.  This patch optimizes this case,
by dropping the flag once the nokey_name dentry becomes plain-text.
Note that non-directory dentries are not moved this way, so they won't
be affected.

Of course, this can only be done if fscrypt is the only thing requiring
revalidation for a dentry.  For this reason, we only disable
d_revalidate if the .d_revalidate hook is fscrypt_d_revalidate itself.

It is safe to do it here because when moving the dentry to the
plain-text version, we are holding the d_lock.  We might race with a
concurrent RCU lookup but this is harmless because, at worst, we will
get an extra d_revalidate on the keyed dentry, which will still find the
dentry to be valid.

Finally, now that we do more than just clear the DCACHE_NOKEY_NAME in
fscrypt_handle_d_move, skip it entirely for plaintext dentries, to avoid
extra costs.

Signed-off-by: Gabriel Krisman Bertazi <krisman@suse.de>

---
Changes since v6:
  - mention this is only for directory dentries in the commit msg(eric)
Changes since v5:
  - Merge with another patch(eric)
  - revert conditional check (eric)
  - drop comment (eric)
Changes since v3:
  - Fix null-ptr-deref for filesystems that don't support fscrypt (ktr)
Changes since v2:
  - Do it when moving instead of when revalidating the dentry. (me)
Changes since v1:
  - Improve commit message (Eric)
  - Drop & in function comparison (Eric)
---
 include/linux/fscrypt.h | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 78af02b35bd9..772f822dc6b8 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -192,6 +192,8 @@ struct fscrypt_operations {
 					     unsigned int *num_devs);
 };
 
+int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags);
+
 static inline struct fscrypt_inode_info *
 fscrypt_get_inode_info(const struct inode *inode)
 {
@@ -221,15 +223,29 @@ static inline bool fscrypt_needs_contents_encryption(const struct inode *inode)
 }
 
 /*
- * When d_splice_alias() moves a directory's no-key alias to its plaintext alias
- * as a result of the encryption key being added, DCACHE_NOKEY_NAME must be
- * cleared.  Note that we don't have to support arbitrary moves of this flag
- * because fscrypt doesn't allow no-key names to be the source or target of a
- * rename().
+ * When d_splice_alias() moves a directory's no-key alias to its
+ * plaintext alias as a result of the encryption key being added,
+ * DCACHE_NOKEY_NAME must be cleared and there might be an opportunity
+ * to disable d_revalidate.  Note that we don't have to support the
+ * inverse operation because fscrypt doesn't allow no-key names to be
+ * the source or target of a rename().
  */
 static inline void fscrypt_handle_d_move(struct dentry *dentry)
 {
-	dentry->d_flags &= ~DCACHE_NOKEY_NAME;
+	/*
+	 * VFS calls fscrypt_handle_d_move even for non-fscrypt
+	 * filesystems.
+	 */
+	if (dentry->d_flags & DCACHE_NOKEY_NAME) {
+		dentry->d_flags &= ~DCACHE_NOKEY_NAME;
+
+		/*
+		 * Other filesystem features might be handling dentry
+		 * revalidation, in which case it cannot be disabled.
+		 */
+		if (dentry->d_op->d_revalidate == fscrypt_d_revalidate)
+			dentry->d_flags &= ~DCACHE_OP_REVALIDATE;
+	}
 }
 
 /**
@@ -397,7 +413,6 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode,
 bool fscrypt_match_name(const struct fscrypt_name *fname,
 			const u8 *de_name, u32 de_name_len);
 u64 fscrypt_fname_siphash(const struct inode *dir, const struct qstr *name);
-int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags);
 
 /* bio.c */
 bool fscrypt_decrypt_bio(struct bio *bio);
-- 
2.43.0


  parent reply	other threads:[~2024-02-21 17:14 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-21 17:14 [PATCH v7 00/10] Set casefold/fscrypt dentry operations through sb->s_d_op Gabriel Krisman Bertazi
2024-02-21 17:14 ` [PATCH v7 01/10] ovl: Always reject mounting over case-insensitive directories Gabriel Krisman Bertazi
2024-02-21 17:14 ` [PATCH v7 02/10] fscrypt: Factor out a helper to configure the lookup dentry Gabriel Krisman Bertazi
2024-02-21 17:14 ` [PATCH v7 03/10] fscrypt: Drop d_revalidate for valid dentries during lookup Gabriel Krisman Bertazi
2024-02-21 17:14 ` Gabriel Krisman Bertazi [this message]
2024-02-21 17:14 ` [PATCH v7 05/10] libfs: Merge encrypted_ci_dentry_ops and ci_dentry_ops Gabriel Krisman Bertazi
2024-02-21 17:14 ` [PATCH v7 06/10] libfs: Add helper to choose dentry operations at mount-time Gabriel Krisman Bertazi
2024-02-21 17:14 ` [PATCH v7 07/10] ext4: Configure dentry operations at dentry-creation time Gabriel Krisman Bertazi
2024-02-21 17:14 ` [PATCH v7 08/10] f2fs: " Gabriel Krisman Bertazi
2024-02-21 17:14 ` [PATCH v7 09/10] ubifs: " Gabriel Krisman Bertazi
2024-02-21 17:14 ` [PATCH v7 10/10] libfs: Drop generic_set_encrypted_ci_d_ops Gabriel Krisman Bertazi
2024-02-27  6:36 ` [PATCH v7 00/10] Set casefold/fscrypt dentry operations through sb->s_d_op Eric Biggers
2024-02-27 21:28   ` Gabriel Krisman Bertazi
2024-03-18 20:45 ` [f2fs-dev] " patchwork-bot+f2fs

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240221171412.10710-5-krisman@suse.de \
    --to=krisman@suse.de \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=ebiggers@kernel.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).