Re: [PATCH] mm/memcg: fix NULL nodeinfo[] dereference on late-onlined nodes

[Muchun Song <muchun.song@linux.dev>]

> On Aug 11, 2026, at 13:16, Prakash Gupta <prakash.gupta@oss.qualcomm.com> wrote:
> 
> 
> 
> On 8/10/2026 1:28 PM, Muchun Song wrote:
>> 
>> 
>>> On Aug 10, 2026, at 13:46, Prakash Gupta <prakash.gupta@oss.qualcomm.com> wrote:
>>> 
>>> memcg->nodeinfo[] entries are allocated only for nodes present at
>>> css_alloc time. When a node is onlined after a memcg is created its
>>> nodeinfo[] slot remains NULL. Two call sites dereference these slots
>>> unconditionally:
>> 
>> I don't think the premise of this patch is correct.
>> 
>> memcg->nodeinfo[] is not allocated only for nodes that are present or
>> online at css_alloc time. mem_cgroup_alloc() allocates per-node info with
>> for_each_node(), and for_each_node() iterates N_POSSIBLE nodes:
>> 
>>        for_each_node(node)
>>                alloc_mem_cgroup_per_node_info(memcg, node);
>> 
> 
> You are right. I rechecked and nodeinfo[] is allocated for all possible
> nodes at mem_cgroup_alloc() time, not just currently online ones.
> 
>> If memcg->nodeinfo[nid] is NULL on your system, that looks like a
>> violation of this invariant, or possibly a downstream-specific change,
>> bad nid, allocation/lifetime issue, or memory corruption. I don't think
>> the generic explanation that "the node was onlined after the memcg was
>> created" is sufficient.
>> 
> Agreed. I will investigate further before resubmitting that part.
> 
>>> 
>>> lruvec_stat_mod_folio() calls mem_cgroup_lruvec() which reads
>>> memcg->nodeinfo[pgdat->node_id] without a NULL check. On a system
>>> where a node is onlined after the memcg is created, any folio stat
>>> update for that node crashes with a NULL pointer dereference:
>>> 
>>> Unable to handle kernel paging request at virtual address ffffffbebf7e1908
>>> pc : lruvec_stat_mod_folio+0xf0/0x444   [6.18.21-android17-5]
>>> lr : lruvec_stat_mod_folio+0x88/0x444
>>> Call trace:
>>>  lruvec_stat_mod_folio+0xf0/0x444
>>>  folio_add_new_anon_rmap+0xac/0x2b8
>>>  do_wp_page+0x768/0xc80
>>>  handle_mm_fault+0x37c/0x8c4
>>>  do_page_fault+0x140/0xa1c
>>>  do_mem_abort+0x54/0x74
>>>  el0_da+0x48/0x8c
>>>  el0t_64_sync_handler+0x20/0x130
>>>  el0t_64_sync+0x1c4/0x1c8
>>> 
>>> __invalidate_reclaim_iterators() iterates for_each_node() and reads
>>> from->nodeinfo[nid]->iter without checking for NULL. for_each_node()
>>> visits all possible nodes, so this is reachable whenever a node is
>>> onlined after the memcg was created.
>>> 
>>> Fix lruvec_stat_mod_folio() by checking nodeinfo[pgdat->node_id]
>>> directly and falling back to mod_node_page_state() when NULL, mirroring
>>> the existing !memcg early-return path. The fallback must not go through
>>> mod_lruvec_state() since that calls mod_memcg_lruvec_state() which uses
>>> container_of() to recover the mem_cgroup_per_node from the lruvec
>>> pointer; passing &pgdat->__lruvec there produces a garbage pointer.
>>> When nodeinfo[nid] is NULL the memcg has no per-node accounting
>>> structure for that node, so node-level accounting is correct.
>>> 
>>> Fix __invalidate_reclaim_iterators() by skipping NULL nodeinfo[] slots.
>>> 
>>> Also switch lruvec_stat_mod_folio() to folio_memcg_check() which uses
>>> READ_ONCE() to safely read folio->memcg_data in an unlocked context.
>>> 
>>> Fixes: 6c77b607ee26 ("mm: kill lock|unlock_page_memcg()")
>> 
>> The Fixes tag also seems odd. 6c77b607ee26 ("mm: kill
>> lock|unlock_page_memcg()") only removed/renamed the lock_page_memcg()
>> wrappers and does not appear to change nodeinfo[] allocation or memory
>> hotplug handling. Could you explain how that commit introduced the NULL
>> nodeinfo condition?
>> 
> 
> It did not. It seems I picked up the commit based on git blame on
> function as there were two related bugs reports that I was conflating
> into one patch as listed below, will fix that in v2.
> 
> Variant A (missed to mentioned in commit msg) — NULL dereference:
> 
>    Unable to handle kernel NULL pointer dereference
>      at virtual address 0000000000000528
>    ESR = 0x0000000096000005 (read fault)
>    Workqueue: events delayed_fput
>    pc : lruvec_stat_mod_folio+0x5c/0x444  [6.18.21-android17-5]
>    lr : lruvec_stat_mod_folio+0x2c/0x444
> 
> As you suggested this may need more investigation.
> 
> Variant B (the crash included in the patch) — stale pointer:
> 
>    Unable to handle kernel paging request
>      at virtual address ffffffbebf7e1908
>    ESR = 0x0000000096000045 (write fault)
>    pc : __lruvec_stat_mod_folio+0xf0/0x444  [6.18.21-android17-5]
>    lr : __lruvec_stat_mod_folio+0x88/0x444
>    Call trace:
>      __lruvec_stat_mod_folio+0xf0/0x444
>      folio_add_new_anon_rmap+0xac/0x2b8
>      do_wp_page+0x768/0xc80
>      handle_mm_fault+0x37c/0x8c4
>      do_page_fault+0x140/0xa1c
> 
> __lruvec_stat_mod_folio+0xf0 places it after the !memcg NULL check.This
> is consistent with folio_memcg() returning a stale memcg pointer that
> passes the NULL check.
> 
> folio_memcg_check() uses READ_ONCE(folio->memcg_data) which seems the
> correct API for unlocked contexts. folio_memcg_check() has been
> available since becacb04fdd4 ("mm: memcg: add folio_memcg_check()")
> but lruvec_stat_mod_folio() was never updated to use it.
> 
> I should also note that this crash is difficult to reproduce in a
> controlled environment — the analysis is based on crashdump inspection.
> I have not been able to construct a reliable reproducer so far.
> 
> let me know your thoughts on this, accordingly I can send a v2 to cover
> only Variant B fix as explained above.

I just want to point out that the Fixes tag you listed here, regardless
of whether this patch is correct, shouldn't be a commit that introduces
a bug. Because that commit is purely a code cleanup and the removal of
redundant interfaces, theoretically it shouldn't have any impact at all,
yet you cited it anyway, which doesn't make sense. It feels to me like
you didn't carefully check this.

Muchun,
Thanks.

> 
>>> Cc: stable@vger.kernel.org
>>> Assisted-by: pi:claude-sonnet-4-5
>> 
>> Given the Assisted-by tag, I assume some of the analysis may have been
>> tool-assisted. That's fine, but the author still needs to validate the
>> reasoning against the actual code before submission.
>> 
> Agree, I will be more careful in next submission.
> 
>> Did you confirm that the relevant allocation and hotplug paths were
>> manually checked against the affected tree? In particular, I wonder
>> whether this behavior depends on downstream changes around
>> mem_cgroup_alloc(), for_each_node(), node_possible_map setup, or memory
>> hotplug nid validation.
>> 
> I checked all four paths against the affected tree
> (6.18.21-android17-5). Only difference in
> mem_cgroup_alloc() are cosmetic and do not touch the nodeinfo[]
> allocation path. The NULL nodeinfo[nid] in Variant A is not explained
> by any downstream change — the root cause remain unknown.
> 
> Thank you for the review.
> 
> Thanks,
> Prakash