[PATCH] mm: memcg: initialize *locked in memcg1_oom_prepare() stub

[PATCH] mm: memcg: initialize *locked in memcg1_oom_prepare() stub

[Breno Leitao]

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.

Fixes: e93d4166b40a ("mm: memcg: put cgroup v1-specific code under a config option")
Cc: stable@vger.kernel.org
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 mm/memcontrol-v1.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/mm/memcontrol-v1.h b/mm/memcontrol-v1.h
index f92f81108d5ed..4fa6e2bc8413f 100644
--- a/mm/memcontrol-v1.h
+++ b/mm/memcontrol-v1.h
@@ -107,7 +107,11 @@ static inline void memcg1_remove_from_trees(struct mem_cgroup *memcg) {}
 static inline void memcg1_soft_limit_reset(struct mem_cgroup *memcg) {}
 static inline void memcg1_css_offline(struct mem_cgroup *memcg) {}
 
-static inline bool memcg1_oom_prepare(struct mem_cgroup *memcg, bool *locked) { return true; }
+static inline bool memcg1_oom_prepare(struct mem_cgroup *memcg, bool *locked)
+{
+	*locked = false;
+	return true;
+}
 static inline void memcg1_oom_finish(struct mem_cgroup *memcg, bool locked) {}
 static inline void memcg1_oom_recover(struct mem_cgroup *memcg) {}
 

---
base-commit: 4e5dfb7c84012007c3c7061126491bbc92d71bf1
change-id: 20260626-memcg-oom-uninit-locked-5ec79dff4396

Best regards,
-- 
Breno Leitao <leitao@debian.org>

Re: [PATCH] mm: memcg: initialize *locked in memcg1_oom_prepare() stub

[Joshua Hahn]

Hi Breno, I hope you are doing well : -)
Woah, thank you for finding and fixing this bug! 

> Fixes: e93d4166b40a ("mm: memcg: put cgroup v1-specific code under a config option")
> Cc: stable@vger.kernel.org
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  mm/memcontrol-v1.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memcontrol-v1.h b/mm/memcontrol-v1.h
> index f92f81108d5ed..4fa6e2bc8413f 100644
> --- a/mm/memcontrol-v1.h
> +++ b/mm/memcontrol-v1.h
> @@ -107,7 +107,11 @@ static inline void memcg1_remove_from_trees(struct mem_cgroup *memcg) {}
>  static inline void memcg1_soft_limit_reset(struct mem_cgroup *memcg) {}
>  static inline void memcg1_css_offline(struct mem_cgroup *memcg) {}
>  
> -static inline bool memcg1_oom_prepare(struct mem_cgroup *memcg, bool *locked) { return true; }
> +static inline bool memcg1_oom_prepare(struct mem_cgroup *memcg, bool *locked)
> +{
> +	*locked = false;
> +	return true;
> +}
>  static inline void memcg1_oom_finish(struct mem_cgroup *memcg, bool locked) {}
>  static inline void memcg1_oom_recover(struct mem_cgroup *memcg) {}

Part of me wonders if we should just initialize locked = false in the
caller (mem_cgroup_oom) as to not make the stub have side effects,
but your chnage looks correct and this is a fix so perhaps that is
not so important.

Looks good to me! Thank you again Breno : -)

Reviewed-by: Joshua Hahn <joshua.hahnjy@gmail.com>

Re: [PATCH] mm: memcg: initialize *locked in memcg1_oom_prepare() stub

[Breno Leitao]

Hello Joshua,

On Fri, Jun 26, 2026 at 06:56:11AM -0700, Joshua Hahn wrote:
> Part of me wonders if we should just initialize locked = false in the
> caller (mem_cgroup_oom) as to not make the stub have side effects,
> but your chnage looks correct and this is a fix so perhaps that is
> not so important.

Nice, your approach seems to be even better than my silly one. I will
wait for further review and respin with your approach.

Thanks,
--breno