linux-kernel - Re: [PATCH v3] slab: Add check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJuCfpGBxUmvWoe2xv2-bsF+TY4fK-m1-Z_E3OcyTiSYz5KeAA@mail.gmail.com>
Date: Tue, 14 Oct 2025 09:12:43 -0700
From: Suren Baghdasaryan <surenb@...gle.com>
To: Hao Ge <hao.ge@...ux.dev>
Cc: Vlastimil Babka <vbabka@...e.cz>, Alexei Starovoitov <ast@...nel.org>, 
	Andrew Morton <akpm@...ux-foundation.org>, Johannes Weiner <hannes@...xchg.org>, 
	Shakeel Butt <shakeel.butt@...ux.dev>, Michal Hocko <mhocko@...nel.org>, 
	Roman Gushchin <roman.gushchin@...ux.dev>, Muchun Song <muchun.song@...ux.dev>, 
	Harry Yoo <harry.yoo@...cle.com>, cgroups@...r.kernel.org, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org, Hao Ge <gehao@...inos.cn>
Subject: Re: [PATCH v3] slab: Add check for memcg_data != OBJEXTS_ALLOC_FAIL
 in folio_memcg_kmem

On Tue, Oct 14, 2025 at 8:28 AM Hao Ge <hao.ge@...ux.dev> wrote:
>
> From: Hao Ge <gehao@...inos.cn>
>
> Since OBJEXTS_ALLOC_FAIL and MEMCG_DATA_OBJEXTS currently share
> the same bit position, we cannot determine whether memcg_data still
> points to the slabobj_ext vector simply by checking
> folio->memcg_data & MEMCG_DATA_OBJEXTS.
>
> If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL,
> and during the release of the associated folio, the BUG check is triggered
> because it was mistakenly assumed that a valid folio->memcg_data
> was not cleared before freeing the folio.
>
> So let's check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem.
>
> Fixes: 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
> Suggested-by: Harry Yoo <harry.yoo@...cle.com>
> Signed-off-by: Hao Ge <gehao@...inos.cn>

Reviewed-by: Suren Baghdasaryan <surenb@...gle.com>

nit: I think it would be helpful if the changelog explained why we
need the additional check. We can have the same bit set in two
different situations:
1. object extension vector allocation failure;
2. memcg_data pointing to a valid mem_cgroup.
To distinguish between them, we need to check not only the bit itself
but also the rest of this field. If the rest is NULL, we have case 1,
otherwise case 2.

> ---
> v3: Simplify the solution, per Harry's suggestion in the v1 comments
>     Add Suggested-by: Harry Yoo <harry.yoo@...cle.com>
> ---
>  include/linux/memcontrol.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 873e510d6f8d..7ed15f858dc4 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -534,7 +534,9 @@ static inline struct mem_cgroup *get_mem_cgroup_from_objcg(struct obj_cgroup *ob
>  static inline bool folio_memcg_kmem(struct folio *folio)
>  {
>         VM_BUG_ON_PGFLAGS(PageTail(&folio->page), &folio->page);
> -       VM_BUG_ON_FOLIO(folio->memcg_data & MEMCG_DATA_OBJEXTS, folio);
> +       VM_BUG_ON_FOLIO((folio->memcg_data != OBJEXTS_ALLOC_FAIL) &&
> +                       (folio->memcg_data & MEMCG_DATA_OBJEXTS),
> +                       folio);
>         return folio->memcg_data & MEMCG_DATA_KMEM;
>  }
>
> --
> 2.25.1
>