Re: [PATCH v2] xfs_db: release ip resource before returning from get_next_unlinked()

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

On Wed, Aug 07, 2024 at 01:15:54PM -0500, Bill O'Donnell wrote:
> Fix potential memory leak in function get_next_unlinked(). Call
> libxfs_irele(ip) before exiting.
> 
> Details:
> Error: RESOURCE_LEAK (CWE-772):
> xfsprogs-6.5.0/db/iunlink.c:51:2: alloc_arg: "libxfs_iget" allocates memory that is stored into "ip".
> xfsprogs-6.5.0/db/iunlink.c:68:2: noescape: Resource "&ip->i_imap" is not freed or pointed-to in "libxfs_imap_to_bp".
> xfsprogs-6.5.0/db/iunlink.c:76:2: leaked_storage: Variable "ip" going out of scope leaks the storage it points to.
> #   74|   	libxfs_buf_relse(ino_bp);
> #   75|
> #   76|-> 	return ret;
> #   77|   bad:
> #   78|   	dbprintf(_("AG %u agino %u: %s\n"), agno, agino, strerror(error));
> 
> Signed-off-by: Bill O'Donnell <bodonnel@redhat.com>
> ---
> v2: cover the error case.
> ---
>  db/iunlink.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/db/iunlink.c b/db/iunlink.c
> index d87562e3..bd973600 100644
> --- a/db/iunlink.c
> +++ b/db/iunlink.c
> @@ -72,9 +72,12 @@ get_next_unlinked(
>  	dip = xfs_buf_offset(ino_bp, ip->i_imap.im_boffset);
>  	ret = be32_to_cpu(dip->di_next_unlinked);
>  	libxfs_buf_relse(ino_bp);
> +	libxfs_irele(ip);
>  
>  	return ret;
>  bad:
> +	libxfs_buf_relse(ino_bp);

What if we got here via the first 'goto bad' in this function?  We'll be
feeding an uninitialized variable into libxfs_buf_relse.

--D

> +	libxfs_irele(ip);
>  	dbprintf(_("AG %u agino %u: %s\n"), agno, agino, strerror(error));
>  	return NULLAGINO;
>  }
> -- 
> 2.45.2
> 
>