Re: [PATCH v5 9/9] mm/page_owner: use memcg_data snapshot instead of PageMemcgKmem() to avoid TOCTOU VM_BUG_ON

[Ye Liu]

在 2026/7/2 10:02, Zi Yan 写道:
> On 1 Jul 2026, at 2:10, Ye Liu wrote:
> 
>> print_page_owner_memcg() takes a snapshot of page->memcg_data via
>> READ_ONCE at the top of the function and guards against tail pages
>> and NULL memcg_data.  However, at the end it calls PageMemcgKmem(page)
>> which internally calls folio_memcg_kmem() — and that function re-reads
>> folio->memcg_data and page->compound_head locklessly, wrapping both
>> in VM_BUG_ON assertions:
>>
>>     VM_BUG_ON_PGFLAGS(PageTail(&folio->page), &folio->page);
>>     VM_BUG_ON_FOLIO(folio->memcg_data & MEMCG_DATA_OBJEXTS, folio);
>>
>> If the page is concurrently freed and reallocated as a THP tail page
>> or a slab page between the initial guards and this final call, the
>> VM_BUG_ON assertions can fire on debug builds (CONFIG_DEBUG_VM=y),
>> causing a kernel panic.
>>
>> Fix by reusing the memcg_data snapshot already taken at function entry
>> instead of calling PageMemcgKmem(), which is semantically equivalent:
>> PageMemcgKmem()->folio_memcg_kmem()->folio->memcg_data & MEMCG_DATA_KMEM.
>> This avoids both the TOCTOU window and the assertions entirely.
>>
>> Signed-off-by: Ye Liu <ye.liu@xxxxxxxxx>
>> ---
>>  mm/page_owner.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
> LGTM.
> 
> Reviewed-by: Zi Yan <ziy@xxxxxxxxxx>Hi,Zi,Vlastimil                                                                                  
                                                                                                 
This patch still has warnings (see sashiko link[1]).                                             
I've made the following modifications, or do you have any better suggestions?
                  
[1]:https://sashiko.dev/#/patchset/20260701061101.344679-1-ye.liu@xxxxxxxxx                      
                                                                                                 
diff --git a/mm/page_owner.c b/mm/page_owner.c                                                   
index 2e3880053a34..e18512a49e38 100644                                                          
--- a/mm/page_owner.c                                                                            
+++ b/mm/page_owner.c                                                                            
@@ -540,6 +540,7 @@ static inline int print_page_owner_memcg(char *kbuf, size_t count, int ret,  
                                         struct page *page)                                      
 {                                                                                               
        unsigned long memcg_data;                                                                
+       struct obj_cgroup *objcg;                                                                
        struct mem_cgroup *memcg;                                                                
        bool online;                                                                             
        char name[80];                                                                           
@@ -549,11 +550,14 @@ static inline int print_page_owner_memcg(char *kbuf, size_t count, int ret,
        if (!memcg_data || PageTail(page))                                                       
                goto out_unlock;                                                                 
                                                                                                 
-       if (memcg_data & MEMCG_DATA_OBJEXTS)                                                     
+       if (memcg_data & MEMCG_DATA_OBJEXTS) {                                                   
                ret += scnprintf(kbuf + ret, count - ret,                                        
                                "Slab cache page\n");                                            
+               goto out_unlock;                                                                 
+       }                                                                                        
                                                                                                 
-       memcg = page_memcg_check(page);                                                          
+       objcg = (void *)(memcg_data & ~OBJEXTS_FLAGS_MASK);                                      
+       memcg = objcg ? obj_cgroup_memcg(objcg) : NULL;                                          
        if (!memcg)                                                                              
                goto out_unlock;                                                                 
                                                                                                 
@@ -561,7 +565,7 @@ static inline int print_page_owner_memcg(char *kbuf, size_t count, int ret,  
        cgroup_name(memcg->css.cgroup, name, sizeof(name));                                      
        ret += scnprintf(kbuf + ret, count - ret,                                                
                        "Charged %sto %smemcg %s\n",                                             
-                       PageMemcgKmem(page) ? "(via objcg) " : "",                               
+                       (memcg_data & MEMCG_DATA_KMEM) ? "(via objcg) " : "",                    
                        online ? "" : "offline ",                                                
                        name);                                                                   
 out_unlock:                                                                                     

> 
> Best Regards,
> Yan, Zi

-- 
Thanks,
Ye Liu