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

[JP Kobryn <inwardvessel@gmail.com>]

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 it's lacking the reason why it's a bug fix, I believe you can modify 
> the commit message to include the analyze and the fixes tag.
> 
> 
> I'm also curious to learn the proper way for such situation.

It's new to me as well. For reference, there are some commits on the
list that have language like this: "This is a stable-only fix".