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
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
