linux-kernel - Re: [PATCH] mm: memcontrol: fix forget to obtain the ref to objcg in split_page

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: <YGU/ZojpKXXK9AnU@carbon.DHCP.thefacebook.com>
Date:   Wed, 31 Mar 2021 20:35:02 -0700
From:   Roman Gushchin <guro@...com>
To:     Miaohe Lin <linmiaohe@...wei.com>
CC:     Muchun Song <songmuchun@...edance.com>,
        <linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>,
        <duanxiongchun@...edance.com>,
        Christian Borntraeger <borntraeger@...ibm.com>,
        <hannes@...xchg.org>, <mhocko@...nel.org>,
        <akpm@...ux-foundation.org>, <shakeelb@...gle.com>,
        <vdavydov.dev@...il.com>
Subject: Re: [PATCH] mm: memcontrol: fix forget to obtain the ref to objcg in
 split_page_memcg

On Thu, Apr 01, 2021 at 11:31:16AM +0800, Miaohe Lin wrote:
> On 2021/4/1 11:01, Muchun Song wrote:
> > Christian Borntraeger reported a warning about "percpu ref
> > (obj_cgroup_release) <= 0 (-1) after switching to atomic".
> > Because we forgot to obtain the reference to the objcg and
> > wrongly obtain the reference of memcg.
> > 
> > Reported-by: Christian Borntraeger <borntraeger@...ibm.com>
> > Signed-off-by: Muchun Song <songmuchun@...edance.com>
> 
> Thanks for the patch.
> Is a Fixes tag needed?

No, as the original patch hasn't been merged into the Linus's tree yet.
So the fix can be simply squashed.

Btw, the fix looks good to me.

Acked-by: Roman Gushchin <guro@...com>

> 
> > ---
> >  include/linux/memcontrol.h | 6 ++++++
> >  mm/memcontrol.c            | 6 +++++-
> >  2 files changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > index 0e8907957227..c960fd49c3e8 100644
> > --- a/include/linux/memcontrol.h
> > +++ b/include/linux/memcontrol.h
> > @@ -804,6 +804,12 @@ static inline void obj_cgroup_get(struct obj_cgroup *objcg)
> >  	percpu_ref_get(&objcg->refcnt);
> >  }
> >  
> > +static inline void obj_cgroup_get_many(struct obj_cgroup *objcg,
> > +				       unsigned long nr)
> > +{
> > +	percpu_ref_get_many(&objcg->refcnt, nr);
> > +}
> > +
> >  static inline void obj_cgroup_put(struct obj_cgroup *objcg)
> >  {
> >  	percpu_ref_put(&objcg->refcnt);
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index c0b83a396299..64ada9e650a5 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -3133,7 +3133,11 @@ void split_page_memcg(struct page *head, unsigned int nr)
> >  
> >  	for (i = 1; i < nr; i++)
> >  		head[i].memcg_data = head->memcg_data;
> > -	css_get_many(&memcg->css, nr - 1);
> > +
> > +	if (PageMemcgKmem(head))
> > +		obj_cgroup_get_many(__page_objcg(head), nr - 1);
> > +	else
> > +		css_get_many(&memcg->css, nr - 1);
> >  }
> >  
> >  #ifdef CONFIG_MEMCG_SWAP
> > 
>