Re: [PATCH 6.12] btrfs: prevent use-after-free prealloc_file_extent_cluster()

[JP Kobryn <inwardvessel@gmail.com>]

On 1/31/26 3:32 PM, Qu Wenruo wrote:
> 
> 
> 在 2026/2/1 09:53, JP Kobryn 写道:
>> On 1/31/26 1:08 PM, Qu Wenruo wrote:
>>>
>>>
>>> 在 2026/2/1 05:23, JP Kobryn 写道:
>>>> Users of filemap_lock_folio() need to guard against the situation where
>>>> release_folio() has been invoked during reclaim but the folio was
>>>> ultimately not removed from the page cache. This patch covers one 
>>>> location
>>>> that was overlooked. Affected code has changed as of 6.17, so this 
>>>> patch is
>>>> only targeting stable trees prior.
>>>>
>>>> After acquiring the folio, use set_folio_extent_mapped() to ensure the
>>>> folio private state is valid. This is especially important in the 
>>>> subpage
>>>> case, where the private field is an allocated struct containing 
>>>> bitmap and
>>>> lock data.
>>>>
>>>> Without this protection, the race below is possible:
>>>>
>>>> [mm] page cache reclaim path        [fs] relocation in subpage mode
>>>> shrink_folio_list()
>>>>    folio_trylock() /* lock acquired */
>>>>    filemap_release_folio()
>>>>      mapping->a_ops->release_folio()
>>>>        btrfs_release_folio()
>>>>          __btrfs_release_folio()
>>>>            clear_folio_extent_mapped()
>>>>              btrfs_detach_folio_state()
>>>>                bfs = folio_detach_private(folio)
>>>>                btrfs_free_folio_state(folio)
>>>>                  kfree(bfs) /* point A */
>>>>
>>>>                                     prealloc_file_extent_cluster()
>>>>                                       filemap_lock_folio()
>>>>                                         folio_try_get() /* inc 
>>>> refcount */
>>>>                                         folio_lock() /* wait for 
>>>> lock */
>>>>
>>>>    if (...)
>>>>      ...
>>>>    else if (!mapping || !__remove_mapping(..))
>>>>      /*
>>>>       * __remove_mapping() returns zero when
>>>>       * folio_ref_freeze(folio, refcount) fails /* point B */
>>>>       */
>>>>      goto keep_locked /* folio remains in cache */
>>>>
>>>> keep_locked:
>>>>    folio_unlock(folio) /* lock released */
>>>>
>>>>                                     /* lock acquired */
>>>>                                     btrfs_subpage_clear_updodate()
>>>>                                       bfs = folio->priv /* use- 
>>>> after- free */
>>>
>>> This patch itself and the root cause look good to me.
>>>
>>> Reviewed-by: Qu Wenruo <wqu@suse.com>
>>>
>>
>> Much appreciated :)
>>
>>>>
>>>> This patch is intended as a minimal fix for backporting to affected
>>>> kernels. As of 6.17, a commit [0] replaced the vulnerable
>>>> filemap_lock_folio() + btrfs_subpage_clear_uptodate() sequence with
>>>> filemap_invalidate_inode() avoiding the race entirely. That commit 
>>>> was part
>>>> of a series with a different goal of preparing for large folio 
>>>> support so
>>>> backporting may not be straight forward.
>>>
>>> However I'm not sure if stable tree even accepts non-upstreamed patches.
>>>
>>> Thus the stable maintainer may ask you the same question as I did 
>>> before, why not backport the upstream commit 4e346baee95f?
>>
>> That commit relies on filemap_invalidate_folio() which was introduced in
>> 6.10 so it would not apply to earlier stable branches.
>>
>> We need to fix as far back as 5.15 so I can send one additional patch to
>> cover stable trees 5.15 to 6.6. The patch would be almost identical,
>> with the only change being using the page API instead of the folio API
>> (set_folio_extent_mapped() -> set_page_extent_mapped()). Let me know if
>> you're in agreement and I can send the extra patch.
> 
> If stable chooses to go this path, I'm totally fine with similar backports.
> 

Great. Let me resend this patch and the additional one with the proper
stable tags showing the specific kernel versions each apply to.