linux-kernel - Re: [PATCH v2] mm: memcg/slab: Fix return child memcg objcg for root 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: <20201029212557.GJ827280@carbon.dhcp.thefacebook.com>
Date:   Thu, 29 Oct 2020 14:25:57 -0700
From:   Roman Gushchin <guro@...com>
To:     Shakeel Butt <shakeelb@...gle.com>
CC:     Muchun Song <songmuchun@...edance.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Michal Hocko <mhocko@...nel.org>,
        Vladimir Davydov <vdavydov.dev@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Yafang Shao <laoar.shao@...il.com>,
        Chris Down <chris@...isdown.name>,
        Christian Brauner <christian.brauner@...ntu.com>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Thomas Gleixner <tglx@...utronix.de>, <esyr@...hat.com>,
        Suren Baghdasaryan <surenb@...gle.com>, <areber@...hat.com>,
        Marco Elver <elver@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Cgroups <cgroups@...r.kernel.org>, Linux MM <linux-mm@...ck.org>
Subject: Re: [PATCH v2] mm: memcg/slab: Fix return child memcg objcg for root
 memcg

On Thu, Oct 29, 2020 at 01:34:57PM -0700, Shakeel Butt wrote:
> On Thu, Oct 29, 2020 at 10:10 AM Roman Gushchin <guro@...com> wrote:
> >
> > On Thu, Oct 29, 2020 at 08:48:45AM -0700, Shakeel Butt wrote:
> > > On Tue, Oct 27, 2020 at 8:50 PM Muchun Song <songmuchun@...edance.com> wrote:
> > > >
> > > > Consider the following memcg hierarchy.
> > > >
> > > >                     root
> > > >                    /    \
> > > >                   A      B
> > > >
> > > > If we get the objcg of memcg A failed,
> > >
> > > Please fix the above statement.
> > >
> > > > the get_obj_cgroup_from_current
> > > > can return the wrong objcg for the root memcg.
> > > >
> > > > Fixes: bf4f059954dc ("mm: memcg/slab: obj_cgroup API")
> > > > Signed-off-by: Muchun Song <songmuchun@...edance.com>
> > > > ---
> > > >  changelog in v2:
> > > >  1. Do not use a comparison with the root_mem_cgroup
> > > >
> > > >  mm/memcontrol.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > > > index 1337775b04f3..8c8b4c3ed5a0 100644
> > > > --- a/mm/memcontrol.c
> > > > +++ b/mm/memcontrol.c
> > > > @@ -2961,6 +2961,7 @@ __always_inline struct obj_cgroup *get_obj_cgroup_from_current(void)
> > > >                 objcg = rcu_dereference(memcg->objcg);
> > > >                 if (objcg && obj_cgroup_tryget(objcg))
> > > >                         break;
> > > > +               objcg = NULL;
> > >
> > > Roman, in your cleanup, are you planning to have objcg for root memcg as well?
> >
> > Yes. I'll just change the for loop to include the root_mem_cgroup.
> >
> 
> Then do we really need this patch since it's not tagged for stable?

A good question.

Honestly, I really doubt that obj_cgroup_tryget() can fail even with the current code.
But, formally speaking, it's possible. So there is likely no practical difference if
we'll apply this patch or not, even without the root handling cleanup.

But you're right, with the root handling cleanup, it will make even less of a difference.

Thanks!