cgroups.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] cgroup: Fix race condition at rebind_subsystems()
@ 2022-08-23  5:09 Jing-Ting Wu
  0 siblings, 0 replies; 4+ messages in thread
From: Jing-Ting Wu @ 2022-08-23  5:09 UTC (permalink / raw)
  To: Michal Koutný, Tejun Heo, Zefan Li, Johannes Weiner,
	Matthias Brugger
  Cc: Jonathan.JMChen-NuS5LvNUpcJWk0Htik3J/w,
	Lixiong.Liu-NuS5LvNUpcJWk0Htik3J/w,
	wsd_upstream-NuS5LvNUpcJWk0Htik3J/w,
	Wenju.Xu-NuS5LvNUpcJWk0Htik3J/w, Jing-Ting Wu,
	cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Root cause:
The rebind_subsystems() is no lock held when move css object from A
list to B list,then let B's head be treated as css node at
list_for_each_entry_rcu().

Solution:
Add grace period before invalidating the removed rstat_css_node.

Suggested-by: Michal Koutn√Ω <mkoutny-IBi9RG/b67k@public.gmane.org>
Signed-off-by: Jing-Ting Wu <Jing-Ting.Wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 kernel/cgroup/cgroup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index ffaccd6373f1..0d0c959966ed 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1820,6 +1820,7 @@ int rebind_subsystems(struct cgroup_root *dst_root, u16 ss_mask)
 
 		if (ss->css_rstat_flush) {
 			list_del_rcu(&css->rstat_css_node);
+			synchronize_rcu();
 			list_add_rcu(&css->rstat_css_node,
 				     &dcgrp->rstat_css_list);
 		}
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH 1/1] cgroup: Fix race condition at rebind_subsystems()
@ 2022-08-23  5:41 Jing-Ting Wu
       [not found] ` <20220823054148.29346-1-Jing-Ting.Wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
  2022-08-23 18:12 ` Tejun Heo
  0 siblings, 2 replies; 4+ messages in thread
From: Jing-Ting Wu @ 2022-08-23  5:41 UTC (permalink / raw)
  To: Michal Koutný, Tejun Heo, Zefan Li, Johannes Weiner,
	Matthias Brugger
  Cc: Jonathan.JMChen-NuS5LvNUpcJWk0Htik3J/w,
	Lixiong.Liu-NuS5LvNUpcJWk0Htik3J/w,
	wsd_upstream-NuS5LvNUpcJWk0Htik3J/w,
	Wenju.Xu-NuS5LvNUpcJWk0Htik3J/w, Jing-Ting Wu, Jing-Ting Wu,
	cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Root cause:
The rebind_subsystems() is no lock held when move css object from A
list to B list,then let B's head be treated as css node at
list_for_each_entry_rcu().

Solution:
Add grace period before invalidating the removed rstat_css_node.

Reported-by: Jing-Ting Wu <jing-ting.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Suggested-by: Michal Koutn√Ω <mkoutny-IBi9RG/b67k@public.gmane.org>
Signed-off-by: Jing-Ting Wu <jing-ting.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Tested-by: Jing-Ting Wu <jing-ting.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Link: https://lore.kernel.org/linux-arm-kernel/d8f0bc5e2fb6ed259f9334c83279b4c011283c41.camel-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org/T/
---
 kernel/cgroup/cgroup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index ffaccd6373f1..0d0c959966ed 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1820,6 +1820,7 @@ int rebind_subsystems(struct cgroup_root *dst_root, u16 ss_mask)
 
 		if (ss->css_rstat_flush) {
 			list_del_rcu(&css->rstat_css_node);
+			synchronize_rcu();
 			list_add_rcu(&css->rstat_css_node,
 				     &dcgrp->rstat_css_list);
 		}
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] cgroup: Fix race condition at rebind_subsystems()
       [not found] ` <20220823054148.29346-1-Jing-Ting.Wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
@ 2022-08-23  7:33   ` Mukesh Ojha
  0 siblings, 0 replies; 4+ messages in thread
