linux-kernel - Re: [PATCH] mm: memcontrol: fix root_mem

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]

Date:   Tue, 2 Mar 2021 10:58:36 -0800
From:   Roman Gushchin <guro@...com>
To:     Muchun Song <songmuchun@...edance.com>
CC:     <hannes@...xchg.org>, <mhocko@...nel.org>,
        <akpm@...ux-foundation.org>, <shakeelb@...gle.com>,
        <linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>
Subject: Re: [PATCH] mm: memcontrol: fix root_mem_cgroup charging

On Tue, Mar 02, 2021 at 04:18:23PM +0800, Muchun Song wrote:
> CPU0:                                   CPU1:
> 
> objcg = get_obj_cgroup_from_current();
> obj_cgroup_charge(objcg);
>                                         memcg_reparent_objcgs();
>                                             xchg(&objcg->memcg, root_mem_cgroup);
>     // memcg == root_mem_cgroup
>     memcg = obj_cgroup_memcg(objcg);
>     __memcg_kmem_charge(memcg);
>         // Do not charge to the root memcg
>         try_charge(memcg);
> 
> If the objcg->memcg is reparented to the root_mem_cgroup,
> obj_cgroup_charge() can pass root_mem_cgroup as the first
> parameter to here. The root_mem_cgroup is skipped in the
> try_charge(). So the page counters of it do not update.
> 
> When we uncharge this, we will decrease the page counters
> (e.g. memory and memsw) of the root_mem_cgroup. This will
> cause the page counters of the root_mem_cgroup to be out
> of balance. Fix it by charging the page to the
> root_mem_cgroup unconditional.

Is this a problem? It seems that we do not expose root memcg's counters
except kmem and tcp. It seems that the described problem is not
applicable to the kmem counter. Please, explain.

Thanks!

> 
> Fixes: bf4f059954dc ("mm: memcg/slab: obj_cgroup API")
> Signed-off-by: Muchun Song <songmuchun@...edance.com>
> ---
>  mm/memcontrol.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 2db2aeac8a9e..edf604824d63 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3078,6 +3078,19 @@ static int __memcg_kmem_charge(struct mem_cgroup *memcg, gfp_t gfp,
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * If the objcg->memcg is reparented to the root_mem_cgroup,
> +	 * obj_cgroup_charge() can pass root_mem_cgroup as the first
> +	 * parameter to here. We should charge the page to the
> +	 * root_mem_cgroup unconditional to keep it's page counters
> +	 * balance.
> +	 */
> +	if (unlikely(mem_cgroup_is_root(memcg))) {
> +		page_counter_charge(&memcg->memory, nr_pages);
> +		if (do_memsw_account())
> +			page_counter_charge(&memcg->memsw, nr_pages);
> +	}
> +
>  	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) &&
>  	    !page_counter_try_charge(&memcg->kmem, nr_pages, &counter)) {
>  
> -- 
> 2.11.0
>