2023-05-04 20:18 GMT+09:00, Dan Carpenter <dan.carpenter@xxxxxxxxxx>:
> Hello Namjae Jeon,
>
> The patch 74d7970febf7: "ksmbd: fix racy issue from using ->d_parent
> and ->d_name" from Apr 21, 2023, leads to the following Smatch static
> checker warning:
>
> fs/ksmbd/vfs.c:1159 ksmbd_vfs_kern_path_locked()
> info: return a literal instead of 'err'
>
> fs/ksmbd/vfs.c
> 1149 int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char
> *name,
> 1150 unsigned int flags, struct path
> *path,
> 1151 bool caseless)
> 1152 {
> 1153 struct ksmbd_share_config *share_conf =
> work->tcon->share_conf;
> 1154 int err;
> 1155 struct path parent_path;
> 1156
> 1157 err = ksmbd_vfs_path_lookup_locked(share_conf, name, flags,
> path);
> 1158 if (!err)
> --> 1159 return err;
>
> Originally this was return 0, but the patch changed it to return ret.
> The reason for this check is that sometimes people accidentally reverse
> their if statements so they write if (!err) instead of if (err). Here
> it looks intentional but it's hard to be sure.
Okay. Can you send the patch for this ? I don't know how to write the
patch description.
Thanks!
>
> 1160
> 1161 if (caseless) {
> 1162 char *filepath;
> 1163 size_t path_len, remain_len;
> 1164
> 1165 filepath = kstrdup(name, GFP_KERNEL);
> 1166 if (!filepath)
> 1167 return -ENOMEM;
> 1168
> 1169 path_len = strlen(filepath);
> 1170 remain_len = path_len;
> 1171
> 1172 parent_path = share_conf->vfs_path;
> 1173 path_get(&parent_path);
> 1174
> 1175 while (d_can_lookup(parent_path.dentry)) {
> 1176 char *filename = filepath + path_len -
> remain_len;
> 1177 char *next = strchrnul(filename, '/');
> 1178 size_t filename_len = next - filename;
> 1179 bool is_last = !next[0];
> 1180
> 1181 if (filename_len == 0)
> 1182 break;
> 1183
> 1184 err = ksmbd_vfs_lookup_in_dir(&parent_path,
> filename,
> 1185
> filename_len,
> 1186
> work->conn->um);
> 1187 if (err)
> 1188 goto out2;
> 1189
> 1190 next[0] = '\0';
> 1191
> 1192 err =
> vfs_path_lookup(share_conf->vfs_path.dentry,
> 1193
> share_conf->vfs_path.mnt,
> 1194 filepath,
> 1195 flags,
> 1196 path);
> 1197 if (err)
> 1198 goto out2;
> 1199 else if (is_last)
> 1200 goto out1;
> 1201 path_put(&parent_path);
> 1202 parent_path = *path;
> 1203
> 1204 next[0] = '/';
> 1205 remain_len -= filename_len + 1;
> 1206 }
> 1207
> 1208 err = -EINVAL;
> 1209 out2:
> 1210 path_put(&parent_path);
> 1211 out1:
> 1212 kfree(filepath);
> 1213 }
> 1214
> 1215 if (!err) {
> 1216 err = ksmbd_vfs_lock_parent(parent_path.dentry,
> path->dentry);
> 1217 if (err)
> 1218 dput(path->dentry);
> 1219 path_put(&parent_path);
> 1220 }
> 1221 return err;
> 1222 }
>
> regards,
> dan carpenter
>
