All of lore.kernel.org
 help / color / mirror / Atom feed
From: Muchun Song <muchun.song@linux.dev>
To: "T.J. Mercier" <tjmercier@google.com>
Cc: hannes@cmpxchg.org, mhocko@kernel.org, roman.gushchin@linux.dev,
	shakeel.butt@linux.dev, zhengqi.arch@bytedance.com,
	akpm@linux-foundation.org, cgroups@vger.kernel.org,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH] mm: memcontrol: fix NULL pointer deref in high_work_func() on dying memcg
Date: Wed, 28 Oct 2026 10:45:11 +0800	[thread overview]
Message-ID: <47089233-E973-4F25-B64F-DBBBE0F31FC0@linux.dev> (raw)
In-Reply-To: <CABdmKX3DEDwgO_wj3JDYvx15FYPKGCav8HKvHC-u_EHdGhWwLg@mail.gmail.com>



> On Oct 28, 2026, at 07:11, T.J. Mercier <tjmercier@google.com> wrote:
> 
> On Tue, Oct 27, 2026 at 10:42 AM T.J. Mercier <tjmercier@google.com> wrote:
>> 
>> On Mon, Oct 26, 2026 at 7:42 PM Muchun Song <muchun.song@linux.dev> wrote:
>>> 
>>> 
>>> 
>>>> On Oct 24, 2026, at 06:23, T.J. Mercier <tjmercier@google.com> wrote:
>>>> 
>>>> When memory.high is exceeded in non-task context, try_charge_memcg()
>>>> schedules memcg->high_work without taking a CSS reference on the memcg,
>>>> and high_work_func() invokes reclaim_high() without pinning the memcg.
>>>> 
>>>> Before commit f1cf8d2f36dc ("mm: memcontrol: eliminate the problem of
>>>> dying memory cgroup for LRU folios"), memory usage above memory.high
>>>> kept the memcg's refcount non-zero. With reparenting, folios and objcgs
>>>> charged to a child memcg are reparented to its parent in
>>>> mem_cgroup_css_offline(), leaving the offlined child's memory usage
>>>> permanently above its memory.high even after its last reference is
>>>> dropped.
>>>> 
>>>> cancel_work_sync(&memcg->high_work) only runs at the end of teardown in
>>>> mem_cgroup_css_free(), so high_work_func() can start or still be running
>>>> reclaim_high() after mem_cgroup_css_released() or shrink_many() has
>>>> called lru_gen_release_memcg() and unhashed lruvec->lrugen.list. This
>>>> causes lru_gen_rotate_memcg() to dereference a NULL pprev pointer.
>>>> 
>>>> Fix this by acquiring a reference with css_tryget_online() in
>>>> high_work_func() before calling reclaim_high(), and by resetting
>>>> memory.high and swap.high to PAGE_COUNTER_MAX in
>>>> mem_cgroup_css_offline() so that charges via references on an offlined
>>>> memcg do not trigger high_work on the dead child or skip checking live
>>>> ancestors in try_charge_memcg().
>>>> 
>>>> Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP KASAN NOPTI
>>>> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
>>>> CPU: 1 UID: 0 PID: 27 Comm: kworker/1:0 Tainted: G    B   W           7.3.0-rc5-g2bba28e98578 #284 PREEMPT(full) <- Taint from lru_gen_rotate_memcg() VM_WARN_ON_ONCE
>>>> Tainted: [B]=BAD_PAGE, [W]=WARN
>>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
>>>> Workqueue: events high_work_func
>>>> RIP: 0010:lru_gen_rotate_memcg+0x1a3/0xe90
>>>> Code: fd 7f 09 45 84 ed 0f 85 64 0c 00 00 48 89 cf c6 83 6a 07 00 00 00 48 c1 ef 03 88 83 69 07 00 00 48 b8 00 00 00 00 00 fc ff df <80> 3c 07 00 0f 85 ec 0b 00 00 48 89 31 40 f6 c6 01 0f 85 4e 00
>>>> RSP: 0018:ffa00000001ef4c8 EFLAGS: 00010046
>>>> RAX: dffffc0000000000 RBX: ff1100000779e440 RCX: 0000000000000000
>>>> RDX: 0000000000000001 RSI: ff1100000779b9b0 RDI: 0000000000000000
>>>> RBP: ff110000bffd8e40 R08: 0000000000000000 R09: ff1100000779ebb8
>>>> R10: ff1100000779ebb0 R11: 0000000000000246 R12: 0000000000000000
>>>> R13: 0000000000000000 R14: ffa00000001efb30 R15: 0000000000000002
>>>> FS:  0000000000000000(0000) GS:ff1100011c0e7000(0000) knlGS:0000000000000000
>>>> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> CR2: 000000000251ac88 CR3: 0000000009d6c000 CR4: 0000000000751ef0
>>>> PKRU: 55555554
>>>> Call Trace:
>>>> <TASK>
>>>> ? read_ctrl_pos+0x22f/0x3f0
>>>> shrink_lruvec+0x16cb/0x1b80
>>>> ? ret_from_fork_asm+0x1a/0x30
>>>> ? lapic_next_event+0x15/0x20
>>>> ? clockevents_program_event+0x2bb/0x6f0
>>>> ? lapic_next_event+0x15/0x20
>>>> ? __pfx_shrink_lruvec+0x10/0x10
>>>> ? __pfx_clockevents_program_event+0x10/0x10
>>>> ? _raw_spin_lock+0x84/0xe0
>>>> ? __pfx__raw_spin_lock+0x10/0x10
>>>> ? kasan_save_stack+0x30/0x50
>>>> ? psi_group_change+0x381/0x740
>>>> ? __pfx_sched_clock_cpu+0x10/0x10
>>>> ? save_fpregs_to_fpstate+0xc4/0x2c0
>>>> ? finish_task_switch.isra.0+0x1de/0xac0
>>>> ? __schedule+0xf62/0x42c0
>>>> ? __pfx___schedule+0x10/0x10
>>>> ? __pfx___irq_work_queue_local+0x10/0x10
>>>> ? shrink_node+0x626/0x31e0
>>>> shrink_node+0x626/0x31e0
>>>> ? ring_buffer_unlock_commit+0x11d/0x5c0
>>>> ? kswapd_clear_hopeless+0xa0/0x150
>>>> ? __pfx_shrink_node+0x10/0x10
>>>> ? 0xffffffffc0000095
>>>> do_try_to_free_pages+0x255/0x13b0
>>>> ? ring_buffer_unlock_commit+0x11d/0x5c0
>>>> ? reclaim_high+0x10a/0x1b0
>>>> try_to_free_mem_cgroup_pages+0x1f9/0x5a0
>>>> ? __pfx_try_to_free_mem_cgroup_pages+0x10/0x10
>>>> ? irqentry_exit+0xc6/0x670
>>>> reclaim_high+0x10a/0x1b0
>>>> ? __pfx_reclaim_high+0x10/0x10
>>>> ? process_one_work+0x5ff/0x1030
>>>> ? __pfx_high_work_func+0x10/0x10
>>>> process_one_work+0x6e2/0x1030
>>>> ? __pfx___schedule+0x10/0x10
>>>> ? __pfx_process_one_work+0x10/0x10
>>>> worker_thread+0x422/0xbb0
>>>> ? __pfx_worker_thread+0x10/0x10
>>>> kthread+0x2cd/0x3a0
>>>> ? recalc_sigpending+0x15c/0x200
>>>> ? __pfx_kthread+0x10/0x10
>>>> ret_from_fork+0x368/0x590
>>>> ? __pfx_ret_from_fork+0x10/0x10
>>>> ? __switch_to+0x588/0xf50
>>>> ? __pfx_kthread+0x10/0x10
>>>> ret_from_fork_asm+0x1a/0x30
>>>> </TASK>
>>>> 
>>>> Fixes: f1cf8d2f36dc ("mm: memcontrol: eliminate the problem of dying memory cgroup for LRU folios")
>>> 
>>> Hi T.J.,
>>> 
>>> Thanks for fixing this.
>>> 
>>> I have one question about the Fixes tag.
>>> 
>>> The changelog says that, before f1cf8d2f36dc, memory usage above
>>> memory.high kept the memcg reference count non-zero. This is true for
>>> LRU folios: before that commit, each charged LRU folio held a CSS
>>> reference until it was uncharged.
>>> 
>>> However, I wonder whether objcg-based kmem already provided a reachable
>>> path before f1cf8d2f36dc.
>>> 
>>> In particular, BPF alloc_bulk() mostly runs from irq_work. It obtains a
>>> temporary memcg reference from a persistent objcg, uses set_active_memcg()
>>> while performing __GFP_ACCOUNT allocations, and drops the temporary
>>> reference after the allocation. The charge can therefore queue
>>> memcg->high_work in non-task context without transferring a CSS
>>> reference to the work item.
>>> 
>>> If this races with memcg offlining, objcg reparenting can redirect the
>>> objcg to the parent while the old child's page-counter usage remains
>>> above memory.high. Once the temporary CSS reference is dropped,
>>> mem_cgroup_css_released() can call lru_gen_release_memcg() before the
>>> queued high_work runs. The worker can then enter reclaim_high() and
>>> rotate an already removed lrugen.list.
>>> 
>>> This path appears to have all the required pieces after:
>>> 
>>>  e4dde56cd208 ("mm: multi-gen LRU: per-node lru_gen_folio lists")
>>> 
>>> Commit 6867c7a33206 later changed the release operation to
>>> hlist_nulls_del_init_rcu(), which explains the exact NULL pprev seen in
>>> the reported crash. Before that, the same invalid second deletion would
>>> instead encounter LIST_POISON2.
>>> 
>>> So f1cf8d2f36dc clearly broadens the problem to ordinary LRU folios, but
>>> I am not very sure it is the first commit after which the underlying race is
>>> reachable.
>>> 
>>> Is there an additional lifetime guarantee that prevents the pre-f1cf
>>> objcg/kmem path above? If no, would the following be more appropriate?
>>> 
>>> Fixes: e4dde56cd208 ("mm: multi-gen LRU: per-node lru_gen_folio lists")
>>> 
>>> Thanks,
>>> Muchun
>> 
>> Hi Muchun,
>> 
>> Everything you said sounds correct to me. I don't think there's
>> anything preventing kmem from keeping the memcg above high while the
>> refcount hits 0, and hitting the same problem in
>> lru_gen_rotate_memcg() from lru_gen_shrink_lruvec().
>> 
>> So using Fixes: e4dde56cd208 ("mm: multi-gen LRU: per-node
>> lru_gen_folio lists") sounds good to me.
>> 
>> Thanks,
>> T.J.
> 
> I was able to get a reproduction on e4dde56cd208. Fortunately this
> patch applies cleanly on e4dde56cd208, so I expect the stable
> backports to be straightforward.

Excellent job. Yeah, we need this backported.

Thanks,
Muchun

> 
> general protection fault, probably for non-canonical address
> 0xfbd59c0000000024: 0000 [#1] PREEMPT SMP KASAN NOPTI
> KASAN: maybe wild-memory-access in range [0xdead000000000120-0xdead000000000127]
> CPU: 1 PID: 37 Comm: kworker/1:1 Not tainted 6.2.0-rc4-00108-ge4dde56cd208 #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.17.0-debian-1.17.0-1 04/01/2014
> Workqueue: events high_work_func
> RIP: 0010:lru_gen_rotate_memcg+0x147/0xef0
> Code: 49 c1 ea 03 41 80 3c 3a 00 0f 85 a5 09 00 00 48 89 f7 4c 8b 93
> 08 07 00 00 48 c1 ef 03 48 89 fa 48 bf 00 00 00 00 00 fc ff df <80> 3c
> 3a 00 0f 85 2a 07 00 00 4c 89 16 41 f6 c2 01 0f 85 72 00
> RSP: 0018:ffa00000002a75e0 EFLAGS: 00010206
> RAX: ff11000008d46701 RBX: ff11000008d46000 RCX: 0000000000000000
> RDX: 1bd5a00000000024 RSI: dead000000000122 RDI: dffffc0000000000
> RBP: ff110000bfe54000 R08: ff11000008d46708 R09: ff11000008d46710
> R10: ff11000009a5a708 R11: 0000000000000000 R12: 0000000000000006
> R13: 0000000000000004 R14: 1ff4000000054ee4 R15: 0000000000000001
> FS:  0000000000000000(0000) GS:ff110000a3480000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000000 CR3: 0000000007d5c000 CR4: 0000000000751ee0
> PKRU: 55555554
> Call Trace:
> <TASK>
> ? reset_ctrl_pos+0x10f/0x460
> shrink_lruvec+0x19e0/0x2000
> ? kasan_set_track+0x25/0x30
> ? __kasan_slab_alloc+0x66/0x80
> ? kmem_cache_alloc+0x165/0x310
> ? __kernfs_new_node+0xd3/0x590
> ? __kernfs_create_file+0x2d/0x280
> ? sysfs_add_file_mode_ns+0x12b/0x3a0
> ? internal_create_group+0x286/0x940
> ? internal_create_groups+0x84/0x110
> ? really_probe+0x223/0x690
> ? unwind_next_frame+0xa9a/0x2040
> ? arch_stack_walk+0x7a/0x110
> ? unwind_next_frame+0xa9a/0x2040
> ? arch_stack_walk+0x7a/0x110
> ? __pfx_shrink_lruvec+0x10/0x10
> ? preempt_count_add+0x83/0x180
> ? unwind_next_frame+0xb2/0x2040
> ? unwind_next_frame+0x212/0x2040
> ? ret_from_fork+0x2c/0x50
> ? __pfx_stack_trace_consume_entry+0x10/0x10
> ? arch_stack_walk+0xa2/0x110
> ? sched_clock_local+0x132/0x210
> ? __pfx_sched_clock_local+0x10/0x10
> ? unwind_next_frame+0x212/0x2040
> ? __pfx_shrink_slab+0x10/0x10
> ? sched_clock_cpu+0x232/0x2c0
> ? __pfx_sched_clock_local+0x10/0x10
> ? __pfx_sched_clock_cpu+0x10/0x10
> ? mem_cgroup_iter+0x269/0x600
> ? vmpressure+0x30/0x3b0
> shrink_node+0xb39/0x26a0
> ? __pfx_shrink_node+0x10/0x10
> ? kasan_save_stack+0x42/0x60
> ? ring_buffer_lock_reserve+0x2fb/0xe30
> do_try_to_free_pages+0x31a/0x1880
> ? __pfx_do_try_to_free_pages+0x10/0x10
> try_to_free_mem_cgroup_pages+0x282/0x5f0
> ? try_to_free_mem_cgroup_pages+0x4/0x5f0
> ? reclaim_high.constprop.0+0x191/0x250
> ? __pfx_try_to_free_mem_cgroup_pages+0x10/0x10
> ? try_to_free_mem_cgroup_pages+0x9/0x5f0
> reclaim_high.constprop.0+0x191/0x250
> ? __pfx_reclaim_high.constprop.0+0x10/0x10
> ? __pfx_high_work_func+0x10/0x10
> process_one_work+0x641/0x1010
> worker_thread+0x424/0xec0
> ? __kthread_parkme+0x89/0x160
> ? __pfx_worker_thread+0x10/0x10
> kthread+0x26c/0x300
> ? __pfx_kthread+0x10/0x10
> ret_from_fork+0x2c/0x50
> </TASK>


      reply	other threads:[~2026-10-28  2:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-23 22:23 [PATCH] mm: memcontrol: fix NULL pointer deref in high_work_func() on dying memcg T.J. Mercier
2026-10-26 13:25 ` Michal Hocko
2026-10-26 18:01   ` T.J. Mercier
2026-10-27  9:40     ` Michal Hocko
2026-10-27 17:28       ` T.J. Mercier
2026-10-27 18:07         ` Shakeel Butt
2026-10-27  2:42 ` Muchun Song
2026-10-27 17:42   ` T.J. Mercier
2026-10-27 23:11     ` T.J. Mercier
2026-10-28  2:45       ` Muchun Song [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47089233-E973-4F25-B64F-DBBBE0F31FC0@linux.dev \
    --to=muchun.song@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=roman.gushchin@linux.dev \
    --cc=shakeel.butt@linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=tjmercier@google.com \
    --cc=zhengqi.arch@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.