[PATCH v2] mm/memcontrol: publish null pointer before dropping css reference

[PATCH v2] mm/memcontrol: publish null pointer before dropping css reference

[Joshua Hahn]

struct memcg_stock_pcp hold references to cgroups that it tracks to make
sure that the memcg never gets freed with unaccounted charges still
living in the stock. When the cached stock goes to 0 for a memcg, its
reference is dropped and the cached memcg pointer is set to NULL.

If that was the last reference, the css_put would free the associated
cgroup before publishing the NULL pointer, which can be racy on 
PREEMPT_RT. To summarize Shakeel Butt's analysis in [2],

CPU 1					CPU 2
local_trylock() takes a per-CPU
spinlock, which is an rcu read
lock along with migrate_disable

drain_stock
css_put drops the last reference
release work is queued

kworker runs css_release_work_fn()
call_rcu() starts grace period G
which waits for the refill task.

<preempt>
					drain_all_stock
					is_memcg_drain_needed()
					enters rcu_read_lock(), but
					 grace period G does not wait
					since it started after G did

					<prempt>

stock->cached[i] is cleared and
unlocks. Grace period G ends,
old is freed

					mem_cgroup_is_descendant() reads
					old->css.cgroup which has just
					been freed.

To close this gap on PREEMPT_RT, switch the order of the referene drop
and the publishing so that we publish the NULL pointer before dropping
the reference. Note that this can only happen from drain_stock who can
hold the last reference, but not on consume_stock since another
reference will always be held. But fix this in both sites to maintain
consistency. 

This is based on a report from Sashiko, who found this pre-existing
issue in my page_counter_stock series [1].

[1] https://sashiko.dev/#/patchset/20260928192349.3432886-1-joshua.hahnjy%40gmail.com
[2] https://lore.kernel.org/all/atfMngq7nyZpt4u-@linux.dev/

Fixes: e1a366be5cb4 ("mm: memcontrol: switch to rcu protection in drain_all_stock()")
Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
---
v1 --> v2
- Rewrote cover letter for clarity on what the actual condition to hit
  the race condition, which Shakeel Butt provided. 

 mm/memcontrol.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index e7d7db5718f9a..a05d9a8baa1b6 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2172,8 +2172,8 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 			stock_pages -= nr_pages;
 			WRITE_ONCE(stock->nr_pages[i], stock_pages);
 			if (!stock_pages) {
-				css_put(&memcg->css);
 				WRITE_ONCE(stock->cached[i], NULL);
+				css_put(&memcg->css);
 			}
 			ret = true;
 		}
@@ -2209,8 +2209,8 @@ static void drain_stock(struct memcg_stock_pcp *stock, int i)
 		WRITE_ONCE(stock->nr_pages[i], 0);
 	}
 
-	css_put(&old->css);
 	WRITE_ONCE(stock->cached[i], NULL);
+	css_put(&old->css);
 }
 
 static void drain_stock_fully(struct memcg_stock_pcp *stock)
-- 
2.53.0-Meta

Re: [PATCH v2] mm/memcontrol: publish null pointer before dropping css reference

[Shakeel Butt]

On Tue, Oct 20, 2026 at 06:00:50PM -0700, Joshua Hahn wrote:
> struct memcg_stock_pcp hold references to cgroups that it tracks to make
> sure that the memcg never gets freed with unaccounted charges still
> living in the stock. When the cached stock goes to 0 for a memcg, its
> reference is dropped and the cached memcg pointer is set to NULL.
> 
> If that was the last reference, the css_put would free the associated
> cgroup before publishing the NULL pointer, which can be racy on 
> PREEMPT_RT. To summarize Shakeel Butt's analysis in [2],
> 
> CPU 1					CPU 2
> local_trylock() takes a per-CPU
> spinlock, which is an rcu read
> lock along with migrate_disable
> 
> drain_stock
> css_put drops the last reference
> release work is queued
> 
> kworker runs css_release_work_fn()
> call_rcu() starts grace period G
> which waits for the refill task.
> 
> <preempt>
> 					drain_all_stock
> 					is_memcg_drain_needed()
> 					enters rcu_read_lock(), but
> 					 grace period G does not wait
> 					since it started after G did
> 
> 					<prempt>
> 
> stock->cached[i] is cleared and
> unlocks. Grace period G ends,
> old is freed
> 
> 					mem_cgroup_is_descendant() reads
> 					old->css.cgroup which has just
> 					been freed.
> 
> To close this gap on PREEMPT_RT, switch the order of the referene drop
> and the publishing so that we publish the NULL pointer before dropping
> the reference. Note that this can only happen from drain_stock who can
> hold the last reference, but not on consume_stock since another
> reference will always be held. But fix this in both sites to maintain
> consistency. 
> 
> This is based on a report from Sashiko, who found this pre-existing
> issue in my page_counter_stock series [1].
> 
> [1] https://sashiko.dev/#/patchset/20260928192349.3432886-1-joshua.hahnjy%40gmail.com
> [2] https://lore.kernel.org/all/atfMngq7nyZpt4u-@linux.dev/
> 
> Fixes: e1a366be5cb4 ("mm: memcontrol: switch to rcu protection in drain_all_stock()")
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>

Re: [PATCH v2] mm/memcontrol: publish null pointer before dropping css reference

[Muchun Song]

> On Oct 21, 2026, at 09:00, Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> 
> struct memcg_stock_pcp hold references to cgroups that it tracks to make
> sure that the memcg never gets freed with unaccounted charges still
> living in the stock. When the cached stock goes to 0 for a memcg, its
> reference is dropped and the cached memcg pointer is set to NULL.
> 
> If that was the last reference, the css_put would free the associated
> cgroup before publishing the NULL pointer, which can be racy on 
> PREEMPT_RT. To summarize Shakeel Butt's analysis in [2],
> 
> CPU 1 CPU 2
> local_trylock() takes a per-CPU
> spinlock, which is an rcu read
> lock along with migrate_disable
> 
> drain_stock
> css_put drops the last reference
> release work is queued
> 
> kworker runs css_release_work_fn()
> call_rcu() starts grace period G
> which waits for the refill task.
> 
> <preempt>
> drain_all_stock
> is_memcg_drain_needed()
> enters rcu_read_lock(), but
> grace period G does not wait
> since it started after G did
> 
> <prempt>
> 
> stock->cached[i] is cleared and
> unlocks. Grace period G ends,
> old is freed
> 
> mem_cgroup_is_descendant() reads
> old->css.cgroup which has just
> been freed.
> 
> To close this gap on PREEMPT_RT, switch the order of the referene drop
> and the publishing so that we publish the NULL pointer before dropping
> the reference. Note that this can only happen from drain_stock who can
> hold the last reference, but not on consume_stock since another
> reference will always be held. But fix this in both sites to maintain
> consistency. 
> 
> This is based on a report from Sashiko, who found this pre-existing
> issue in my page_counter_stock series [1].
> 
> [1] https://sashiko.dev/#/patchset/20260928192349.3432886-1-joshua.hahnjy%40gmail.com
> [2] https://lore.kernel.org/all/atfMngq7nyZpt4u-@linux.dev/
> 
> Fixes: e1a366be5cb4 ("mm: memcontrol: switch to rcu protection in drain_all_stock()")
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>

Acked-by: Muchun Song <muchun.song@linux.dev>

Thanks.