Re: [RFC PATCH] smb: client: add support for O_TMPFILE

[Al Viro <viro@zeniv.linux.org.uk>]

On Tue, Mar 31, 2026 at 10:11:53PM -0300, Paulo Alcantara wrote:

>  	d_drop(direntry);
> -	d_add(direntry, newinode);
> -
> +	inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
> +	if (oflags & O_TMPFILE) {
> +		set_nlink(newinode, 0);
> +		mark_inode_dirty(newinode);
> +		d_instantiate(direntry, newinode);
> +	} else {
> +		d_add(direntry, newinode);
> +	}

What d_unhashed(dentry) is going to be when we arrive to that thing
with O_TMPFILE?

> +static void cifs_d_mark_tmpfile(struct file *file,
> +				const unsigned char *name,
> +				size_t namelen)
> +{
> +	struct dentry *dentry = file->f_path.dentry;
> +
> +	BUG_ON(dentry->d_name.name != dentry->d_shortname.string ||
> +	       !hlist_unhashed(&dentry->d_u.d_alias) ||
> +	       !d_unlinked(dentry) ||
> +	       namelen > DNAME_INLINE_LEN - 1);
> +	spin_lock(&dentry->d_parent->d_lock);
> +	spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);
> +	dentry->__d_name.len = sprintf(dentry->d_shortname.string, "%.*s",
> +				       (int)namelen, name);

That's one hell of an odd way to spell that...  What's wrong with using union
shortname_store for an argument?  Just memchr() for '\0' to verify it's there
and use the result to calculate the length and plain assignment to d_shortname
for copying...

And in any case, that !hlist_unhashed(....) in there is d_really_is_positive().

> +static int set_tmpfile_name(struct file *file)
> +{
> +	struct dentry *dentry = file->f_path.dentry;
> +	unsigned char name[CIFS_TMPNAME_LEN + 1];
> +	struct dentry *sdentry = NULL;
> +
> +	do {
> +		dput(sdentry);
> +		scnprintf(name, sizeof(name),
> +			  CIFS_TMPNAME_PREFIX "%0*x",
> +			  CIFS_TMPNAME_COUNTER_LEN,
> +			  atomic_inc_return(&cifs_tmpcounter));
> +		sdentry = lookup_noperm_unlocked(&QSTR(name), dentry->d_parent);
> +		if (IS_ERR(sdentry))
> +			return -EBUSY;
> +	} while (!d_is_negative(sdentry));
> +	dput(sdentry);

That looks racy.  Checking it doesn't exist at the moment is fine, but what if
it's created right after that lookup?  You are not holding any locks, so even
the same-client race (with plain create()) is possible...

> +	cifs_d_mark_tmpfile(file, name, sizeof(name) - 1);
> +	return 0;
> +}