On Mon, 17 Feb 2025, Dan Carpenter wrote:
> Hello NeilBrown,
>
> Commit 22d9d5e93d0e ("VFS: add common error checks to
> lookup_one_qstr_excl()") from Feb 7, 2025 (linux-next), leads to the
> following Smatch static checker warning:
>
> fs/namei.c:1696 lookup_one_qstr_excl()
> error: 'dentry' dereferencing possible ERR_PTR()
>
> fs/namei.c
> 1671 struct dentry *lookup_one_qstr_excl(const struct qstr *name,
> 1672 struct dentry *base,
> 1673 unsigned int flags)
> 1674 {
> 1675 struct dentry *dentry = lookup_dcache(name, base, flags);
> 1676 struct dentry *old;
> 1677 struct inode *dir = base->d_inode;
> 1678
> 1679 if (dentry)
> 1680 goto found;
>
> It looks like lookup_dcache() can return both error pointers and NULL.
Yes it can.
>
> 1681
> 1682 /* Don't create child dentry for a dead directory. */
> 1683 if (unlikely(IS_DEADDIR(dir)))
> 1684 return ERR_PTR(-ENOENT);
> 1685
> 1686 dentry = d_alloc(base, name);
> 1687 if (unlikely(!dentry))
> 1688 return ERR_PTR(-ENOMEM);
> 1689
> 1690 old = dir->i_op->lookup(dir, dentry, flags);
> 1691 if (unlikely(old)) {
> 1692 dput(dentry);
> 1693 dentry = old;
> 1694 }
> 1695 found:
> 1696 if (d_is_negative(dentry) && !(flags & LOOKUP_CREATE)) {
> ^^^^^^
> Unchecked dereference.
so that is bad. A corrected version was merged in the vfs tree a few
hours after this post.
Thanks for your ongoing service!
NeilBrown
>
> 1697 dput(dentry);
> 1698 return ERR_PTR(-ENOENT);
> 1699 }
> 1700 if (d_is_positive(dentry) && (flags & LOOKUP_EXCL)) {
> 1701 dput(dentry);
> 1702 return ERR_PTR(-EEXIST);
> 1703 }
> 1704 return dentry;
> 1705 }
>
> regards,
> dan carpenter
>
