[Cluster-devel] [bug report] new ->follow_link() and ->put_link() calling conventions

[Cluster-devel] [bug report] new ->follow_link() and ->put_link() calling conventions

[Dan Carpenter]

Hello GFS2 devs,

The ancient patch 680baacbca69: "new ->follow_link() and ->put_link()
calling conventions" from May 2, 2015, leads to the following
Smatch static checker warning:

	fs/gfs2/inode.c:1806 gfs2_get_link()
	error: passing non negative 13 to ERR_PTR

fs/gfs2/inode.c
    1788 static const char *gfs2_get_link(struct dentry *dentry,
    1789                                  struct inode *inode,
    1790                                  struct delayed_call *done)
    1791 {
    1792         struct gfs2_inode *ip = GFS2_I(inode);
    1793         struct gfs2_holder i_gh;
    1794         struct buffer_head *dibh;
    1795         unsigned int size;
    1796         char *buf;
    1797         int error;
    1798 
    1799         if (!dentry)
    1800                 return ERR_PTR(-ECHILD);
    1801 
    1802         gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, &i_gh);
                                                          ^
i_gh.gh_flags = 0;  The async flag is not set.

    1803         error = gfs2_glock_nq(&i_gh);

So that means gfs2_glock_nq() can return GLR_TRYFAILED (13).

    1804         if (error) {
    1805                 gfs2_holder_uninit(&i_gh);
--> 1806                 return ERR_PTR(error);
                                ^^^^^^^^^^^^^^
Which will lead to an Oops in the caller.

    1807         }
    1808 
    1809         size = (unsigned int)i_size_read(&ip->i_inode);
    1810         if (size == 0) {
    1811                 gfs2_consist_inode(ip);
    1812                 buf = ERR_PTR(-EIO);
    1813                 goto out;
    1814         }
    1815 
    1816         error = gfs2_meta_inode_buffer(ip, &dibh);
    1817         if (error) {

regards,
dan carpenter

[Cluster-devel] [bug report] new ->follow_link() and ->put_link() calling conventions

[Andreas Gruenbacher]

Hi Dan,

On Tue, Aug 31, 2021 at 12:15 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Hello GFS2 devs,
>
> The ancient patch 680baacbca69: "new ->follow_link() and ->put_link()
> calling conventions" from May 2, 2015, leads to the following
> Smatch static checker warning:
>
>         fs/gfs2/inode.c:1806 gfs2_get_link()
>         error: passing non negative 13 to ERR_PTR

this looks like a false positive: gh->gh_error will only be set to
GLR_TRYFAILED when try_futile is true, and try_futile will only be
true when the LM_FLAG_TRY or LM_FLAG_TRY_1CB flag in gh->gh_flags is
set. The code is a bit convoluted, and it's not surprising that smatch
doesn't understand what's going on.

Thanks,
Andreas