Re: [PATCH v2] mm: memcg: initialize *locked in memcg1_oom_prepare() stub
Johannes Weiner <[email protected]>
| Newsgroups | org.kernel.vger.cgroups,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
On Thu, Jul 16, 2026 at 05:39:49PM -0700, Andrew Morton wrote: > On Thu, 16 Jul 2026 06:42:18 -0700 Breno Leitao <[email protected]> wrote: > > > mem_cgroup_oom() passes an uninitialized "locked" to memcg1_oom_prepare() > > and reads it back in memcg1_oom_finish(): > > > > bool locked, ret; > > ... > > if (!memcg1_oom_prepare(memcg, &locked)) > > return false; > > ret = mem_cgroup_out_of_memory(memcg, mask, order); > > memcg1_oom_finish(memcg, locked); > > > > This relies on memcg1_oom_prepare() setting *locked whenever it returns > > true. The CONFIG_MEMCG_V1=y version does, but the stub used when > > CONFIG_MEMCG_V1=n returns true without touching *locked, so > > memcg1_oom_finish() consumes an uninitialized value. On a memcg OOM this > > is reported by UBSAN: > > > > UBSAN: invalid-load in mm/memcontrol.c:1932:27 > > load of value 0 is not a valid value for type 'bool' (aka '_Bool') > > > > Initialize *locked to false in the stub; with cgroup v1 compiled out > > there is no OOM lock to take. > > Thanks. > > Sashiko, as is its wont, reminds us that we all suck: > https://sashiko.dev/#/patchset/[email protected] > > Does this potential memcg issue look legit? Yes, it looks legit to me. The task is meant to be put to sleep after this until current->memcg_in_oom is cleared. But sleep-and-clear happens on userspace resume and there can be multiple allocs before. Is it worth fixing? It's a deprecated feature inside a deprecated feature (6df4ad704707 ("memcg: initiate deprecation of oom_control"), August 2024). Maybe we just ought to delete all this...