Re: [Cluster-devel] [bug report] gfs2: Use mapping->gfp_mask for metadata inodes

On Wed, Aug 2, 2023 at 9:35 AM Dan Carpenter <[email protected]> wrote:
> Hello Andreas Gruenbacher,
>
> The patch 8f18190e3173: "gfs2: Use mapping->gfp_mask for metadata
> inodes" from Jul 26, 2023 (linux-next), leads to the following Smatch
> static checker warning:
>
>         fs/gfs2/inode.c:286 gfs2_lookup_simple()
>         error: 'inode' dereferencing possible ERR_PTR()
>
> fs/gfs2/inode.c
>     268 struct inode *gfs2_lookup_simple(struct inode *dip, const char *name)
>     269 {
>     270         struct qstr qstr;
>     271         struct inode *inode;
>     272         gfs2_str2qstr(&qstr, name);
>     273         inode = gfs2_lookupi(dip, &qstr, 1);
>     274         /* gfs2_lookupi has inconsistent callers: vfs
>     275          * related routines expect NULL for no entry found,
>     276          * gfs2_lookup_simple callers expect ENOENT
>     277          * and do not check for NULL.
>
> This comment is ancient.  I think how gfs2_lookupi() works is that if
> there is an error it returns an error code, but if the file does not
> exist it returns NULL.  (This is just based on vague assumptions about
> how mixed error pointer/NULL functions work).
>
>     278          */
>     279         if (inode == NULL)
>     280                 return ERR_PTR(-ENOENT);
>     281
>     282         /*
>     283          * Must not call back into the filesystem when allocating
>     284          * pages in the metadata inode's address space.
>     285          */
> --> 286         mapping_set_gfp_mask(inode->i_mapping, GFP_NOFS);
>                                      ^^^^^^^^^^^^^^^^
> error pointer dereference

Ah, right, thanks for pointing this out. I've pushed a fix.

Andreas

>     287
>     288         return inode;
>     289 }
>
> regards,
> dan carpenter