Re: A Potential Bug in fs/xfs/libxfs/xfs_bmap.c

["Darrick J. Wong" <djwong@kernel.org>]

[cc list]

On Fri, Jun 11, 2021 at 11:12:18PM -0700, Yizhuo Zhai wrote:
> Hi All:
> I just found a bug in the cramfs using the static analysis tool, but not

cramfs?  I thought we were in xfs.  Well, I get turned around easily.

> sure if this could happen in reality, could you please advise here? Thanks
> for your attention : )
> 
> In function xfs_bmap_del_extent_real
> <https://elixir.bootlin.com/linux/v5.12/source/fs/xfs/libxfs/v5.12/C/ident/xfs_bmap_del_extent_real>()
> , the structure "got" could be uninitialized if function "
> xfs_iext_get_extent
> <https://elixir.bootlin.com/linux/v5.12/source/fs/xfs/libxfs/v5.12/C/ident/xfs_iext_get_extent>()"
> returns false. However, there's no check for the return value but it is
> still used in the later code.

What's the state of the iext cursor?  Has it moved since the last time
anyone validated it?

--D

> 
> Here's the related code:
> 
> STATIC int xfs_bmap_del_extent_real ()
> {
>         struct xfs_bmbt_irec	got; //"got" declared here but not initialized
>         xfs_iext_get_extent(ifp, icur, &got); //"got" could be
> uninitialized if xfs_iext_get_extent() return false.
> 
> 
>         ASSERT(got.br_startoff <= del->br_startoff); //"got" is used
> here and later code
> }bool
> xfs_iext_get_extent(
> 	struct xfs_ifork	*ifp,
> 	struct xfs_iext_cursor	*cur,
> 	struct xfs_bmbt_irec	*gotp)
> {
> 	if (!xfs_iext_valid(ifp, cur))
> 		return false;
>         ...
> }
> 
> 
> 
> -- 
> Kind Regards,
> 
> *Yizhuo Zhai*
> 
> *Computer Science, Graduate Student*
> *University of California, Riverside *