vfs: get rid of BUG_ON() in d_mark_tmpfile_name()

Do proper error handling in d_mark_tmpfile_name() by returning errors
rather than using BUG_ON()'s.

Adjust caller to check for errors from d_mark_tmpfile_name() as well
as clean it up for using return value from scnprintf() in QSTR_LEN()
to make it more obvious where the tmpfile name's length is coming
from.

Link: https://lore.kernel.org/r/CAHk-=wgerpUKCDhdzKH0FEdLyfhj3doc9t+kO9Yb6rSsTp7hdQ@mail.gmail.com
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jan Kara <jack@suse.cz>
CC: linux-fsdevel@vger.kernel.org
Cc: linux-cifs@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/dcache.c b/fs/dcache.c
index df11bbba0342f3f6ee01a953dec97feb4dde1b8c..dbcbd0affb2648f15dee310665f2ff85751bdf93 100644 (file)
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -3196,15 +3196,18 @@ 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)
+int 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));
-       BUG_ON(d_really_is_positive(dentry));
-       BUG_ON(!d_unlinked(dentry));
-       BUG_ON(name->len > DNAME_INLINE_LEN - 1);
+       if (unlikely(dname_external(dentry) ||
+                    d_really_is_positive(dentry) ||
+                    !d_unlinked(dentry)))
+               return -EINVAL;
+       if (unlikely(name->len > DNAME_INLINE_LEN - 1))
+               return -ENAMETOOLONG;
+
        spin_lock(&dentry->d_parent->d_lock);
        spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);
        dentry->__d_name.len = name->len;
@@ -3212,6 +3215,7 @@ void d_mark_tmpfile_name(struct file *file, const struct qstr *name)
        dname[name->len] = '\0';
        spin_unlock(&dentry->d_lock);
        spin_unlock(&dentry->d_parent->d_lock);
+       return 0;
 }
 EXPORT_SYMBOL(d_mark_tmpfile_name);
 

diff --git a/fs/smb/client/cifsfs.h b/fs/smb/client/cifsfs.h
index 18f9f93a01b41f34378e66a6c8ccaffc4aa217a3..7370b38da938b77afc59ba7b743554f11e77a686 100644 (file)
--- a/fs/smb/client/cifsfs.h
+++ b/fs/smb/client/cifsfs.h
@@ -10,6 +10,7 @@
 #define _CIFSFS_H
 
 #include <linux/hash.h>
+#include <linux/dcache.h>
 
 #define ROOT_I 2
 
