linux-kernel - Re: [PATCH mm-unstable v2 2/5] mm: don't hold css->refcnt during traversal

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: <qh77aw6nnsytwtux6f2bkzmene3fzrh4skegvqktlw4b47jgea@oxovqnsrulef>
Date: Wed, 14 Aug 2024 11:00:25 +0200
From: Michal Koutný <mkoutny@...e.com>
To: Kinsey Ho <kinseyho@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org, cgroups@...r.kernel.org, Yosry Ahmed <yosryahmed@...gle.com>, 
	Roman Gushchin <roman.gushchin@...ux.dev>, Johannes Weiner <hannes@...xchg.org>, 
	Michal Hocko <mhocko@...nel.org>, Shakeel Butt <shakeel.butt@...ux.dev>, 
	Muchun Song <muchun.song@...ux.dev>, Tejun Heo <tj@...nel.org>, Zefan Li <lizefan.x@...edance.com>
Subject: Re: [PATCH mm-unstable v2 2/5] mm: don't hold css->refcnt during
 traversal

On Tue, Aug 13, 2024 at 08:47:12PM GMT, Kinsey Ho <kinseyho@...gle.com> wrote:
> To obtain the pointer to the next memcg position, mem_cgroup_iter()
> currently holds css->refcnt during memcg traversal only to put
> css->refcnt at the end of the routine. This isn't necessary as an
> rcu_read_lock is already held throughout the function. The use of
> the RCU read lock with css_next_descendant_pre() guarantees that
> sibling linkage is safe without holding a ref on the passed-in @css.
> 
> Remove css->refcnt usage during traversal by leveraging RCU.
> 
> Signed-off-by: Kinsey Ho <kinseyho@...gle.com>
> ---
>  include/linux/memcontrol.h |  2 +-
>  mm/memcontrol.c            | 18 +-----------------
>  2 files changed, 2 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 90ecd2dbca06..1aaed2f1f6ae 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -75,7 +75,7 @@ struct lruvec_stats_percpu;
>  struct lruvec_stats;
>  
>  struct mem_cgroup_reclaim_iter {
> -	struct mem_cgroup *position;
> +	struct mem_cgroup __rcu *position;

I'm not sure about this annotation.
This pointer could be modified concurrently with RCU read sections with
the cmpxchg which would assume that's equivalent with
rcu_assign_pointer(). (Which it might be but it's not idiomatic, so it
causes some head wrapping.)
Isn't this situation covered with a regular pointer and
READ_ONCE()+cmpxchg?

Regards,
Michal

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)