On Wed, Aug 07, 2024 at 02:38:03PM -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@xxxxxxxxxx>
> ---
> v2: cover error case.
> v3: fix coverage to not release unitialized variable.
> ---
> db/iunlink.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/db/iunlink.c b/db/iunlink.c
> index d87562e3..57e51140 100644
> --- a/db/iunlink.c
> +++ b/db/iunlink.c
> @@ -66,15 +66,18 @@ get_next_unlinked(
> }
>
> error = -libxfs_imap_to_bp(mp, NULL, &ip->i_imap, &ino_bp);
> - if (error)
> + if (error) {
> + libxfs_buf_relse(ino_bp);
Sorry, I think I've led you astray -- it's not necessary to
libxfs_buf_relse in any of the bailouts.
--D
> goto bad;
> -
> + }
> 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_irele(ip);
> dbprintf(_("AG %u agino %u: %s\n"), agno, agino, strerror(error));
> return NULLAGINO;
> }
> --
> 2.45.2
>
>
