Re: ext4: make the bitmap read routines return real error codes

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Tue, Nov 03, 2015 at 03:04:45PM +0300, Dan Carpenter wrote:
> Hello Darrick J. Wong,
> 
> The patch 7d6232775976: "ext4: make the bitmap read routines return
> real error codes" from Oct 17, 2015, leads to the following static
> checker warning:
> 
> 	fs/ext4/mballoc.c:2989 ext4_mb_mark_diskspace_used()
> 	error: 'bitmap_bh' dereferencing possible ERR_PTR()
> 
> fs/ext4/mballoc.c
>   2899          bitmap_bh = ext4_read_block_bitmap(sb, ac->ac_b_ex.fe_group);
>   2900          if (IS_ERR(bitmap_bh)) {
>   2901                  err = PTR_ERR(bitmap_bh);
>   2902                  goto out_err;
>   2903          }
>   2904  
> 
> [ snip ]
> 
>   2987  
>   2988  out_err:
>   2989          brelse(bitmap_bh);
>   2990          return err;
>   2991  }
> 
> Also:
> 
> fs/ext4/mballoc.c:4894 ext4_free_blocks() error: 'bitmap_bh' dereferencing possible ERR_PTR()
> fs/ext4/mballoc.c:5028 ext4_group_add_blocks() error: 'bitmap_bh' dereferencing possible ERR_PTR()
> 
> This is One Err style error handling where one error label handles every
> possible error so it's error prone (handling every error is more
> complicated than doing a specific thing).
> 
> The old code relied on the sanity check in brelse() to avoid NULL
> dereferences but now we are passing ERR_PTRs so it's not enough.
> Probably the fix is to update the sanity check in brelse().  Another
> idea would be to not free things until they have been allocated.

Or just slip in a "bitmap_bh = NULL;" just before line 2902.  We've saved
the error code, so the pointer can be zeroed.

Hmm, thank you for the report, I'll get a patch out soon.  Guess I should
go figure out how to smatch-scan my dev tree. :)

(Particularly because I fixed this exact problem in other parts of the patch,
but not here. Sigh.)

--D

> 
> regards,
> dan carpenter