On Wed, Aug 07, 2024 at 11:31:43AM -0700, Darrick J. Wong wrote:
> 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@xxxxxxxxxx>
> > ---
> > 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.
ugh. Missed that, thanks.
-Bill
>
> --D
>
> > + libxfs_irele(ip);
> > dbprintf(_("AG %u agino %u: %s\n"), agno, agino, strerror(error));
> > return NULLAGINO;
> > }
> > --
> > 2.45.2
> >
> >
>
