linux-kernel - Re: [PATCH] fs: inline step_into() and walk

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: <20251119194210.GO2441659@ZenIV>
Date: Wed, 19 Nov 2025 19:42:10 +0000
From: Al Viro <viro@...iv.linux.org.uk>
To: Mateusz Guzik <mjguzik@...il.com>
Cc: brauner@...nel.org, jack@...e.cz, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] fs: inline step_into() and walk_component()

On Wed, Nov 19, 2025 at 07:40:01PM +0100, Mateusz Guzik wrote:

> -static const char *pick_link(struct nameidata *nd, struct path *link,
> +static noinline const char *pick_link(struct nameidata *nd, struct path *link,
>  		     struct inode *inode, int flags)
>  {
>  	struct saved *last;
>  	const char *res;
> -	int error = reserve_stack(nd, link);
> +	int error;
> +
> +	if (nd->flags & LOOKUP_RCU) {
> +		/* make sure that d_is_symlink above matches inode */

Where would that "above" be?  Have some pity on the readers...

> +static __always_inline const char *step_into(struct nameidata *nd, int flags,
> +		     struct dentry *dentry)
> +{
> +	struct path path;
> +	struct inode *inode;
> +
> +	path.mnt = nd->path.mnt;
> +	path.dentry = dentry;
> +	if (!(nd->flags & LOOKUP_RCU))
> +		goto slowpath;
> +	if (unlikely(dentry->d_flags & DCACHE_MANAGED_DENTRY))
> +		goto slowpath;
> +	inode = path.dentry->d_inode;
> +	if (unlikely(d_is_symlink(path.dentry)))
> +		goto slowpath;
> +	if (read_seqcount_retry(&path.dentry->d_seq, nd->next_seq))
> +		return ERR_PTR(-ECHILD);
> +	if (unlikely(!inode))
> +		return ERR_PTR(-ENOENT);
> +	nd->path = path;
> +	nd->inode = inode;
> +	nd->seq = nd->next_seq;
> +	return NULL;
> +slowpath:
> +	return step_into_slowpath(nd, flags, dentry);
> +}

Is there any point keeping that struct path here?  Hell, what's wrong
with

static __always_inline const char *step_into(struct nameidata *nd, int flags,
		     struct dentry *dentry)
{
	if (likely((nd->flags & LOOKUP_RCU) &&
	    !(dentry->d_flags & DCACHE_MANAGED_DENTRY) &&
	    !d_is_symlink(dentry))) {
		// Simple case: no messing with refcounts and
		// neither a symlink nor mountpoint.
		struct inode *inode = dentry->d_inode;

		if (read_seqcount_retry(&dentry->d_seq, nd->next_seq))
			return ERR_PTR(-ECHILD);
		if (unlikely(!inode))
			return ERR_PTR(-ENOENT);
		nd->path.dentry = dentry;
		nd->inode = inode;
		nd->seq = nd->next_seq;
		return NULL;
	}
	return step_into_slowpath(nd, flags, dentry);
}

for that matter?