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>

authored by

Paulo Alcantara and committed by

Steve French 2 months ago 15e9e00a 81dc1e4d

+30 -26

4 changed files

expand all

dcache.c

smb

client

cifsfs.h

dir.c

include

linux

dcache.h

+9 -5

fs/dcache.c

··· 3196 3196 } 3197 3197 EXPORT_SYMBOL(d_mark_tmpfile); 3198 3198 3199 - void d_mark_tmpfile_name(struct file *file, const struct qstr *name) 3199 + int d_mark_tmpfile_name(struct file *file, const struct qstr *name) 3200 3200 { 3201 3201 struct dentry *dentry = file->f_path.dentry; 3202 3202 char *dname = dentry->d_shortname.string; 3203 3203 3204 - BUG_ON(dname_external(dentry)); 3205 - BUG_ON(d_really_is_positive(dentry)); 3206 - BUG_ON(!d_unlinked(dentry)); 3207 - BUG_ON(name->len > DNAME_INLINE_LEN - 1); 3204 + if (unlikely(dname_external(dentry) || 3205 + d_really_is_positive(dentry) || 3206 + !d_unlinked(dentry))) 3207 + return -EINVAL; 3208 + if (unlikely(name->len > DNAME_INLINE_LEN - 1)) 3209 + return -ENAMETOOLONG; 3210 + 3208 3211 spin_lock(&dentry->d_parent->d_lock); 3209 3212 spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED); 3210 3213 dentry->__d_name.len = name->len; ··· 3215 3212 dname[name->len] = '\0'; 3216 3213 spin_unlock(&dentry->d_lock); 3217 3214 spin_unlock(&dentry->d_parent->d_lock); 3215 + return 0; 3218 3216 } 3219 3217 EXPORT_SYMBOL(d_mark_tmpfile_name); 3220 3218

+5 -10

fs/smb/client/cifsfs.h

··· 10 10 #define _CIFSFS_H 11 11 12 12 #include <linux/hash.h> 13 + #include <linux/dcache.h> 13 14 14 15 #define ROOT_I 2 15 16 ··· 150 149 151 150 char *cifs_silly_fullpath(struct dentry *dentry); 152 151 153 - #define CIFS_TMPNAME_PREFIX ".__smbfile_tmp" 154 - #define CIFS_TMPNAME_PREFIX_LEN ((int)sizeof(CIFS_TMPNAME_PREFIX) - 1) 155 - #define CIFS_TMPNAME_COUNTER_LEN ((int)sizeof(cifs_tmpcounter) * 2) 156 - #define CIFS_TMPNAME_LEN \ 157 - (CIFS_TMPNAME_PREFIX_LEN + CIFS_TMPNAME_COUNTER_LEN) 152 + #define CIFS_TMPNAME_PREFIX ".__smbfile_tmp" 153 + #define CIFS_TMPNAME_LEN (DNAME_INLINE_LEN - 1) 158 154 159 - #define CIFS_SILLYNAME_PREFIX ".__smbfile_silly" 160 - #define CIFS_SILLYNAME_PREFIX_LEN ((int)sizeof(CIFS_SILLYNAME_PREFIX) - 1) 161 - #define CIFS_SILLYNAME_COUNTER_LEN ((int)sizeof(cifs_sillycounter) * 2) 162 - #define CIFS_SILLYNAME_LEN \ 163 - (CIFS_SILLYNAME_PREFIX_LEN + CIFS_SILLYNAME_COUNTER_LEN) 155 + #define CIFS_SILLYNAME_PREFIX ".__smbfile_silly" 156 + #define CIFS_SILLYNAME_LEN (DNAME_INLINE_LEN - 1) 164 157 165 158 #ifdef CONFIG_CIFS_NFSD_EXPORT 166 159 extern const struct export_operations cifs_export_ops;

+15 -10

fs/smb/client/dir.c

··· 1056 1056 { 1057 1057 struct dentry *dentry = file->f_path.dentry; 1058 1058 struct cifs_sb_info *cifs_sb = CIFS_SB(dir); 1059 + size_t namesize = CIFS_TMPNAME_LEN + 1; 1059 1060 char *path __free(kfree) = NULL, *name; 1060 1061 unsigned int oflags = file->f_flags; 1061 - size_t size = CIFS_TMPNAME_LEN + 1; 1062 1062 int retries = 0, max_retries = 16; 1063 1063 struct TCP_Server_Info *server; 1064 1064 struct cifs_pending_open open; ··· 1070 1070 struct inode *inode; 1071 1071 unsigned int xid; 1072 1072 __u32 oplock; 1073 + int namelen; 1073 1074 int rc; 1074 1075 1075 1076 if (unlikely(cifs_forced_shutdown(cifs_sb))) ··· 1094 1093 server->ops->new_lease_key(&fid); 1095 1094 cifs_add_pending_open(&fid, tlink, &open); 1096 1095 1097 - path = alloc_parent_path(dentry, size - 1); 1096 + path = alloc_parent_path(dentry, namesize - 1); 1098 1097 if (IS_ERR(path)) { 1099 1098 cifs_del_pending_open(&open); 1100 1099 rc = PTR_ERR(path); ··· 1104 1103 1105 1104 name = path + strlen(path); 1106 1105 do { 1107 - scnprintf(name, size, 1108 - CIFS_TMPNAME_PREFIX "%0*x", 1109 - CIFS_TMPNAME_COUNTER_LEN, 1110 - atomic_inc_return(&cifs_tmpcounter)); 1106 + /* Append tmpfile name to @path */ 1107 + namelen = scnprintf(name, namesize, CIFS_TMPNAME_PREFIX "%x", 1108 + atomic_inc_return(&cifs_tmpcounter)); 1111 1109 rc = __cifs_do_create(dir, dentry, path, xid, tlink, oflags, 1112 1110 mode, &oplock, &fid, NULL, &inode); 1113 1111 if (!rc) { 1112 + rc = d_mark_tmpfile_name(file, &QSTR_LEN(name, namelen)); 1113 + if (rc) { 1114 + cifs_dbg(VFS | ONCE, "%s: failed to set filename in dentry: %d\n", 1115 + __func__, rc); 1116 + rc = -EISDIR; 1117 + iput(inode); 1118 + goto err_open; 1119 + } 1114 1120 set_nlink(inode, 0); 1115 1121 mark_inode_dirty(inode); 1116 - d_mark_tmpfile_name(file, &QSTR_LEN(name, size - 1)); 1117 1122 d_instantiate(dentry, inode); 1118 1123 break; 1119 1124 } ··· 1175 1168 1176 1169 do { 1177 1170 dput(sdentry); 1178 - scnprintf(name, namesize, 1179 - CIFS_SILLYNAME_PREFIX "%0*x", 1180 - CIFS_SILLYNAME_COUNTER_LEN, 1171 + scnprintf(name, namesize, CIFS_SILLYNAME_PREFIX "%x", 1181 1172 atomic_inc_return(&cifs_sillycounter)); 1182 1173 sdentry = lookup_noperm(&QSTR(name), dentry->d_parent); 1183 1174 if (IS_ERR(sdentry))

+1 -1

include/linux/dcache.h

··· 264 264 extern struct dentry * d_make_root(struct inode *); 265 265 266 266 extern void d_mark_tmpfile(struct file *, struct inode *); 267 - void d_mark_tmpfile_name(struct file *file, const struct qstr *name); 267 + int d_mark_tmpfile_name(struct file *file, const struct qstr *name); 268 268 extern void d_tmpfile(struct file *, struct inode *); 269 269 270 270 extern struct dentry *d_find_alias(struct inode *);

Configure Feed

Configure Feed