linux-kernel - Re: [PATCH 16/19] VFS: add lookup_and_lock

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20250207212135.GL1977892@ZenIV>
Date: Fri, 7 Feb 2025 21:21:35 +0000
From: Al Viro <viro@...iv.linux.org.uk>
To: NeilBrown <neilb@...e.de>
Cc: Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Jeff Layton <jlayton@...nel.org>,
	Dave Chinner <david@...morbit.com>, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 16/19] VFS: add lookup_and_lock_rename()

On Thu, Feb 06, 2025 at 04:42:53PM +1100, NeilBrown wrote:
> @@ -3451,8 +3451,14 @@ static struct dentry *lock_two_directories(struct dentry *p1, struct dentry *p2)
>  {
>  	struct dentry *p = p1, *q = p2, *r;
>  
> -	while ((r = p->d_parent) != p2 && r != p)
> +	/* Ensure d_update_wait() tests are safe - one barrier for all */
> +	smp_mb();
> +
> +	d_update_wait(p, I_MUTEX_NORMAL);
> +	while ((r = p->d_parent) != p2 && r != p) {
>  		p = r;
> +		d_update_wait(p, I_MUTEX_NORMAL);
> +	}
>  	if (r == p2) {
>  		// p is a child of p2 and an ancestor of p1 or p1 itself
>  		inode_lock_nested(p2->d_inode, I_MUTEX_PARENT);
> @@ -3461,8 +3467,11 @@ static struct dentry *lock_two_directories(struct dentry *p1, struct dentry *p2)
>  	}
>  	// p is the root of connected component that contains p1
>  	// p2 does not occur on the path from p to p1
> -	while ((r = q->d_parent) != p1 && r != p && r != q)
> +	d_update_wait(q, I_MUTEX_NORMAL);
> +	while ((r = q->d_parent) != p1 && r != p && r != q) {
>  		q = r;
> +		d_update_wait(q, I_MUTEX_NORMAL);
> +	}

That makes no sense whatsoever.  What are you waiting on here and _why_
are you waiting on those sucker?  Especially since there's nothing
to prevent the condition for which you wait from arising immediately
afterwards.