Re: [PATCH] btrfs: fix a race in encoded read

[Johannes Thumshirn <Johannes.Thumshirn@wdc.com>]

On 12.12.24 10:08, Daniel Vacek wrote:
> On Thu, Dec 12, 2024 at 10:02 AM Johannes Thumshirn
> <Johannes.Thumshirn@wdc.com> wrote:
>>
>> On 12.12.24 09:53, Daniel Vacek wrote:
>>> On Thu, Dec 12, 2024 at 9:35 AM Johannes Thumshirn
>>> <Johannes.Thumshirn@wdc.com> wrote:
>>>>
>>>> On 12.12.24 09:09, Daniel Vacek wrote:
>>>>> Hi Johannes,
>>>>>
>>>>> On Thu, Dec 12, 2024 at 9:00 AM Johannes Thumshirn
>>>>> <Johannes.Thumshirn@wdc.com> wrote:
>>>>>>
>>>>>> On 12.12.24 08:54, Daniel Vacek wrote:
>>>>>>> While testing the encoded read feature the following crash was observed
>>>>>>> and it can be reliably reproduced:
>>>>>>>
>>>>>>
>>>>>>
>>>>>> Hi Daniel,
>>>>>>
>>>>>> This suspiciously looks like '05b36b04d74a ("btrfs: fix use-after-free
>>>>>> in btrfs_encoded_read_endio()")'. Do you have this patch applied to your
>>>>>> kernel? IIRC it went upstream with 6.13-rc2.
>>>>>
>>>>> Yes, I do. This one is on top of it. The crash happens with
>>>>> `05b36b04d74a` applied. All the crashes were reproduced with
>>>>> `feffde684ac2`.
>>>>>
>>>>> Honestly, `05b36b04d74a` looks a bit suspicious to me as it really
>>>>> does not look to deal correctly with the issue to me. I was a bit
>>>>> surprised/puzzled.
>>>>
>>>> Can you elaborate why?
>>>
>>> As it only touches one of those four atomic_dec_... lines. In theory
>>> the issue can happen also on the two async places, IIUC. It's only a
>>> matter of race probability.
>>>
>>>>> Anyways, I could reproduce the crash in a matter of half an hour. With
>>>>> this fix the torture is surviving for 22 hours atm.
>>>>
>>>> Do you also have '3ff867828e93 ("btrfs: simplify waiting for encoded
>>>> read endios")'? Looking at the diff it doesn't seems so.
>>>
>>> I cannot find that one. Am I missing something? Which repo are you using?
>>
>> The for-next branch for btrfs [1], which is what ppl developing against
>> btrfs should use. Can you please re-test with it and if needed re-base
>> your patch on top of it?
>>
>> [1] https://github.com/btrfs/linux for-next
> 
> I did check here and I don't really see the commit.
> 
> $ git remote -v
> origin    https://github.com/btrfs/linux.git (fetch)
> origin    https://github.com/btrfs/linux.git (push)
> $ git fetch
> $ git show 3ff867828e93 --
> fatal: bad revision '3ff867828e93'
> 
> Note, I was testing v6.13-rc1. This is a fix not a feature development.

It got recently force pushed, 34725028ec5500018f1cb5bfd55c669c7bbf1346 
it is now, sorry.