@@ -149,17 +150,11 @@ struct dentry *cifs_smb3_do_mount(struct file_system_type *fs_type, int flags,
 
 char *cifs_silly_fullpath(struct dentry *dentry);
 
-#define CIFS_TMPNAME_PREFIX        ".__smbfile_tmp"
-#define CIFS_TMPNAME_PREFIX_LEN    ((int)sizeof(CIFS_TMPNAME_PREFIX) - 1)
-#define CIFS_TMPNAME_COUNTER_LEN   ((int)sizeof(cifs_tmpcounter) * 2)
-#define CIFS_TMPNAME_LEN \
-       (CIFS_TMPNAME_PREFIX_LEN + CIFS_TMPNAME_COUNTER_LEN)
-
-#define CIFS_SILLYNAME_PREFIX      ".__smbfile_silly"
-#define CIFS_SILLYNAME_PREFIX_LEN  ((int)sizeof(CIFS_SILLYNAME_PREFIX) - 1)
-#define CIFS_SILLYNAME_COUNTER_LEN ((int)sizeof(cifs_sillycounter) * 2)
-#define CIFS_SILLYNAME_LEN \
-       (CIFS_SILLYNAME_PREFIX_LEN + CIFS_SILLYNAME_COUNTER_LEN)
+#define CIFS_TMPNAME_PREFIX    ".__smbfile_tmp"
+#define CIFS_TMPNAME_LEN       (DNAME_INLINE_LEN - 1)
+
+#define CIFS_SILLYNAME_PREFIX  ".__smbfile_silly"
+#define CIFS_SILLYNAME_LEN     (DNAME_INLINE_LEN - 1)
 
 #ifdef CONFIG_CIFS_NFSD_EXPORT
 extern const struct export_operations cifs_export_ops;

diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
index 6ea1ae7f7a4605ee2f531ceef730df18660873af..e4295a5b55b3473bfb6fa0f44e14bf5fa75a8ba4 100644 (file)
--- a/fs/smb/client/dir.c
+++ b/fs/smb/client/dir.c
@@ -1056,9 +1056,9 @@ int cifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
 {
        struct dentry *dentry = file->f_path.dentry;
        struct cifs_sb_info *cifs_sb = CIFS_SB(dir);
+       size_t namesize = CIFS_TMPNAME_LEN + 1;
        char *path __free(kfree) = NULL, *name;
        unsigned int oflags = file->f_flags;
-       size_t size = CIFS_TMPNAME_LEN + 1;
        int retries = 0, max_retries = 16;
        struct TCP_Server_Info *server;
        struct cifs_pending_open open;
@@ -1070,6 +1070,7 @@ int cifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
        struct inode *inode;
        unsigned int xid;
        __u32 oplock;
+       int namelen;
        int rc;
 
        if (unlikely(cifs_forced_shutdown(cifs_sb)))
@@ -1093,7 +1094,7 @@ int cifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
                server->ops->new_lease_key(&fid);
        cifs_add_pending_open(&fid, tlink, &open);
 
-       path = alloc_parent_path(dentry, size - 1);
+       path = alloc_parent_path(dentry, namesize - 1);
        if (IS_ERR(path)) {
                cifs_del_pending_open(&open);
                rc = PTR_ERR(path);
@@ -1103,16 +1104,22 @@ int cifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
 
        name = path + strlen(path);
        do {
-               scnprintf(name, size,
-                         CIFS_TMPNAME_PREFIX "%0*x",
-                         CIFS_TMPNAME_COUNTER_LEN,
-                         atomic_inc_return(&cifs_tmpcounter));
+               /* Append tmpfile name to @path */
+               namelen = scnprintf(name, namesize, CIFS_TMPNAME_PREFIX "%x",
+                                   atomic_inc_return(&cifs_tmpcounter));
                rc = __cifs_do_create(dir, dentry, path, xid, tlink, oflags,
                                      mode, &oplock, &fid, NULL, &inode);
                if (!rc) {
+                       rc = d_mark_tmpfile_name(file, &QSTR_LEN(name, namelen));
+                       if (rc) {
+                               cifs_dbg(VFS | ONCE, "%s: failed to set filename in dentry: %d\n",
+                                        __func__, rc);
+                               rc = -EISDIR;
+                               iput(inode);
+                               goto err_open;
+                       }
                        set_nlink(inode, 0);
                        mark_inode_dirty(inode);
-                       d_mark_tmpfile_name(file, &QSTR_LEN(name, size - 1));
                        d_instantiate(dentry, inode);
                        break;
                }
@@ -1168,9 +1175,7 @@ char *cifs_silly_fullpath(struct dentry *dentry)
 
        do {
                dput(sdentry);
-               scnprintf(name, namesize,
-                         CIFS_SILLYNAME_PREFIX "%0*x",
-                         CIFS_SILLYNAME_COUNTER_LEN,
+               scnprintf(name, namesize, CIFS_SILLYNAME_PREFIX "%x",
                          atomic_inc_return(&cifs_sillycounter));
                sdentry = lookup_noperm(&QSTR(name), dentry->d_parent);
                if (IS_ERR(sdentry))

diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index f60819dcfebd3e1f7ad7f3f22c409aa80f1069b3..c5bd5a74babab4d03288ba91ea8f6541fe29da71 100644 (file)
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -264,7 +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);
+int 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 *);