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

Paulo Alcantara
2026-04-04T15:54:11Z

Al Viro <viro@zeniv.linux.org.uk> writes:

> 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?

It's called with an unhashed negative dentry. Do you see any problems
with it?

>> +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().

I was thinking more along these lines (e.g. add a new VFS helper and
then call it in CIFS, as you suggested earlier)

diff --git a/fs/dcache.c b/fs/dcache.c
index 7ba1801d8132..c20a9c9e921c 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -3196,6 +3196,25 @@ void d_mark_tmpfile(struct file *file, struct inode *inode)
}
EXPORT_SYMBOL(d_mark_tmpfile);

+void d_mark_tmpfile_name(struct file *file, const struct qstr *name)
+{
+ struct dentry *dentry = file->f_path.dentry;
+ char *dname = dentry->d_shortname.string;
+
+ BUG_ON(dname_external(dentry) ||
+ d_really_is_positive(dentry) ||
+ !d_unlinked(dentry) ||
+ name->len > 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 = name->len;
+ memcpy(dname, name->name, name->len);
+ dname[name->len] = '\0';
+ spin_unlock(&dentry->d_lock);
+ spin_unlock(&dentry->d_parent->d_lock);
+}
+EXPORT_SYMBOL(d_mark_tmpfile_name);
+
void d_tmpfile(struct file *file, struct inode *inode)
{
struct dentry *dentry = file->f_path.dentry;
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 898c60d21c92..f60819dcfebd 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -264,6 +264,7 @@ extern void d_invalidate(struct dentry *);
extern struct dentry * d_make_root(struct inode *);

extern void d_mark_tmpfile(struct file *, struct inode *);
+void d_mark_tmpfile_name(struct file *file, const struct qstr *name);
extern void d_tmpfile(struct file *, struct inode *);

extern struct dentry *d_find_alias(struct inode *);

Looks good?

>> +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...

Yeah, that's definitely a TOCTOU race. I should've looped over
cifs_do_create() instead to make sure that the tmpfile is created in the
server, by using a sane retry counter.