lkml.org 
[lkml]   [2022]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [kbuild] [djwong-xfs:djwong-wtf 349/351] fs/xfs/xfs_bmap_util.c:1372 xfs_map_free_extent() warn: missing error code 'error'
On Mon, Mar 21, 2022 at 02:59:08PM -0700, Darrick J. Wong wrote:
> On Mon, Mar 21, 2022 at 10:33:02AM +0300, Dan Carpenter wrote:
> > b82670045aab66 Darrick J. Wong 2022-01-06 1365
> > b82670045aab66 Darrick J. Wong 2022-01-06 1366 error = xfs_alloc_find_freesp(tp, pag, cursor, end_agbno, &len);
> > b82670045aab66 Darrick J. Wong 2022-01-06 1367 if (error)
> > b82670045aab66 Darrick J. Wong 2022-01-06 1368 goto out_cancel;
> > b82670045aab66 Darrick J. Wong 2022-01-06 1369
> > b82670045aab66 Darrick J. Wong 2022-01-06 1370 /* Bail out if the cursor is beyond what we asked for. */
> > b82670045aab66 Darrick J. Wong 2022-01-06 1371 if (*cursor >= end_agbno)
> > b82670045aab66 Darrick J. Wong 2022-01-06 @1372 goto out_cancel;
> >
> > This looks like it should have an error = -EINVAL;
>
> Nope. If xfs_alloc_find_freesp moves @cursor goes beyond end_agbno, we
> want to exit early so that the xfs_map_free_extent caller will return to
> userspace.
>
> --D

I'm generally pretty happy with this static checker rule. Returning
success on a failure path almost always results if something bad like a
NULL deref or a use after free. But false positives are a real risk
because it's tempting to add an error code to this and introduce a bug.

Smatch will not print the warning if error is set within 4 lines of the
goto.
error = 0;
if (*cursor >= end_agbno)
goto out_cancel;

Another option is that people have started adding comments to these
blocks in response to the checker warning.

Or if you had a different idea about how to silence the checker warning
I can also probably implement that.

regards,
dan carpenter


\
 
 \ /
  Last update: 2022-03-22 06:54    [W:0.079 / U:0.292 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site