mm/shrinker: fix bogus set_shrinker_bit() with cgroup.memory=nokmem

With cgroup.memory=nokmem, shrinker_memcg_alloc() bails out early and never allocates an id, so shrinker->id keeps the 0 it got from the kzalloc() in shrinker_alloc(). __list_lru_init() then copies that 0 into lru->shrinker_id, where it looks like a valid bit index.

Nothing calls expand_shrinker_info() on nokmem either, so shrinker_nr_max stays 0 and every memcg ends up with an empty map (map_nr_max == 0).

deferred_split_folio() hands a real memcg to __list_lru_add() regardless of whether the lru is memcg aware, so the first THP queued in a cgroup does set_shrinker_bit(memcg, nid, 0) and trips the bounds check:

WARNING: mm/shrinker.c:212 at set_shrinker_bit+0x7d/0x90, CPU#126
Call Trace:
 <TASK>
 deferred_split_folio+0x18c/0x220
 map_anon_folio_pmd_nopf+0xdd/0x130
 map_anon_folio_pmd_pf+0x14/0xb0
 do_huge_pmd_anonymous_page+0x1a1/0x620
 __handle_mm_fault+0xea9/0x10d0
 handle_mm_fault+0xe5/0x320
 do_user_addr_fault+0x1cc/0x870
 exc_page_fault+0x81/0x1b0
 asm_exc_page_fault+0x27/0x30
 </TASK>

Harmless, the WARN_ON_ONCE() is what keeps the out of bounds unit[] read from happening, but the id should not look valid in the first place. Clear it before returning.

Two other spots could paper over this: drop the id in __list_lru_init() when nokmem turns memcg_aware off, or make deferred_split_folio() pass NULL like list_lru_add_obj() does. Both leave shrinker->id lying around for the next caller, so fix it where the id is handed out.

Fixes fafaeceb89a5 ("mm: switch deferred split shrinker to list_lru")

Signed-off-by Jiayuan Chen <[email protected]>

---
 mm/shrinker.c | 2 ++
 1 file changed, 2 insertions(+)

mm/shrinker.c +0 -0

index a70aab124a0e..7ec2a9704f6f 100644

Quoted 49 lines · Jiayuan Chen

With cgroup.memory=nokmem, shrinker_memcg_alloc() bails out early and never allocates an id, so shrinker->id keeps the 0 it got from the kzalloc() in shrinker_alloc(). __list_lru_init() then copies that 0 into lru->shrinker_id, where it looks like a valid bit index.

Nothing calls expand_shrinker_info() on nokmem either, so shrinker_nr_max stays 0 and every memcg ends up with an empty map (map_nr_max == 0).

deferred_split_folio() hands a real memcg to __list_lru_add() regardless of whether the lru is memcg aware, so the first THP queued in a cgroup does set_shrinker_bit(memcg, nid, 0) and trips the bounds check:

WARNING: mm/shrinker.c:212 at set_shrinker_bit+0x7d/0x90, CPU#126
Call Trace:
  <TASK>
  deferred_split_folio+0x18c/0x220
  map_anon_folio_pmd_nopf+0xdd/0x130
  map_anon_folio_pmd_pf+0x14/0xb0
  do_huge_pmd_anonymous_page+0x1a1/0x620
  __handle_mm_fault+0xea9/0x10d0
  handle_mm_fault+0xe5/0x320
  do_user_addr_fault+0x1cc/0x870
  exc_page_fault+0x81/0x1b0
  asm_exc_page_fault+0x27/0x30
  </TASK>

Harmless, the WARN_ON_ONCE() is what keeps the out of bounds unit[] read from happening, but the id should not look valid in the first place. Clear it before returning.

Two other spots could paper over this: drop the id in __list_lru_init() when nokmem turns memcg_aware off, or make deferred_split_folio() pass NULL like list_lru_add_obj() does. Both leave shrinker->id lying around for the next caller, so fix it where the id is handed out.

Fixes fafaeceb89a5 ("mm: switch deferred split shrinker to list_lru")

Signed-off-by Jiayuan Chen <[email protected]>

---
  mm/shrinker.c | 2 ++
  1 file changed, 2 insertions(+)

mm/shrinker.c +0 -0

index a70aab124a0e..7ec2a9704f6f 100644

mm/shrinker.c static int shrinker_memcg_alloc(struct shrinker *shrinker) · line 234

Quoted 8 lines

mm/shrinker.c +2 -0

@@ -227,6 +227,8 @@ static int shrinker_memcg_alloc(struct shrinker *shrinker)
  {
  	int id;
 
+	shrinker->id = -1;
+
  	if (mem_cgroup_disabled())
  		return -ENOSYS;
  	if (mem_cgroup_kmem_disabled() && !(shrinker->flags & SHRINKER_NONSLAB))

Thanks for the fix!

I think the initialization would look better in shrinker_alloc()?

With cgroup.memory=nokmem, shrinker_memcg_alloc() bails out early and never allocates an id, so shrinker->id keeps the 0 it got from the kzalloc() in shrinker_alloc(). __list_lru_init() then copies that 0 into lru->shrinker_id, where it looks like a valid bit index.

Nothing calls expand_shrinker_info() on nokmem either, so shrinker_nr_max stays 0 and every memcg ends up with an empty map (map_nr_max == 0).

deferred_split_folio() hands a real memcg to __list_lru_add() regardless of whether the lru is memcg aware, so the first THP queued in a cgroup does set_shrinker_bit(memcg, nid, 0) and trips the bounds check:

WARNING: mm/shrinker.c:212 at set_shrinker_bit+0x7d/0x90, CPU#126
Call Trace:
 <TASK>
 deferred_split_folio+0x18c/0x220
 map_anon_folio_pmd_nopf+0xdd/0x130
 map_anon_folio_pmd_pf+0x14/0xb0
 do_huge_pmd_anonymous_page+0x1a1/0x620
 __handle_mm_fault+0xea9/0x10d0
 handle_mm_fault+0xe5/0x320
 do_user_addr_fault+0x1cc/0x870
 exc_page_fault+0x81/0x1b0
 asm_exc_page_fault+0x27/0x30
 </TASK>

Harmless, the WARN_ON_ONCE() is what keeps the out of bounds unit[] read from happening, but the id should not look valid in the first place. Clear it before returning.

Two other spots could paper over this: drop the id in __list_lru_init() when nokmem turns memcg_aware off, or make deferred_split_folio() pass NULL like list_lru_add_obj() does. Both leave shrinker->id lying around for the next caller, so fix it where the id is handed out.

Fixes fafaeceb89a5 ("mm: switch deferred split shrinker to list_lru")

Signed-off-by Jiayuan Chen <[email protected]>