Dan Carpenter <dan.carpenter@xxxxxxxxxx> writes:
> Hello Luís Henriques,
>
> The patch dd66df0053ef: "ceph: add support for encrypted snapshot
> names" from Aug 25, 2022 (linux-next), leads to the following Smatch
> static checker warning:
>
> fs/ceph/crypto.c:252 parse_longname()
> warn: 'dir' is an error pointer or valid
>
> fs/ceph/crypto.c
> 211 static struct inode *parse_longname(const struct inode *parent,
> 212 const char *name, int *name_len)
> 213 {
> 214 struct inode *dir = NULL;
> 215 struct ceph_vino vino = { .snap = CEPH_NOSNAP };
> 216 char *inode_number;
> 217 char *name_end;
> 218 int orig_len = *name_len;
> 219 int ret = -EIO;
> 220
> 221 /* Skip initial '_' */
> 222 name++;
> 223 name_end = strrchr(name, '_');
> 224 if (!name_end) {
> 225 dout("Failed to parse long snapshot name: %s\n", name);
> 226 return ERR_PTR(-EIO);
> 227 }
> 228 *name_len = (name_end - name);
> 229 if (*name_len <= 0) {
> 230 pr_err("Failed to parse long snapshot name\n");
> 231 return ERR_PTR(-EIO);
> 232 }
> 233
> 234 /* Get the inode number */
> 235 inode_number = kmemdup_nul(name_end + 1,
> 236 orig_len - *name_len - 2,
> 237 GFP_KERNEL);
> 238 if (!inode_number)
> 239 return ERR_PTR(-ENOMEM);
> 240 ret = kstrtou64(inode_number, 10, &vino.ino);
> 241 if (ret) {
> 242 dout("Failed to parse inode number: %s\n", name);
> 243 dir = ERR_PTR(ret);
> 244 goto out;
> 245 }
> 246
> 247 /* And finally the inode */
> 248 dir = ceph_find_inode(parent->i_sb, vino);
> 249 if (!dir) {
> 250 /* This can happen if we're not mounting cephfs on the root */
> 251 dir = ceph_get_inode(parent->i_sb, vino, NULL);
> --> 252 if (!dir)
> 253 dir = ERR_PTR(-ENOENT);
>
> This never returns NULL. If it were tempted to return NULL then it
> returns -ENOMEM instead.
Oops, you're right. But the fix should be as easy as removing that 'if'
statement and let this function simply return 'dir' as-is.
I'll send out an updated version of the patch in reply to this email.
Thanks for the report, Dan.
Cheers,
--
Luís
>
> 254 }
> 255 if (IS_ERR(dir))
> 256 dout("Can't find inode %s (%s)\n", inode_number, name);
> 257
> 258 out:
> 259 kfree(inode_number);
> 260 return dir;
> 261 }
>
> regards,
> dan carpenter
