erofs: is z_erofs_put_pcluster()'s sbi access in the same UAF window as 1aee05e814d2?

erofs: is z_erofs_put_pcluster()'s sbi access in the same UAF window as 1aee05e814d2?

[Zhan Xusheng]

From: Zhan Xusheng <zhanxusheng1024@gmail.com>

Hi Xiang,

Following the race model established by commit 1aee05e814d2 ("erofs: fix
use-after-free on sbi->sync_decompress") -- i.e. unmount does not drain
the async decompress kworker, and unlocking the output folios lets inode
eviction (truncate_inode_pages waits on the locked folios) and thus the
unmount path proceed to kfree(sbi) -- I'd like to ask about another sbi
access that also happens after the unlock, in the same kworker.

In z_erofs_decompress_pcluster():

	erofs_onlinefolio_end(page_folio(page), err, true);  /* unlock */
	...
	z_erofs_put_pcluster(sbi, pcl, try_free);

and in z_erofs_put_pcluster():

	if (try_free && xa_trylock(&sbi->managed_pslots)) {
		free = __erofs_try_to_release_pcluster(sbi, pcl);
		xa_unlock(&sbi->managed_pslots);
	}

So in the try_free path it dereferences sbi->managed_pslots after the
output folios have been unlocked, which on control-flow alone looks
similar to the UAF fixed by 1aee05e814d2.

What makes me unsure, though, is a difference from the sync_decompress
case: sync_decompress is just a plain sbi member, whereas here
z_erofs_put_pcluster() is still operating on a live pcluster that is
registered in sbi->managed_pslots / the managed cache. So it's not clear
to me whether the pcluster / managed-cache lifetime rules implicitly pin
the filesystem instance and keep sbi valid across this window.

This also seems much harder to hit than the sync_decompress case: it is
conditional (try_free, i.e. non-managed compressed pages, plus the
pcluster refcount reaching zero), the window between the unlock and
put_pcluster is narrow, and unmount still has evict_inodes/put_super work
to do before kfree(sbi) -- which may be why syzbot didn't reach it.

Is there any guarantee that sbi stays valid here after the output folios
are unlocked (e.g. via pcluster / managed-cache lifetime or RCU), or
could unmount race with this path similarly to 1aee05e814d2? I'm asking
rather than sending a patch since I couldn't convince myself either way.

Thanks,
  Zhan Xusheng

Re: erofs: is z_erofs_put_pcluster()'s sbi access in the same UAF window as 1aee05e814d2?

[Gao Xiang]

On 2026/6/23 10:49, Zhan Xusheng wrote:
> From: Zhan Xusheng <zhanxusheng1024@gmail.com>
> 
> Hi Xiang,
> 
> Following the race model established by commit 1aee05e814d2 ("erofs: fix
> use-after-free on sbi->sync_decompress") -- i.e. unmount does not drain
> the async decompress kworker, and unlocking the output folios lets inode
> eviction (truncate_inode_pages waits on the locked folios) and thus the
> unmount path proceed to kfree(sbi) -- I'd like to ask about another sbi
> access that also happens after the unlock, in the same kworker.
> 
> In z_erofs_decompress_pcluster():
> 
> 	erofs_onlinefolio_end(page_folio(page), err, true);  /* unlock */
> 	...
> 	z_erofs_put_pcluster(sbi, pcl, try_free);
> 
> and in z_erofs_put_pcluster():
> 
> 	if (try_free && xa_trylock(&sbi->managed_pslots)) {
> 		free = __erofs_try_to_release_pcluster(sbi, pcl);
> 		xa_unlock(&sbi->managed_pslots);
> 	}
> 
> So in the try_free path it dereferences sbi->managed_pslots after the
> output folios have been unlocked, which on control-flow alone looks
> similar to the UAF fixed by 1aee05e814d2.
> 
> What makes me unsure, though, is a difference from the sync_decompress
> case: sync_decompress is just a plain sbi member, whereas here
> z_erofs_put_pcluster() is still operating on a live pcluster that is
> registered in sbi->managed_pslots / the managed cache. So it's not clear
> to me whether the pcluster / managed-cache lifetime rules implicitly pin
> the filesystem instance and keep sbi valid across this window.
> 
> This also seems much harder to hit than the sync_decompress case: it is
> conditional (try_free, i.e. non-managed compressed pages, plus the
> pcluster refcount reaching zero), the window between the unlock and
> put_pcluster is narrow, and unmount still has evict_inodes/put_super work
> to do before kfree(sbi) -- which may be why syzbot didn't reach it.
> 
> Is there any guarantee that sbi stays valid here after the output folios
> are unlocked (e.g. via pcluster / managed-cache lifetime or RCU), or
> could unmount race with this path similarly to 1aee05e814d2? I'm asking
> rather than sending a patch since I couldn't convince myself either way.

No, this is totally false-positive.

> 
> Thanks,
>    Zhan Xusheng

Re: erofs: is z_erofs_put_pcluster()'s sbi access in the same UAF window as 1aee05e814d2?

[Gao Xiang]

On 2026/6/23 10:52, Gao Xiang wrote:
> 
> 
> On 2026/6/23 10:49, Zhan Xusheng wrote:
>> From: Zhan Xusheng <zhanxusheng1024@gmail.com>
>>
>> Hi Xiang,
>>
>> Following the race model established by commit 1aee05e814d2 ("erofs: fix
>> use-after-free on sbi->sync_decompress") -- i.e. unmount does not drain
>> the async decompress kworker, and unlocking the output folios lets inode
>> eviction (truncate_inode_pages waits on the locked folios) and thus the
>> unmount path proceed to kfree(sbi) -- I'd like to ask about another sbi
>> access that also happens after the unlock, in the same kworker.
>>
>> In z_erofs_decompress_pcluster():
>>
>>     erofs_onlinefolio_end(page_folio(page), err, true);  /* unlock */
>>     ...
>>     z_erofs_put_pcluster(sbi, pcl, try_free);
>>
>> and in z_erofs_put_pcluster():
>>
>>     if (try_free && xa_trylock(&sbi->managed_pslots)) {
>>         free = __erofs_try_to_release_pcluster(sbi, pcl);
>>         xa_unlock(&sbi->managed_pslots);
>>     }
>>
>> So in the try_free path it dereferences sbi->managed_pslots after the
>> output folios have been unlocked, which on control-flow alone looks
>> similar to the UAF fixed by 1aee05e814d2.
>>
>> What makes me unsure, though, is a difference from the sync_decompress
>> case: sync_decompress is just a plain sbi member, whereas here
>> z_erofs_put_pcluster() is still operating on a live pcluster that is
>> registered in sbi->managed_pslots / the managed cache. So it's not clear
>> to me whether the pcluster / managed-cache lifetime rules implicitly pin
>> the filesystem instance and keep sbi valid across this window.
>>
>> This also seems much harder to hit than the sync_decompress case: it is
>> conditional (try_free, i.e. non-managed compressed pages, plus the
>> pcluster refcount reaching zero), the window between the unlock and
>> put_pcluster is narrow, and unmount still has evict_inodes/put_super work
>> to do before kfree(sbi) -- which may be why syzbot didn't reach it.
>>
>> Is there any guarantee that sbi stays valid here after the output folios
>> are unlocked (e.g. via pcluster / managed-cache lifetime or RCU), or
>> could unmount race with this path similarly to 1aee05e814d2? I'm asking
>> rather than sending a patch since I couldn't convince myself either way.
> 
> No, this is totally false-positive.

In short, I saw some similar report from LLMs, but I think
erofs_shrinker_unregister() should block this from kfree(sbi)
by design.

Thanks,
Gao Xiang

> 
>>
>> Thanks,
>>    Zhan Xusheng
> 

Re: erofs: is z_erofs_put_pcluster()'s sbi access in the same UAF window as 1aee05e814d2?

[Zhan Xusheng]

From: Zhan Xusheng <zhanxusheng1024@gmail.com>

On 2026/6/23, Gao Xiang wrote:
> In short, I saw some similar report from LLMs, but I think
> erofs_shrinker_unregister() should block this from kfree(sbi)
> by design.

Right, that's the part I'd missed: erofs_shrinker_unregister() drains
sbi->managed_pslots (while (!xa_empty(...)) z_erofs_shrink_scan()) in
->put_super before erofs_sb_free(), so the in-flight pcluster keeps sbi
pinned across z_erofs_put_pcluster(). Thanks for the clarification.

  Zhan Xusheng

Re: erofs: is z_erofs_put_pcluster()'s sbi access in the same UAF window as 1aee05e814d2?

[Gao Xiang]

On 2026/6/23 11:38, Zhan Xusheng wrote:
> From: Zhan Xusheng <zhanxusheng1024@gmail.com>
> 
> On 2026/6/23, Gao Xiang wrote:
>> In short, I saw some similar report from LLMs, but I think
>> erofs_shrinker_unregister() should block this from kfree(sbi)
>> by design.
> 
> Right, that's the part I'd missed: erofs_shrinker_unregister() drains
> sbi->managed_pslots (while (!xa_empty(...)) z_erofs_shrink_scan()) in
> ->put_super before erofs_sb_free(), so the in-flight pcluster keeps sbi
> pinned across z_erofs_put_pcluster(). Thanks for the clarification.

Yes, that is what I meant, also I found usually LLMs could
miss some details by chance, so IMHO we'd better to check
manually again.

Thanks,
Gao Xiang

> 
>    Zhan Xusheng