Re: [PATCH] btrfs: scrub: avoid use-after-free when chunk end is not 64K aligned

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2024/1/17 11:25, David Sterba wrote:
> On Wed, Jan 17, 2024 at 06:36:00AM +1030, Qu Wenruo wrote:
>> On 2024/1/17 04:58, David Sterba wrote:
>>> On Tue, Jan 16, 2024 at 09:20:58AM +1030, Qu Wenruo wrote:
>>>> On 2024/1/15 22:39, Johannes Thumshirn wrote:
>>>> [...]
>>>>>
>>>>>> - Make sure scrub_submit_initial_read() only to read the chunk range
>>>>>>       This is done by calculating the real number of sectors we need to
>>>>>>       read, and add sector-by-sector to the bio.
>>>>>
>>>>> Why can't you do it the same way the RST version does it by checking the
>>>>> extent_sector_bitmap and then add sector-by-sector from it?
>>>>
>>>> Sure, we can, although the whole new scrub code is before RST, and at
>>>> that time, the whole 64K read behavior is considered as a better option,
>>>> as it reduces the IOPS for a fragmented stripe.
>>>
>>> I'd like to keep the scrub fix separte from the RST code, even if
>>> there's a chance for some code sharing or reuse. The scrub fix needs to
>>> be backported so it's better to keep it independent.
>>
>> So do I need to split the fix, so that the first part would be purely
>> for the non-RST scrub part, and then a small fix to the RST part?
>>
>> I can try to do that, but since we need to touch the read endio function
>> anyway, it may mean if we don't do it properly, it may break bisection.
>>
>> Thus I'd prefer to do a manual backport for the older branches without
>> the RST code.
>
> I was not sure how much the scrub and RST are entangled so it was a
> suggestion to make the backport workable. It is preferred by stable to
> take patches 1:1 regarding the code changes (context adjustments are
> ok). In this case the manual backport would be needed, let's say one
> patch is taken without change and another one (regarding the RST
> changes) would be manualy tweaked.
>
No big deal.

It turns out that, I can indeed split the patch into two.

The change on scrub_read_endio() is already shared by both regular and
RST scrub paths.

The only change I did to RST specific path can be split out into another
patch, which doesn't even need to be backported.

So yes, the split is possible, and it should make later backport much
easier.

Thanks,
Qu