On 3/13/25 2:07 PM, Bill O'Donnell wrote:
> On Wed, Feb 26, 2025 at 10:20:02AM -0800, Darrick J. Wong wrote:
...
>>> diff --git a/repair/da_util.c b/repair/da_util.c
>>> index 7f94f4012062..0a4785e6f69b 100644
>>> --- a/repair/da_util.c
>>> +++ b/repair/da_util.c
>>> @@ -66,6 +66,9 @@ da_read_buf(
>>> }
>>> libxfs_buf_read_map(mp->m_dev, map, nex, LIBXFS_READBUF_SALVAGE,
>>> &bp, ops);
>>> + if (bp->b_error == -EFSBADCRC) {
>>> + libxfs_buf_relse(bp);
>>
>> This introduces a use-after-free on the buffer pointer.
>
> I'm not groking why this is a case of use-after-free, and why it
> isn't in other cases? Where is the use after this free of bp?
>
> Thanks-
> Bill
because the final statement in this function is "return bp" - the
bp which just got released by your change, and callers of da_read_buf()
will proceed, assuming that the bp which was just released is still
safe to use.
The purpose of da_read_buf() is to read a buffer (bp) from disk, for
the caller to use. If you release that buffer and hand it back to the
caller to use anyway, that's a problem.
Note that with your patch in place, xfs_repair also issues a bunch of:
cache_node_put: node put on refcount 0 (node=0x7f6fb4073860)
type messages that weren't there before, implying there is now a
get/put imbalance.
-Eric
