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

On 2026/6/23 10:52, Gao Xiang wrote:
> 
> 
> On 2026/6/23 10:49, Zhan Xusheng wrote:
>> From: Zhan Xusheng <[email protected]>
>>
>> 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
>