| Message ID | 20240807181553.243646-2-bodonnel@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2] xfs_db: release ip resource before returning from get_next_unlinked() | expand |
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 > >
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@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. 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 > > > > >
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); + libxfs_irele(ip); dbprintf(_("AG %u agino %u: %s\n"), agno, agino, strerror(error)); return NULLAGINO; }
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(+)