From: Mukesh Ojha @ 2022-08-23  7:33 UTC (permalink / raw)
  To: Jing-Ting Wu, Michal Koutný, Tejun Heo, Zefan Li,
	Johannes Weiner, Matthias Brugger
  Cc: Jonathan.JMChen-NuS5LvNUpcJWk0Htik3J/w,
	Lixiong.Liu-NuS5LvNUpcJWk0Htik3J/w,
	wsd_upstream-NuS5LvNUpcJWk0Htik3J/w,
	Wenju.Xu-NuS5LvNUpcJWk0Htik3J/w, cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r



On 8/23/2022 11:11 AM, Jing-Ting Wu wrote:
> Root cause:
> The rebind_subsystems() is no lock held when move css object from A
> list to B list,then let B's head be treated as css node at
> list_for_each_entry_rcu().
> 
> Solution:
> Add grace period before invalidating the removed rstat_css_node.
> 
> Reported-by: Jing-Ting Wu <jing-ting.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Suggested-by: Michal Koutn√Ω <mkoutny-IBi9RG/b67k@public.gmane.org>
> Signed-off-by: Jing-Ting Wu <jing-ting.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Tested-by: Jing-Ting Wu <jing-ting.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Link: https://lore.kernel.org/linux-arm-kernel/d8f0bc5e2fb6ed259f9334c83279b4c011283c41.camel-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org/T/
> ---
>   kernel/cgroup/cgroup.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index ffaccd6373f1..0d0c959966ed 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -1820,6 +1820,7 @@ int rebind_subsystems(struct cgroup_root *dst_root, u16 ss_mask)
>   
>   		if (ss->css_rstat_flush) {
>   			list_del_rcu(&css->rstat_css_node);
> +			synchronize_rcu();
>   			list_add_rcu(&css->rstat_css_node,
>   				     &dcgrp->rstat_css_list);
>   		}

Good catch.

Acked-by: Mukesh Ojha <quic_mojha-jfJNa2p1gH1BDgjK7y7TUQ@public.gmane.org>

-Mukesh


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] cgroup: Fix race condition at rebind_subsystems()
  2022-08-23  5:41 [PATCH 1/1] cgroup: Fix race condition at rebind_subsystems() Jing-Ting Wu
       [not found] ` <20220823054148.29346-1-Jing-Ting.Wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
@ 2022-08-23 18:12 ` Tejun Heo
  1 sibling, 0 replies; 4+ messages in thread
From: Tejun Heo @ 2022-08-23 18:12 UTC (permalink / raw)
  To: Jing-Ting Wu
  Cc: Michal Koutný, Zefan Li, Johannes Weiner, Matthias Brugger,
	Jonathan.JMChen, Lixiong.Liu, wsd_upstream, Wenju.Xu, cgroups,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Tue, Aug 23, 2022 at 01:41:46PM +0800, Jing-Ting Wu wrote:
> Root cause:
> The rebind_subsystems() is no lock held when move css object from A
> list to B list,then let B's head be treated as css node at
> list_for_each_entry_rcu().
> 
> Solution:
> Add grace period before invalidating the removed rstat_css_node.
> 
> Reported-by: Jing-Ting Wu <jing-ting.wu@mediatek.com>
> Suggested-by: Michal Koutný <mkoutny@suse.com>
> Signed-off-by: Jing-Ting Wu <jing-ting.wu@mediatek.com>
> Tested-by: Jing-Ting Wu <jing-ting.wu@mediatek.com>
> Link: https://lore.kernel.org/linux-arm-kernel/d8f0bc5e2fb6ed259f9334c83279b4c011283c41.camel@mediatek.com/T/

Applied to cgroup/for-6.0-fixes w/ stable tag added.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-23 18:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-23  5:41 [PATCH 1/1] cgroup: Fix race condition at rebind_subsystems() Jing-Ting Wu
     [not found] ` <20220823054148.29346-1-Jing-Ting.Wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
2022-08-23  7:33   ` Mukesh Ojha
2022-08-23 18:12 ` Tejun Heo
  -- strict thread matches above, loose matches on Subject: below --
2022-08-23  5:09 Jing-Ting Wu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).