linux-ext4 - Re: [PATCH v6 5/9] libfs: Validate negative dentries in case-insensitive directories

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: <20231122202044.GF38156@ZenIV>
Date: Wed, 22 Nov 2023 20:20:44 +0000
From: Al Viro <viro@...iv.linux.org.uk>
To: Gabriel Krisman Bertazi <krisman@...e.de>
Cc: brauner@...nel.org, tytso@....edu, ebiggers@...nel.org,
	jaegeuk@...nel.org, linux-fsdevel@...r.kernel.org,
	linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
	Gabriel Krisman Bertazi <krisman@...labora.com>
Subject: Re: [PATCH v6 5/9] libfs: Validate negative dentries in
 case-insensitive directories

On Wed, Aug 16, 2023 at 01:07:59AM -0400, Gabriel Krisman Bertazi wrote:

> +static int generic_ci_d_revalidate(struct dentry *dentry,
> +				   const struct qstr *name,
> +				   unsigned int flags)
> +{
> +	const struct dentry *parent;
> +	const struct inode *dir;
> +
> +	if (!d_is_negative(dentry))
> +		return 1;
> +
> +	parent = READ_ONCE(dentry->d_parent);
> +	dir = READ_ONCE(parent->d_inode);
> +
> +	if (!dir || !IS_CASEFOLDED(dir))
> +		return 1;
> +
> +	/*
> +	 * Negative dentries created prior to turning the directory
> +	 * case-insensitive cannot be trusted, since they don't ensure
> +	 * any possible case version of the filename doesn't exist.
> +	 */
> +	if (!d_is_casefolded_name(dentry))
> +		return 0;
> +
> +	/*
> +	 * If the lookup is for creation, then a negative dentry can only be
> +	 * reused if it's a case-sensitive match, not just a case-insensitive
> +	 * one.  This is needed to make the new file be created with the name
> +	 * the user specified, preserving case.
> +	 *
> +	 * LOOKUP_CREATE or LOOKUP_RENAME_TARGET cover most creations.  In these
> +	 * cases, ->d_name is stable and can be compared to 'name' without
> +	 * taking ->d_lock because the caller must hold dir->i_rwsem.  (This
> +	 * is because the directory lock blocks the dentry from being
> +	 * concurrently instantiated, and negative dentries are never moved.)
> +	 *
> +	 * All other creations actually use flags==0.  These come from the edge
> +	 * case of filesystems calling functions like lookup_one() that do a
> +	 * lookup without setting the lookup flags at all.  Such lookups might
> +	 * or might not be for creation, and if not don't guarantee stable
> +	 * ->d_name.  Therefore, invalidate all negative dentries when flags==0.
> +	 */
> +	if (flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET)) {
> +		if (dentry->d_name.len != name->len ||
> +		    memcmp(dentry->d_name.name, name->name, name->len))
> +			return 0;

Frankly, I would rather moved that to fs/dcache.c and used dentry_cmp() instead
of memcmp() here.  Avoids the discussion of ->d_name stability for this one.