Re: [PATCH] mm/memcg: fix NULL nodeinfo[] dereference on late-onlined nodes
From: Prakash Gupta
Date: Tue Aug 11 2026 - 01:18:39 EST
On 8/10/2026 1:28 PM, Muchun Song wrote:
>
>
>> On Aug 10, 2026, at 13:46, Prakash Gupta <prakash.gupta@xxxxxxxxxxxxxxxx> 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.
>> Cc: stable@xxxxxxxxxxxxxxx
>> 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