linux-ext4 - Re: [PATCH v6 03/10] fscrypt: Drop d_revalidate for valid dentries during lookup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240214235904.GH1638@sol.localdomain>
Date: Wed, 14 Feb 2024 15:59:04 -0800
From: Eric Biggers <ebiggers@...nel.org>
To: Gabriel Krisman Bertazi <krisman@...e.de>
Cc: viro@...iv.linux.org.uk, jaegeuk@...nel.org, tytso@....edu,
	amir73il@...il.com, linux-ext4@...r.kernel.org,
	linux-f2fs-devel@...ts.sourceforge.net,
	linux-fsdevel@...r.kernel.org, brauner@...nel.org
Subject: Re: [PATCH v6 03/10] fscrypt: Drop d_revalidate for valid dentries
 during lookup

On Mon, Feb 12, 2024 at 09:13:14PM -0500, Gabriel Krisman Bertazi wrote:
> Finally, we need to clean the dentry->flags even for unencrypted
> dentries, so the ->d_lock might be acquired even for them.  In order to

might => must?

> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index 47567a6a4f9d..d1f17b90c30f 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -951,10 +951,29 @@ static inline int fscrypt_prepare_rename(struct inode *old_dir,
>  static inline void fscrypt_prepare_dentry(struct dentry *dentry,
>  					  bool is_nokey_name)
>  {
> +	/*
> +	 * This code tries to only take ->d_lock when necessary to write
> +	 * to ->d_flags.  We shouldn't be peeking on d_flags for
> +	 * DCACHE_OP_REVALIDATE unlocked, but in the unlikely case
> +	 * there is a race, the worst it can happen is that we fail to
> +	 * unset DCACHE_OP_REVALIDATE and pay the cost of an extra
> +	 * d_revalidate.
> +	 */
>  	if (is_nokey_name) {
>  		spin_lock(&dentry->d_lock);
>  		dentry->d_flags |= DCACHE_NOKEY_NAME;
>  		spin_unlock(&dentry->d_lock);
> +	} else if (dentry->d_flags & DCACHE_OP_REVALIDATE &&
> +		   dentry->d_op->d_revalidate == fscrypt_d_revalidate) {
> +		/*
> +		 * Unencrypted dentries and encrypted dentries where the
> +		 * key is available are always valid from fscrypt
> +		 * perspective. Avoid the cost of calling
> +		 * fscrypt_d_revalidate unnecessarily.
> +		 */
> +		spin_lock(&dentry->d_lock);
> +		dentry->d_flags &= ~DCACHE_OP_REVALIDATE;
> +		spin_unlock(&dentry->d_lock);
>  	}
>  }

Does this all get optimized out when !CONFIG_FS_ENCRYPTION?

As-is, I don't think the d_revalidate part will be optimized out.

You may need to create a !CONFIG_FS_ENCRYPTION stub explicitly.

- Eric