On 8/5/24 9:42 AM, Bill O'Donnell wrote:
> On Fri, Aug 02, 2024 at 04:23:00PM -0700, Darrick J. Wong wrote:
>> On Fri, Aug 02, 2024 at 05:25:52PM -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>
>>> ---
>>> db/iunlink.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/db/iunlink.c b/db/iunlink.c
>>> index d87562e3..3b2417c5 100644
>>> --- a/db/iunlink.c
>>> +++ b/db/iunlink.c
>>> @@ -72,6 +72,7 @@ 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);
>>
>> I think this needs to cover the error return for libxfs_imap_to_bp too,
>> doesn't it?
>
> I considered that, but there are several places in the code where the
> error return doesn't release the resource. Not that that's correct, but the
> scans didn't flag them. For example, in bmap_inflate.c, bmapinflate_f()
> does not release the resource and scans didn't flag it.
Upstream coverity scan does flag it, CID 1554242 (that CID actually covers
both instances of the leak).
-Eric
> Thanks-
> Bill
>
>
>>
>> --D
>>
>>>
>>> return ret;
>>> bad:
>>> --
>>> 2.45.2
>>>
>>
>
>
