[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <61b5f5fd-ed50-40b3-af32-159106f0af7c@gmail.com>
Date: Sat, 31 Jan 2026 20:33:18 -0800
From: JP Kobryn <inwardvessel@...il.com>
To: Qu Wenruo <wqu@...e.com>, boris@....io, clm@...com, dsterba@...e.com
Cc: linux-btrfs@...r.kernel.org, stable@...r.kernel.org,
linux-kernel@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH 6.12] btrfs: prevent use-after-free
prealloc_file_extent_cluster()
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@...e.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.
Powered by blists - more mailing lists