linux-kernel - Re: [PATCH v5 03/14] VFS: tidy up do

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: <cb5e89380ea2f156e1fed63835c649d125c78fe1.camel@kernel.org>
Date: Wed, 12 Nov 2025 09:47:23 -0500
From: Jeff Layton <jlayton@...nel.org>
To: NeilBrown <neil@...wn.name>, Alexander Viro <viro@...iv.linux.org.uk>, 
 Christian Brauner
	 <brauner@...nel.org>, Amir Goldstein <amir73il@...il.com>
Cc: Jan Kara <jack@...e.cz>, linux-fsdevel@...r.kernel.org, Chris Mason	
 <clm@...com>, David Sterba <dsterba@...e.com>, David Howells
 <dhowells@...hat.com>,  Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 "Rafael J. Wysocki" <rafael@...nel.org>, Danilo Krummrich	
 <dakr@...nel.org>, Tyler Hicks <code@...icks.com>, Miklos Szeredi	
 <miklos@...redi.hu>, Chuck Lever <chuck.lever@...cle.com>, Olga
 Kornievskaia	 <okorniev@...hat.com>, Dai Ngo <Dai.Ngo@...cle.com>, Namjae
 Jeon	 <linkinjeon@...nel.org>, Steve French <smfrench@...il.com>, Sergey
 Senozhatsky	 <senozhatsky@...omium.org>, Carlos Maiolino <cem@...nel.org>,
 John Johansen	 <john.johansen@...onical.com>, Paul Moore
 <paul@...l-moore.com>, James Morris	 <jmorris@...ei.org>, "Serge E. Hallyn"
 <serge@...lyn.com>, Stephen Smalley	 <stephen.smalley.work@...il.com>,
 Ondrej Mosnacek <omosnace@...hat.com>,  Mateusz Guzik <mjguzik@...il.com>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Stefan Berger	
 <stefanb@...ux.ibm.com>, "Darrick J. Wong" <djwong@...nel.org>, 
	linux-kernel@...r.kernel.org, netfs@...ts.linux.dev,
 ecryptfs@...r.kernel.org, 	linux-nfs@...r.kernel.org,
 linux-unionfs@...r.kernel.org, 	linux-cifs@...r.kernel.org,
 linux-xfs@...r.kernel.org, 	linux-security-module@...r.kernel.org,
 selinux@...r.kernel.org
Subject: Re: [PATCH v5 03/14] VFS: tidy up do_unlinkat()

On Thu, 2025-11-06 at 11:50 +1100, NeilBrown wrote:
> From: NeilBrown <neil@...wn.name>
> 
> The simplification of locking in the previous patch opens up some room
> for tidying up do_unlinkat()
> 
> - change all "exit" labels to describe what will happen at the label.
> - always goto an exit label on an error - unwrap the "if (!IS_ERR())" branch.
> - Move the "slashes" handing inline, but mark it as unlikely()
> - simplify use of the "inode" variable - we no longer need to test for NULL.
> 
> Reviewed-by: Amir Goldstein <amir73il@...il.com>
> Signed-off-by: NeilBrown <neil@...wn.name>
> ---
>  fs/namei.c | 55 ++++++++++++++++++++++++++----------------------------
>  1 file changed, 26 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 231e1ffd4b8d..93c5fce2d814 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -4755,65 +4755,62 @@ int do_unlinkat(int dfd, struct filename *name)
>  	struct path path;
>  	struct qstr last;
>  	int type;
> -	struct inode *inode = NULL;
> +	struct inode *inode;
>  	struct inode *delegated_inode = NULL;
>  	unsigned int lookup_flags = 0;
>  retry:
>  	error = filename_parentat(dfd, name, lookup_flags, &path, &last, &type);
>  	if (error)
> -		goto exit1;
> +		goto exit_putname;
>  
>  	error = -EISDIR;
>  	if (type != LAST_NORM)
> -		goto exit2;
> +		goto exit_path_put;
>  
>  	error = mnt_want_write(path.mnt);
>  	if (error)
> -		goto exit2;
> +		goto exit_path_put;
>  retry_deleg:
>  	dentry = start_dirop(path.dentry, &last, lookup_flags);
>  	error = PTR_ERR(dentry);
> -	if (!IS_ERR(dentry)) {
> +	if (IS_ERR(dentry))
> +		goto exit_drop_write;
>  
> -		/* Why not before? Because we want correct error value */
> -		if (last.name[last.len])
> -			goto slashes;
> -		inode = dentry->d_inode;
> -		ihold(inode);
> -		error = security_path_unlink(&path, dentry);
> -		if (error)
> -			goto exit3;
> -		error = vfs_unlink(mnt_idmap(path.mnt), path.dentry->d_inode,
> -				   dentry, &delegated_inode);
> -exit3:
> +	/* Why not before? Because we want correct error value */
> +	if (unlikely(last.name[last.len])) {
> +		if (d_is_dir(dentry))
> +			error = -EISDIR;
> +		else
> +			error = -ENOTDIR;
>  		end_dirop(dentry);
> +		goto exit_drop_write;
>  	}
> -	if (inode)
> -		iput(inode);	/* truncate the inode here */
> -	inode = NULL;
> +	inode = dentry->d_inode;
> +	ihold(inode);
> +	error = security_path_unlink(&path, dentry);
> +	if (error)
> +		goto exit_end_dirop;
> +	error = vfs_unlink(mnt_idmap(path.mnt), path.dentry->d_inode,
> +			   dentry, &delegated_inode);
> +exit_end_dirop:
> +	end_dirop(dentry);
> +	iput(inode);	/* truncate the inode here */
>  	if (delegated_inode) {
>  		error = break_deleg_wait(&delegated_inode);
>  		if (!error)
>  			goto retry_deleg;
>  	}
> +exit_drop_write:
>  	mnt_drop_write(path.mnt);
> -exit2:
> +exit_path_put:
>  	path_put(&path);
>  	if (retry_estale(error, lookup_flags)) {
>  		lookup_flags |= LOOKUP_REVAL;
> -		inode = NULL;
>  		goto retry;
>  	}
> -exit1:
> +exit_putname:
>  	putname(name);
>  	return error;
> -
> -slashes:
> -	if (d_is_dir(dentry))
> -		error = -EISDIR;
> -	else
> -		error = -ENOTDIR;
> -	goto exit3;
>  }
>  
>  SYSCALL_DEFINE3(unlinkat, int, dfd, const char __user *, pathname, int, flag)

Reviewed-by: Jeff Layton <jlayton@...nel.org>