* [PATCH 5.10/5.15 1/2] cgroup: Make operations on the cgroup root_list RCU safe
2024-09-19 8:47 ` Fedor Pchelkin
@ 2024-09-19 8:51 ` Fedor Pchelkin
2024-09-19 8:51 ` [PATCH 5.10/5.15 2/2] cgroup: Move rcu_head up near the top of cgroup_root Fedor Pchelkin
2024-09-19 9:26 ` 5.10.225 stable kernel cgroup_mutex not held assertion failure chenridong
` (5 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: Fedor Pchelkin @ 2024-09-19 8:51 UTC (permalink / raw)
To: Greg Thelen
Cc: Fedor Pchelkin, Chen Ridong, Tejun Heo, Shivani Agarwal,
Greg Kroah-Hartman, stable, lvc-project, Michal Koutný,
Yafang Shao
From: Yafang Shao <laoar.shao@gmail.com>
commit d23b5c577715892c87533b13923306acc6243f93 upstream.
At present, when we perform operations on the cgroup root_list, we must
hold the cgroup_mutex, which is a relatively heavyweight lock. In reality,
we can make operations on this list RCU-safe, eliminating the need to hold
the cgroup_mutex during traversal. Modifications to the list only occur in
the cgroup root setup and destroy paths, which should be infrequent in a
production environment. In contrast, traversal may occur frequently.
Therefore, making it RCU-safe would be beneficial.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
[fp: adapt to 5.10 mainly because of changes made by e210a89f5b07
("cgroup.c: add helper __cset_cgroup_from_root to cleanup duplicated
codes")]
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
include/linux/cgroup-defs.h | 1 +
kernel/cgroup/cgroup-internal.h | 3 ++-
kernel/cgroup/cgroup.c | 23 ++++++++++++++++-------
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index c9fafca1c30c..08912a5dd438 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -512,6 +512,7 @@ struct cgroup_root {
/* A list running through the active hierarchies */
struct list_head root_list;
+ struct rcu_head rcu;
/* Hierarchy-specific flags */
unsigned int flags;
diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index d8fcc139ac05..f38f56b8cc41 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -172,7 +172,8 @@ extern struct list_head cgroup_roots;
/* iterate across the hierarchies */
#define for_each_root(root) \
- list_for_each_entry((root), &cgroup_roots, root_list)
+ list_for_each_entry_rcu((root), &cgroup_roots, root_list, \
+ lockdep_is_held(&cgroup_mutex))
/**
* for_each_subsys - iterate all enabled cgroup subsystems
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 11400eba6124..3a0e60605cbe 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1304,7 +1304,7 @@ static void cgroup_exit_root_id(struct cgroup_root *root)
void cgroup_free_root(struct cgroup_root *root)
{
- kfree(root);
+ kfree_rcu(root, rcu);
}
static void cgroup_destroy_root(struct cgroup_root *root)
@@ -1337,7 +1337,7 @@ static void cgroup_destroy_root(struct cgroup_root *root)
spin_unlock_irq(&css_set_lock);
if (!list_empty(&root->root_list)) {
- list_del(&root->root_list);
+ list_del_rcu(&root->root_list);
cgroup_root_count--;
}
@@ -1392,7 +1392,6 @@ static struct cgroup *cset_cgroup_from_root(struct css_set *cset,
{
struct cgroup *res = NULL;
- lockdep_assert_held(&cgroup_mutex);
lockdep_assert_held(&css_set_lock);
if (cset == &init_css_set) {
@@ -1412,13 +1411,23 @@ static struct cgroup *cset_cgroup_from_root(struct css_set *cset,
}
}
- BUG_ON(!res);
+ /*
+ * If cgroup_mutex is not held, the cgrp_cset_link will be freed
+ * before we remove the cgroup root from the root_list. Consequently,
+ * when accessing a cgroup root, the cset_link may have already been
+ * freed, resulting in a NULL res_cgroup. However, by holding the
+ * cgroup_mutex, we ensure that res_cgroup can't be NULL.
+ * If we don't hold cgroup_mutex in the caller, we must do the NULL
+ * check.
+ */
return res;
}
/*
* Return the cgroup for "task" from the given hierarchy. Must be
- * called with cgroup_mutex and css_set_lock held.
+ * called with css_set_lock held to prevent task's groups from being modified.
+ * Must be called with either cgroup_mutex or rcu read lock to prevent the
+ * cgroup root from being destroyed.
*/
struct cgroup *task_cgroup_from_root(struct task_struct *task,
struct cgroup_root *root)
@@ -1950,7 +1959,7 @@ void init_cgroup_root(struct cgroup_fs_context *ctx)
struct cgroup_root *root = ctx->root;
struct cgroup *cgrp = &root->cgrp;
- INIT_LIST_HEAD(&root->root_list);
+ INIT_LIST_HEAD_RCU(&root->root_list);
atomic_set(&root->nr_cgrps, 1);
cgrp->root = root;
init_cgroup_housekeeping(cgrp);
@@ -2028,7 +2037,7 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask)
* care of subsystems' refcounts, which are explicitly dropped in
* the failure exit path.
*/
- list_add(&root->root_list, &cgroup_roots);
+ list_add_rcu(&root->root_list, &cgroup_roots);
cgroup_root_count++;
/*
--
2.39.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 5.10/5.15 2/2] cgroup: Move rcu_head up near the top of cgroup_root
2024-09-19 8:51 ` [PATCH 5.10/5.15 1/2] cgroup: Make operations on the cgroup root_list RCU safe Fedor Pchelkin
@ 2024-09-19 8:51 ` Fedor Pchelkin
0 siblings, 0 replies; 29+ messages in thread
From: Fedor Pchelkin @ 2024-09-19 8:51 UTC (permalink / raw)
To: Greg Thelen
Cc: Fedor Pchelkin, Chen Ridong, Tejun Heo, Shivani Agarwal,
Greg Kroah-Hartman, stable, lvc-project, Michal Koutný,
Stephen Rothwell, Waiman Long, Yafang Shao, Yosry Ahmed
From: Waiman Long <longman@redhat.com>
commit a7fb0423c201ba12815877a0b5a68a6a1710b23a upstream.
Commit d23b5c577715 ("cgroup: Make operations on the cgroup root_list RCU
safe") adds a new rcu_head to the cgroup_root structure and kvfree_rcu()
for freeing the cgroup_root.
The current implementation of kvfree_rcu(), however, has the limitation
that the offset of the rcu_head structure within the larger data
structure must be less than 4096 or the compilation will fail. See the
macro definition of __is_kvfree_rcu_offset() in include/linux/rcupdate.h
for more information.
By putting rcu_head below the large cgroup structure, any change to the
cgroup structure that makes it larger run the risk of causing build
failure under certain configurations. Commit 77070eeb8821 ("cgroup:
Avoid false cacheline sharing of read mostly rstat_cpu") happens to be
the last straw that breaks it. Fix this problem by moving the rcu_head
structure up before the cgroup structure.
Fixes: d23b5c577715 ("cgroup: Make operations on the cgroup root_list RCU safe")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Closes: https://lore.kernel.org/lkml/20231207143806.114e0a74@canb.auug.org.au/
Signed-off-by: Waiman Long <longman@redhat.com>
Acked-by: Yafang Shao <laoar.shao@gmail.com>
Reviewed-by: Yosry Ahmed <yosryahmed@google.com>
Reviewed-by: Michal Koutný <mkoutny@suse.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
include/linux/cgroup-defs.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 08912a5dd438..20b2b32f825b 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -501,6 +501,10 @@ struct cgroup_root {
/* Unique id for this hierarchy. */
int hierarchy_id;
+ /* A list running through the active hierarchies */
+ struct list_head root_list;
+ struct rcu_head rcu;
+
/* The root cgroup. Root is destroyed on its release. */
struct cgroup cgrp;
@@ -510,10 +514,6 @@ struct cgroup_root {
/* Number of cgroups in the hierarchy, used only for /proc/cgroups */
atomic_t nr_cgrps;
- /* A list running through the active hierarchies */
- struct list_head root_list;
- struct rcu_head rcu;
-
/* Hierarchy-specific flags */
unsigned int flags;
--
2.39.5
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: 5.10.225 stable kernel cgroup_mutex not held assertion failure
2024-09-19 8:47 ` Fedor Pchelkin
2024-09-19 8:51 ` [PATCH 5.10/5.15 1/2] cgroup: Make operations on the cgroup root_list RCU safe Fedor Pchelkin
@ 2024-09-19 9:26 ` chenridong
2024-09-20 9:28 ` Shivani Agarwal
` (4 subsequent siblings)
6 siblings, 0 replies; 29+ messages in thread
From: chenridong @ 2024-09-19 9:26 UTC (permalink / raw)
To: Fedor Pchelkin, Greg Thelen
Cc: Tejun Heo, Shivani Agarwal, Greg Kroah-Hartman, stable,
lvc-project, Michal Koutný
On 2024/9/19 16:47, Fedor Pchelkin wrote:
> Greg Thelen wrote:
>> Linux stable v5.10.226 suffers a lockdep warning when accessing
>> /proc/PID/cpuset. cset_cgroup_from_root() is called without cgroup_mutex
>> is held, which causes assertion failure.
>>
>> Bisect blames 5.10.225 commit 688325078a8b ("cgroup/cpuset: Prevent UAF
>> in proc_cpuset_show()"). I've have not easily reproduced the problem
>> that this change fixes, so I'm not sure if it's best to revert the fix
>> or adapt it to meet the 5.10 locking expectations.
>>
>> The lockdep complaint:
>>
>> $ cat /proc/1/cpuset
>> $ dmesg
>> [ 198.744891] ------------[ cut here ]------------
>> [ 198.744918] WARNING: CPU: 4 PID: 9301 at kernel/cgroup/cgroup.c:1395
>> cset_cgroup_from_root+0xb2/0xd0
>> [ 198.744957] RIP: 0010:cset_cgroup_from_root+0xb2/0xd0
>> [ 198.744960] Code: 02 00 00 74 11 48 8b 09 48 39 cb 75 eb eb 19 49 83 c6
>> 10 4c 89 f0 48 85 c0 74 0d 5b 41 5e c3 48 8b 43 60 48 85 c0 75 f3 0f 0b
>> <0f> 0b 83 3d 69 01 ee 01 00 0f 85 78 ff ff ff eb 8b 0f 0b eb 87 66
>> [ 198.744962] RSP: 0018:ffffb492608a7ce8 EFLAGS: 00010046
>> [ 198.744977] RAX: 0000000000000000 RBX: ffffffff8f4171b8 RCX:
>> cc949de848c33e00
>> [ 198.744979] RDX: 0000000000001000 RSI: ffffffff8f415450 RDI:
>> ffff92e5417c4dc0
>> [ 198.744981] RBP: ffff9303467e3f00 R08: 0000000000000008 R09:
>> ffffffff9122d568
>> [ 198.744983] R10: ffff92e5417c4380 R11: 0000000000000000 R12:
>> ffff92e3d9506000
>> [ 198.744984] R13: 0000000000000000 R14: ffff92e443a96000 R15:
>> ffff92e3d9506000
>> [ 198.744987] FS: 00007f15d94ed740(0000) GS:ffff9302bf500000(0000)
>> knlGS:0000000000000000
>> [ 198.744988] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [ 198.744990] CR2: 00007f15d94ca000 CR3: 00000002816ca003 CR4:
>> 00000000001706e0
>> [ 198.744992] Call Trace:
>> [ 198.744996] ? __warn+0xcd/0x1c0
>> [ 198.745000] ? cset_cgroup_from_root+0xb2/0xd0
>> [ 198.745008] ? report_bug+0x87/0xf0
>> [ 198.745015] ? handle_bug+0x42/0x80
>> [ 198.745017] ? exc_invalid_op+0x16/0x70
>> [ 198.745021] ? asm_exc_invalid_op+0x12/0x20
>> [ 198.745030] ? cset_cgroup_from_root+0xb2/0xd0
>> [ 198.745034] ? cset_cgroup_from_root+0x28/0xd0
>> [ 198.745038] cgroup_path_ns_locked+0x23/0x50
>> [ 198.745044] proc_cpuset_show+0x115/0x210
>> [ 198.745049] proc_single_show+0x4a/0xa0
>> [ 198.745056] seq_read_iter+0x14d/0x400
>> [ 198.745063] seq_read+0x103/0x130
>> [ 198.745074] vfs_read+0xea/0x320
>> [ 198.745078] ? do_user_addr_fault+0x25b/0x390
>> [ 198.745085] ? do_user_addr_fault+0x25b/0x390
>> [ 198.745090] ksys_read+0x70/0xe0
>> [ 198.745096] do_syscall_64+0x2d/0x40
>> [ 198.745099] entry_SYSCALL_64_after_hwframe+0x61/0xcb
>
> Hello,
>
> we've also encountered this problem. The thing is that commit 688325078a8b
> ("cgroup/cpuset: Prevent UAF in proc_cpuset_show()") relies on the RCU
> synchronization changes introduced by commit d23b5c577715 ("cgroup: Make
> operations on the cgroup root_list RCU safe") which wasn't backported to
> 5.10 as it couldn't be cleanly applied there. That commit converted access
> to the root_list synchronization from depending on cgroup mutex to be
> RCU-safe.
>
> 5.15 also has this problem, while 6.1 and later stables have the backport
> of this RCU-changing commit so they are not affected. As mentioned by
> Michal here:
> https://lore.kernel.org/stable/xrc6s5oyf3b5hflsffklogluuvd75h2khanrke2laes3en5js2@6kvpkcxs7ufj/
>
Yes, I think commit d23b5c577715 ("cgroup: Make operations on the cgroup
root_list RCU safe") is needed.
> In the next email I'll send the adapted to 5.10/5.15 commit along with its
> upstream-fix to avoid build failure in some situations. Would be nice if
> you give them a try. Thanks!
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: 5.10.225 stable kernel cgroup_mutex not held assertion failure
2024-09-19 8:47 ` Fedor Pchelkin
2024-09-19 8:51 ` [PATCH 5.10/5.15 1/2] cgroup: Make operations on the cgroup root_list RCU safe Fedor Pchelkin
2024-09-19 9:26 ` 5.10.225 stable kernel cgroup_mutex not held assertion failure chenridong
@ 2024-09-20 9:28 ` Shivani Agarwal
2024-10-30 7:29 ` Siddh Raman Pant
2024-09-20 9:29 ` [PATCH v4.19] " Shivani Agarwal
` (3 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: Shivani Agarwal @ 2024-09-20 9:28 UTC (permalink / raw)
To: pchelkin, gregkh, stable
Cc: chenridong, gthelen, lvc-project, mkoutny, shivani.agarwal, tj,
lizefan.x, cgroups, ajay.kaher
> we've also encountered this problem. The thing is that commit 688325078a8b
> ("cgroup/cpuset: Prevent UAF in proc_cpuset_show()") relies on the RCU
> synchronization changes introduced by commit d23b5c577715 ("cgroup: Make
> operations on the cgroup root_list RCU safe") which wasn't backported to
> 5.10 as it couldn't be cleanly applied there. That commit converted access
> to the root_list synchronization from depending on cgroup mutex to be
> RCU-safe.
> 5.15 also has this problem, while 6.1 and later stables have the backport
> of this RCU-changing commit so they are not affected. As mentioned by
> Michal here:
> https://lore.kernel.org/stable/xrc6s5oyf3b5hflsffklogluuvd75h2khanrke2laes3en5js2@6kvpkcxs7ufj/
> In the next email I'll send the adapted to 5.10/5.15 commit along with its
> upstream-fix to avoid build failure in some situations. Would be nice if
> you give them a try. Thanks!
Thanks Fedor.
Upstream commit 1be59c97c83c is merged in 5.4 with commit 10aeaa47e4aa and
in 4.19 with commit 27d6dbdc6485. The issue is reproducible in 5.4 and 4.19
also.
I am sending the backport patch of d23b5c577715 and a7fb0423c201 for 5.4 and
4.19 in the next email.
Thanks,
Shivani
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: 5.10.225 stable kernel cgroup_mutex not held assertion failure
2024-09-20 9:28 ` Shivani Agarwal
@ 2024-10-30 7:29 ` Siddh Raman Pant
2024-11-06 6:10 ` gregkh
0 siblings, 1 reply; 29+ messages in thread
From: Siddh Raman Pant @ 2024-10-30 7:29 UTC (permalink / raw)
To: sashal@kernel.org, stable@vger.kernel.org,
gregkh@linuxfoundation.org
Cc: linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
shivani.agarwal@broadcom.com
[-- Attachment #1: Type: text/plain, Size: 641 bytes --]
Hello maintainers,
On Fri, 20 Sep 2024 02:28:03 -0700, Shivani Agarwal wrote:
> Thanks Fedor.
>
> Upstream commit 1be59c97c83c is merged in 5.4 with commit 10aeaa47e4aa and
> in 4.19 with commit 27d6dbdc6485. The issue is reproducible in 5.4 and 4.19
> also.
>
> I am sending the backport patch of d23b5c577715 and a7fb0423c201 for 5.4 and
> 4.19 in the next email.
Please backport these changes to stable.
"cgroup/cpuset: Prevent UAF in proc_cpuset_show()" has already been
backported and bears CVE-2024-43853. As reported, we may already have
introduced another problem due to the missing backport.
Thanks,
Siddh
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: 5.10.225 stable kernel cgroup_mutex not held assertion failure
2024-10-30 7:29 ` Siddh Raman Pant
@ 2024-11-06 6:10 ` gregkh
2024-11-06 6:24 ` Siddh Raman Pant
0 siblings, 1 reply; 29+ messages in thread
From: gregkh @ 2024-11-06 6:10 UTC (permalink / raw)
To: Siddh Raman Pant
Cc: sashal@kernel.org, stable@vger.kernel.org,
linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
shivani.agarwal@broadcom.com
On Wed, Oct 30, 2024 at 07:29:38AM +0000, Siddh Raman Pant wrote:
> Hello maintainers,
>
> On Fri, 20 Sep 2024 02:28:03 -0700, Shivani Agarwal wrote:
> > Thanks Fedor.
> >
> > Upstream commit 1be59c97c83c is merged in 5.4 with commit 10aeaa47e4aa and
> > in 4.19 with commit 27d6dbdc6485. The issue is reproducible in 5.4 and 4.19
> > also.
> >
> > I am sending the backport patch of d23b5c577715 and a7fb0423c201 for 5.4 and
> > 4.19 in the next email.
>
> Please backport these changes to stable.
>
> "cgroup/cpuset: Prevent UAF in proc_cpuset_show()" has already been
> backported and bears CVE-2024-43853. As reported, we may already have
> introduced another problem due to the missing backport.
What exact commits are needed here? Please submit backported and tested
commits and we will be glad to queue them up.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: 5.10.225 stable kernel cgroup_mutex not held assertion failure
2024-11-06 6:10 ` gregkh
@ 2024-11-06 6:24 ` Siddh Raman Pant
2024-11-20 14:46 ` Siddh Raman Pant
0 siblings, 1 reply; 29+ messages in thread
From: Siddh Raman Pant @ 2024-11-06 6:24 UTC (permalink / raw)
To: gregkh@linuxfoundation.org
Cc: sashal@kernel.org, stable@vger.kernel.org,
linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
shivani.agarwal@broadcom.com
[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]
On Wed, Nov 06 2024 at 11:40:39 +0530, gregkh@linuxfoundation.org
wrote:
> On Wed, Oct 30, 2024 at 07:29:38AM +0000, Siddh Raman Pant wrote:
> > Hello maintainers,
> >
> > On Fri, 20 Sep 2024 02:28:03 -0700, Shivani Agarwal wrote:
> > > Thanks Fedor.
> > >
> > > Upstream commit 1be59c97c83c is merged in 5.4 with commit 10aeaa47e4aa and
> > > in 4.19 with commit 27d6dbdc6485. The issue is reproducible in 5.4 and 4.19
> > > also.
> > >
> > > I am sending the backport patch of d23b5c577715 and a7fb0423c201 for 5.4 and
> > > 4.19 in the next email.
> >
> > Please backport these changes to stable.
> >
> > "cgroup/cpuset: Prevent UAF in proc_cpuset_show()" has already been
> > backported and bears CVE-2024-43853. As reported, we may already have
> > introduced another problem due to the missing backport.
>
> What exact commits are needed here? Please submit backported and tested
> commits and we will be glad to queue them up.
>
> thanks,
>
> greg k-h
Please see the following thread where Shivani posted the patches:
https://lore.kernel.org/all/20240920092803.101047-1-shivani.agarwal@broadcom.com/
Thanks,
Siddh
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: 5.10.225 stable kernel cgroup_mutex not held assertion failure
2024-11-06 6:24 ` Siddh Raman Pant
@ 2024-11-20 14:46 ` Siddh Raman Pant
2024-11-20 14:58 ` gregkh
0 siblings, 1 reply; 29+ messages in thread
From: Siddh Raman Pant @ 2024-11-20 14:46 UTC (permalink / raw)
To: gregkh@linuxfoundation.org
Cc: sashal@kernel.org, stable@vger.kernel.org,
linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
shivani.agarwal@broadcom.com
[-- Attachment #1: Type: text/plain, Size: 1310 bytes --]
On Wed, Nov 06 2024 at 11:54:32 +0530, Siddh Raman Pant wrote:
> On Wed, Nov 06 2024 at 11:40:39 +0530, gregkh@linuxfoundation.org
> wrote:
> > On Wed, Oct 30, 2024 at 07:29:38AM +0000, Siddh Raman Pant wrote:
> > > Hello maintainers,
> > >
> > > On Fri, 20 Sep 2024 02:28:03 -0700, Shivani Agarwal wrote:
> > > > Thanks Fedor.
> > > >
> > > > Upstream commit 1be59c97c83c is merged in 5.4 with commit 10aeaa47e4aa and
> > > > in 4.19 with commit 27d6dbdc6485. The issue is reproducible in 5.4 and 4.19
> > > > also.
> > > >
> > > > I am sending the backport patch of d23b5c577715 and a7fb0423c201 for 5.4 and
> > > > 4.19 in the next email.
> > >
> > > Please backport these changes to stable.
> > >
> > > "cgroup/cpuset: Prevent UAF in proc_cpuset_show()" has already been
> > > backported and bears CVE-2024-43853. As reported, we may already have
> > > introduced another problem due to the missing backport.
> >
> > What exact commits are needed here? Please submit backported and tested
> > commits and we will be glad to queue them up.
> >
> > thanks,
> >
> > greg k-h
>
> Please see the following thread where Shivani posted the patches:
>
> https://lore.kernel.org/all/20240920092803.101047-1-shivani.agarwal@broadcom.com/
>
> Thanks,
> Siddh
Ping...
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: 5.10.225 stable kernel cgroup_mutex not held assertion failure
2024-11-20 14:46 ` Siddh Raman Pant
@ 2024-11-20 14:58 ` gregkh
2024-11-20 17:47 ` Siddh Raman Pant
0 siblings, 1 reply; 29+ messages in thread
From: gregkh @ 2024-11-20 14:58 UTC (permalink / raw)
To: Siddh Raman Pant
Cc: sashal@kernel.org, stable@vger.kernel.org,
linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
shivani.agarwal@broadcom.com
On Wed, Nov 20, 2024 at 02:46:32PM +0000, Siddh Raman Pant wrote:
> On Wed, Nov 06 2024 at 11:54:32 +0530, Siddh Raman Pant wrote:
> > On Wed, Nov 06 2024 at 11:40:39 +0530, gregkh@linuxfoundation.org
> > wrote:
> > > On Wed, Oct 30, 2024 at 07:29:38AM +0000, Siddh Raman Pant wrote:
> > > > Hello maintainers,
> > > >
> > > > On Fri, 20 Sep 2024 02:28:03 -0700, Shivani Agarwal wrote:
> > > > > Thanks Fedor.
> > > > >
> > > > > Upstream commit 1be59c97c83c is merged in 5.4 with commit 10aeaa47e4aa and
> > > > > in 4.19 with commit 27d6dbdc6485. The issue is reproducible in 5.4 and 4.19
> > > > > also.
> > > > >
> > > > > I am sending the backport patch of d23b5c577715 and a7fb0423c201 for 5.4 and
> > > > > 4.19 in the next email.
> > > >
> > > > Please backport these changes to stable.
> > > >
> > > > "cgroup/cpuset: Prevent UAF in proc_cpuset_show()" has already been
> > > > backported and bears CVE-2024-43853. As reported, we may already have
> > > > introduced another problem due to the missing backport.
> > >
> > > What exact commits are needed here? Please submit backported and tested
> > > commits and we will be glad to queue them up.
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > Please see the following thread where Shivani posted the patches:
> >
> > https://lore.kernel.org/all/20240920092803.101047-1-shivani.agarwal@broadcom.com/
> >
> > Thanks,
> > Siddh
>
> Ping...
I don't understand what you want here, sorry.
greg k-h
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: 5.10.225 stable kernel cgroup_mutex not held assertion failure
2024-11-20 14:58 ` gregkh
@ 2024-11-20 17:47 ` Siddh Raman Pant
2024-11-28 10:40 ` Siddh Raman Pant
2024-12-02 9:45 ` gregkh
0 siblings, 2 replies; 29+ messages in thread
From: Siddh Raman Pant @ 2024-11-20 17:47 UTC (permalink / raw)
To: gregkh@linuxfoundation.org
Cc: sashal@kernel.org, stable@vger.kernel.org,
linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
shivani.agarwal@broadcom.com
[-- Attachment #1.1: Type: text/plain, Size: 1969 bytes --]
On Wed, Nov 20 2024 at 20:28:36 +0530, gregkh@linuxfoundation.org
wrote:
> On Wed, Nov 20, 2024 at 02:46:32PM +0000, Siddh Raman Pant wrote:
> > On Wed, Nov 06 2024 at 11:54:32 +0530, Siddh Raman Pant wrote:
> > > On Wed, Nov 06 2024 at 11:40:39 +0530, gregkh@linuxfoundation.org
> > > wrote:
> > > > On Wed, Oct 30, 2024 at 07:29:38AM +0000, Siddh Raman Pant wrote:
> > > > > Hello maintainers,
> > > > >
> > > > > On Fri, 20 Sep 2024 02:28:03 -0700, Shivani Agarwal wrote:
> > > > > > Thanks Fedor.
> > > > > >
> > > > > > Upstream commit 1be59c97c83c is merged in 5.4 with commit 10aeaa47e4aa and
> > > > > > in 4.19 with commit 27d6dbdc6485. The issue is reproducible in 5.4 and 4.19
> > > > > > also.
> > > > > >
> > > > > > I am sending the backport patch of d23b5c577715 and a7fb0423c201 for 5.4 and
> > > > > > 4.19 in the next email.
> > > > >
> > > > > Please backport these changes to stable.
> > > > >
> > > > > "cgroup/cpuset: Prevent UAF in proc_cpuset_show()" has already been
> > > > > backported and bears CVE-2024-43853. As reported, we may already have
> > > > > introduced another problem due to the missing backport.
> > > >
> > > > What exact commits are needed here? Please submit backported and tested
> > > > commits and we will be glad to queue them up.
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > >
> > > Please see the following thread where Shivani posted the patches:
> > >
> > > https://lore.kernel.org/all/20240920092803.101047-1-shivani.agarwal@broadcom.com/
> > >
> > > Thanks,
> > > Siddh
> >
> > Ping...
>
> I don't understand what you want here, sorry.
Please find attached the patch emails for 5.4 with this email. They
apply cleanly to the linux-5.4.y branch.
I was earlier referring to the email thread where the fixed-up commits
for backporting were posted by someone else (i.e. I'm not the original
backporter / conflict fix author).
Thanks,
Siddh
[-- Attachment #1.2: PATCH-v5.4-cgroup-Move-rcu_head-up-near-the-top-of-cgroup_root.mbox --]
[-- Type: application/mbox, Size: 7761 bytes --]
[-- Attachment #1.3: PATCH-v5.4-cgroup-Make-operations-on-the-cgroup-root_list-RCU-safe.mbox --]
[-- Type: application/mbox, Size: 9706 bytes --]
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: 5.10.225 stable kernel cgroup_mutex not held assertion failure
2024-11-20 17:47 ` Siddh Raman Pant
@ 2024-11-28 10:40 ` Siddh Raman Pant
2024-12-02 9:45 ` gregkh
1 sibling, 0 replies; 29+ messages in thread
From: Siddh Raman Pant @ 2024-11-28 10:40 UTC (permalink / raw)
To: gregkh@linuxfoundation.org
Cc: sashal@kernel.org, stable@vger.kernel.org,
linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
shivani.agarwal@broadcom.com
[-- Attachment #1: Type: text/plain, Size: 2059 bytes --]
On Wed, Nov 20 2024 at 23:17:28 +0530, Siddh Raman Pant wrote:
> On Wed, Nov 20 2024 at 20:28:36 +0530, gregkh@linuxfoundation.org
> wrote:
> > On Wed, Nov 20, 2024 at 02:46:32PM +0000, Siddh Raman Pant wrote:
> > > On Wed, Nov 06 2024 at 11:54:32 +0530, Siddh Raman Pant wrote:
> > > > On Wed, Nov 06 2024 at 11:40:39 +0530, gregkh@linuxfoundation.org
> > > > wrote:
> > > > > On Wed, Oct 30, 2024 at 07:29:38AM +0000, Siddh Raman Pant wrote:
> > > > > > Hello maintainers,
> > > > > >
> > > > > > On Fri, 20 Sep 2024 02:28:03 -0700, Shivani Agarwal wrote:
> > > > > > > Thanks Fedor.
> > > > > > >
> > > > > > > Upstream commit 1be59c97c83c is merged in 5.4 with commit 10aeaa47e4aa and
> > > > > > > in 4.19 with commit 27d6dbdc6485. The issue is reproducible in 5.4 and 4.19
> > > > > > > also.
> > > > > > >
> > > > > > > I am sending the backport patch of d23b5c577715 and a7fb0423c201 for 5.4 and
> > > > > > > 4.19 in the next email.
> > > > > >
> > > > > > Please backport these changes to stable.
> > > > > >
> > > > > > "cgroup/cpuset: Prevent UAF in proc_cpuset_show()" has already been
> > > > > > backported and bears CVE-2024-43853. As reported, we may already have
> > > > > > introduced another problem due to the missing backport.
> > > > >
> > > > > What exact commits are needed here? Please submit backported and tested
> > > > > commits and we will be glad to queue them up.
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > >
> > > > Please see the following thread where Shivani posted the patches:
> > > >
> > > > https://lore.kernel.org/all/20240920092803.101047-1-shivani.agarwal@broadcom.com/
> > > >
> > > > Thanks,
> > > > Siddh
> > >
> > > Ping...
> >
> > I don't understand what you want here, sorry.
>
> Please find attached the patch emails for 5.4 with this email. They
> apply cleanly to the linux-5.4.y branch.
Ping?
To be clear again - I want the patches I had posted in the previous
email to be applied to 5.4.y branch.
Thanks,
Siddh
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: 5.10.225 stable kernel cgroup_mutex not held assertion failure
2024-11-20 17:47 ` Siddh Raman Pant
2024-11-28 10:40 ` Siddh Raman Pant
@ 2024-12-02 9:45 ` gregkh
2024-12-02 9:59 ` [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe Siddh Raman Pant
1 sibling, 1 reply; 29+ messages in thread
From: gregkh @ 2024-12-02 9:45 UTC (permalink / raw)
To: Siddh Raman Pant
Cc: sashal@kernel.org, stable@vger.kernel.org,
linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
shivani.agarwal@broadcom.com
On Wed, Nov 20, 2024 at 05:47:36PM +0000, Siddh Raman Pant wrote:
> On Wed, Nov 20 2024 at 20:28:36 +0530, gregkh@linuxfoundation.org
> wrote:
> > On Wed, Nov 20, 2024 at 02:46:32PM +0000, Siddh Raman Pant wrote:
> > > On Wed, Nov 06 2024 at 11:54:32 +0530, Siddh Raman Pant wrote:
> > > > On Wed, Nov 06 2024 at 11:40:39 +0530, gregkh@linuxfoundation.org
> > > > wrote:
> > > > > On Wed, Oct 30, 2024 at 07:29:38AM +0000, Siddh Raman Pant wrote:
> > > > > > Hello maintainers,
> > > > > >
> > > > > > On Fri, 20 Sep 2024 02:28:03 -0700, Shivani Agarwal wrote:
> > > > > > > Thanks Fedor.
> > > > > > >
> > > > > > > Upstream commit 1be59c97c83c is merged in 5.4 with commit 10aeaa47e4aa and
> > > > > > > in 4.19 with commit 27d6dbdc6485. The issue is reproducible in 5.4 and 4.19
> > > > > > > also.
> > > > > > >
> > > > > > > I am sending the backport patch of d23b5c577715 and a7fb0423c201 for 5.4 and
> > > > > > > 4.19 in the next email.
> > > > > >
> > > > > > Please backport these changes to stable.
> > > > > >
> > > > > > "cgroup/cpuset: Prevent UAF in proc_cpuset_show()" has already been
> > > > > > backported and bears CVE-2024-43853. As reported, we may already have
> > > > > > introduced another problem due to the missing backport.
> > > > >
> > > > > What exact commits are needed here? Please submit backported and tested
> > > > > commits and we will be glad to queue them up.
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > >
> > > > Please see the following thread where Shivani posted the patches:
> > > >
> > > > https://lore.kernel.org/all/20240920092803.101047-1-shivani.agarwal@broadcom.com/
> > > >
> > > > Thanks,
> > > > Siddh
> > >
> > > Ping...
> >
> > I don't understand what you want here, sorry.
>
> Please find attached the patch emails for 5.4 with this email. They
> apply cleanly to the linux-5.4.y branch.
Please resend these as patches, in the correct order, not as attachments
as it's hard to review and handle them this way.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe
2024-12-02 9:45 ` gregkh
@ 2024-12-02 9:59 ` Siddh Raman Pant
2024-12-02 9:59 ` [PATCH 2/2] cgroup: Move rcu_head up near the top of cgroup_root Siddh Raman Pant
2024-12-02 10:01 ` [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe Siddh Raman Pant
0 siblings, 2 replies; 29+ messages in thread
From: Siddh Raman Pant @ 2024-12-02 9:59 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sasha Levin, stable, linux-kernel, cgroups, Shivani Agarwal
From: Yafang Shao <laoar.shao@gmail.com>
commit d23b5c577715892c87533b13923306acc6243f93 upstream.
At present, when we perform operations on the cgroup root_list, we must
hold the cgroup_mutex, which is a relatively heavyweight lock. In reality,
we can make operations on this list RCU-safe, eliminating the need to hold
the cgroup_mutex during traversal. Modifications to the list only occur in
the cgroup root setup and destroy paths, which should be infrequent in a
production environment. In contrast, traversal may occur frequently.
Therefore, making it RCU-safe would be beneficial.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
[fp: adapt to 5.10 mainly because of changes made by e210a89f5b07
("cgroup.c: add helper __cset_cgroup_from_root to cleanup duplicated
codes")]
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
[Shivani: Modified to apply on v5.4.y]
Signed-off-by: Shivani Agarwal <shivani.agarwal@broadcom.com>
Reviewed-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
include/linux/cgroup-defs.h | 1 +
kernel/cgroup/cgroup-internal.h | 3 ++-
kernel/cgroup/cgroup.c | 23 ++++++++++++++++-------
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index d15884957e7f..c64f11674850 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -517,6 +517,7 @@ struct cgroup_root {
/* A list running through the active hierarchies */
struct list_head root_list;
+ struct rcu_head rcu;
/* Hierarchy-specific flags */
unsigned int flags;
diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index 803989eae99e..bb85acc1114e 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -172,7 +172,8 @@ extern struct list_head cgroup_roots;
/* iterate across the hierarchies */
#define for_each_root(root) \
- list_for_each_entry((root), &cgroup_roots, root_list)
+ list_for_each_entry_rcu((root), &cgroup_roots, root_list, \
+ lockdep_is_held(&cgroup_mutex))
/**
* for_each_subsys - iterate all enabled cgroup subsystems
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 79e57b6df731..273a8a42cb72 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1314,7 +1314,7 @@ void cgroup_free_root(struct cgroup_root *root)
{
if (root) {
idr_destroy(&root->cgroup_idr);
- kfree(root);
+ kfree_rcu(root, rcu);
}
}
@@ -1348,7 +1348,7 @@ static void cgroup_destroy_root(struct cgroup_root *root)
spin_unlock_irq(&css_set_lock);
if (!list_empty(&root->root_list)) {
- list_del(&root->root_list);
+ list_del_rcu(&root->root_list);
cgroup_root_count--;
}
@@ -1401,7 +1401,6 @@ static struct cgroup *cset_cgroup_from_root(struct css_set *cset,
{
struct cgroup *res = NULL;
- lockdep_assert_held(&cgroup_mutex);
lockdep_assert_held(&css_set_lock);
if (cset == &init_css_set) {
@@ -1421,13 +1420,23 @@ static struct cgroup *cset_cgroup_from_root(struct css_set *cset,
}
}
- BUG_ON(!res);
+ /*
+ * If cgroup_mutex is not held, the cgrp_cset_link will be freed
+ * before we remove the cgroup root from the root_list. Consequently,
+ * when accessing a cgroup root, the cset_link may have already been
+ * freed, resulting in a NULL res_cgroup. However, by holding the
+ * cgroup_mutex, we ensure that res_cgroup can't be NULL.
+ * If we don't hold cgroup_mutex in the caller, we must do the NULL
+ * check.
+ */
return res;
}
/*
* Return the cgroup for "task" from the given hierarchy. Must be
- * called with cgroup_mutex and css_set_lock held.
+ * called with css_set_lock held to prevent task's groups from being modified.
+ * Must be called with either cgroup_mutex or rcu read lock to prevent the
+ * cgroup root from being destroyed.
*/
struct cgroup *task_cgroup_from_root(struct task_struct *task,
struct cgroup_root *root)
@@ -2012,7 +2021,7 @@ void init_cgroup_root(struct cgroup_fs_context *ctx)
struct cgroup_root *root = ctx->root;
struct cgroup *cgrp = &root->cgrp;
- INIT_LIST_HEAD(&root->root_list);
+ INIT_LIST_HEAD_RCU(&root->root_list);
atomic_set(&root->nr_cgrps, 1);
cgrp->root = root;
init_cgroup_housekeeping(cgrp);
@@ -2094,7 +2103,7 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask)
* care of subsystems' refcounts, which are explicitly dropped in
* the failure exit path.
*/
- list_add(&root->root_list, &cgroup_roots);
+ list_add_rcu(&root->root_list, &cgroup_roots);
cgroup_root_count++;
/*
--
2.45.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 2/2] cgroup: Move rcu_head up near the top of cgroup_root
2024-12-02 9:59 ` [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe Siddh Raman Pant
@ 2024-12-02 9:59 ` Siddh Raman Pant
2024-12-02 10:01 ` [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe Siddh Raman Pant
1 sibling, 0 replies; 29+ messages in thread
From: Siddh Raman Pant @ 2024-12-02 9:59 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sasha Levin, stable, linux-kernel, cgroups, Shivani Agarwal
From: Waiman Long <longman@redhat.com>
commit a7fb0423c201ba12815877a0b5a68a6a1710b23a upstream.
Commit d23b5c577715 ("cgroup: Make operations on the cgroup root_list RCU
safe") adds a new rcu_head to the cgroup_root structure and kvfree_rcu()
for freeing the cgroup_root.
The current implementation of kvfree_rcu(), however, has the limitation
that the offset of the rcu_head structure within the larger data
structure must be less than 4096 or the compilation will fail. See the
macro definition of __is_kvfree_rcu_offset() in include/linux/rcupdate.h
for more information.
By putting rcu_head below the large cgroup structure, any change to the
cgroup structure that makes it larger run the risk of causing build
failure under certain configurations. Commit 77070eeb8821 ("cgroup:
Avoid false cacheline sharing of read mostly rstat_cpu") happens to be
the last straw that breaks it. Fix this problem by moving the rcu_head
structure up before the cgroup structure.
Fixes: d23b5c577715 ("cgroup: Make operations on the cgroup root_list RCU safe")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Closes: https://lore.kernel.org/lkml/20231207143806.114e0a74@canb.auug.org.au/
Signed-off-by: Waiman Long <longman@redhat.com>
Acked-by: Yafang Shao <laoar.shao@gmail.com>
Reviewed-by: Yosry Ahmed <yosryahmed@google.com>
Reviewed-by: Michal Koutný <mkoutny@suse.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
[Shivani: Modified to apply on v5.4.y]
Signed-off-by: Shivani Agarwal <shivani.agarwal@broadcom.com>
Reviewed-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
include/linux/cgroup-defs.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index c64f11674850..f0798d98be8e 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -506,6 +506,10 @@ struct cgroup_root {
/* Unique id for this hierarchy. */
int hierarchy_id;
+ /* A list running through the active hierarchies */
+ struct list_head root_list;
+ struct rcu_head rcu;
+
/* The root cgroup. Root is destroyed on its release. */
struct cgroup cgrp;
@@ -515,10 +519,6 @@ struct cgroup_root {
/* Number of cgroups in the hierarchy, used only for /proc/cgroups */
atomic_t nr_cgrps;
- /* A list running through the active hierarchies */
- struct list_head root_list;
- struct rcu_head rcu;
-
/* Hierarchy-specific flags */
unsigned int flags;
--
2.45.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe
2024-12-02 9:59 ` [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe Siddh Raman Pant
2024-12-02 9:59 ` [PATCH 2/2] cgroup: Move rcu_head up near the top of cgroup_root Siddh Raman Pant
@ 2024-12-02 10:01 ` Siddh Raman Pant
2024-12-02 10:06 ` gregkh
1 sibling, 1 reply; 29+ messages in thread
From: Siddh Raman Pant @ 2024-12-02 10:01 UTC (permalink / raw)
To: gregkh@linuxfoundation.org
Cc: sashal@kernel.org, stable@vger.kernel.org,
linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
shivani.agarwal@broadcom.com
[-- Attachment #1: Type: text/plain, Size: 296 bytes --]
On Mon, Dec 02 2024 at 15:29:25 +0530, Siddh Raman Pant wrote:
> Reviewed-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
> Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
Oops, please remove signed-off-by it was added by git format-patch
automatically.
Thanks,
Siddh
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe
2024-12-02 10:01 ` [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe Siddh Raman Pant
@ 2024-12-02 10:06 ` gregkh
2024-12-02 10:11 ` Siddh Raman Pant
0 siblings, 1 reply; 29+ messages in thread
From: gregkh @ 2024-12-02 10:06 UTC (permalink / raw)
To: Siddh Raman Pant
Cc: sashal@kernel.org, stable@vger.kernel.org,
linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
shivani.agarwal@broadcom.com
On Mon, Dec 02, 2024 at 10:01:54AM +0000, Siddh Raman Pant wrote:
> On Mon, Dec 02 2024 at 15:29:25 +0530, Siddh Raman Pant wrote:
> > Reviewed-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
> > Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
>
> Oops, please remove signed-off-by it was added by git format-patch
> automatically.
Please fix things up so maintainers do not have to manually hand-edit
patches :(
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe
2024-12-02 10:06 ` gregkh
@ 2024-12-02 10:11 ` Siddh Raman Pant
2024-12-02 10:11 ` [PATCH 2/2] cgroup: Move rcu_head up near the top of cgroup_root Siddh Raman Pant
2024-12-02 10:17 ` [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe Greg Kroah-Hartman
0 siblings, 2 replies; 29+ messages in thread
From: Siddh Raman Pant @ 2024-12-02 10:11 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sasha Levin, stable, linux-kernel, cgroups, Shivani Agarwal
From: Yafang Shao <laoar.shao@gmail.com>
commit d23b5c577715892c87533b13923306acc6243f93 upstream.
At present, when we perform operations on the cgroup root_list, we must
hold the cgroup_mutex, which is a relatively heavyweight lock. In reality,
we can make operations on this list RCU-safe, eliminating the need to hold
the cgroup_mutex during traversal. Modifications to the list only occur in
the cgroup root setup and destroy paths, which should be infrequent in a
production environment. In contrast, traversal may occur frequently.
Therefore, making it RCU-safe would be beneficial.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
[fp: adapt to 5.10 mainly because of changes made by e210a89f5b07
("cgroup.c: add helper __cset_cgroup_from_root to cleanup duplicated
codes")]
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
[Shivani: Modified to apply on v5.4.y]
Signed-off-by: Shivani Agarwal <shivani.agarwal@broadcom.com>
Reviewed-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
include/linux/cgroup-defs.h | 1 +
kernel/cgroup/cgroup-internal.h | 3 ++-
kernel/cgroup/cgroup.c | 23 ++++++++++++++++-------
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index d15884957e7f..c64f11674850 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -517,6 +517,7 @@ struct cgroup_root {
/* A list running through the active hierarchies */
struct list_head root_list;
+ struct rcu_head rcu;
/* Hierarchy-specific flags */
unsigned int flags;
diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index 803989eae99e..bb85acc1114e 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -172,7 +172,8 @@ extern struct list_head cgroup_roots;
/* iterate across the hierarchies */
#define for_each_root(root) \
- list_for_each_entry((root), &cgroup_roots, root_list)
+ list_for_each_entry_rcu((root), &cgroup_roots, root_list, \
+ lockdep_is_held(&cgroup_mutex))
/**
* for_each_subsys - iterate all enabled cgroup subsystems
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 79e57b6df731..273a8a42cb72 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1314,7 +1314,7 @@ void cgroup_free_root(struct cgroup_root *root)
{
if (root) {
idr_destroy(&root->cgroup_idr);
- kfree(root);
+ kfree_rcu(root, rcu);
}
}
@@ -1348,7 +1348,7 @@ static void cgroup_destroy_root(struct cgroup_root *root)
spin_unlock_irq(&css_set_lock);
if (!list_empty(&root->root_list)) {
- list_del(&root->root_list);
+ list_del_rcu(&root->root_list);
cgroup_root_count--;
}
@@ -1401,7 +1401,6 @@ static struct cgroup *cset_cgroup_from_root(struct css_set *cset,
{
struct cgroup *res = NULL;
- lockdep_assert_held(&cgroup_mutex);
lockdep_assert_held(&css_set_lock);
if (cset == &init_css_set) {
@@ -1421,13 +1420,23 @@ static struct cgroup *cset_cgroup_from_root(struct css_set *cset,
}
}
- BUG_ON(!res);
+ /*
+ * If cgroup_mutex is not held, the cgrp_cset_link will be freed
+ * before we remove the cgroup root from the root_list. Consequently,
+ * when accessing a cgroup root, the cset_link may have already been
+ * freed, resulting in a NULL res_cgroup. However, by holding the
+ * cgroup_mutex, we ensure that res_cgroup can't be NULL.
+ * If we don't hold cgroup_mutex in the caller, we must do the NULL
+ * check.
+ */
return res;
}
/*
* Return the cgroup for "task" from the given hierarchy. Must be
- * called with cgroup_mutex and css_set_lock held.
+ * called with css_set_lock held to prevent task's groups from being modified.
+ * Must be called with either cgroup_mutex or rcu read lock to prevent the
+ * cgroup root from being destroyed.
*/
struct cgroup *task_cgroup_from_root(struct task_struct *task,
struct cgroup_root *root)
@@ -2012,7 +2021,7 @@ void init_cgroup_root(struct cgroup_fs_context *ctx)
struct cgroup_root *root = ctx->root;
struct cgroup *cgrp = &root->cgrp;
- INIT_LIST_HEAD(&root->root_list);
+ INIT_LIST_HEAD_RCU(&root->root_list);
atomic_set(&root->nr_cgrps, 1);
cgrp->root = root;
init_cgroup_housekeeping(cgrp);
@@ -2094,7 +2103,7 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask)
* care of subsystems' refcounts, which are explicitly dropped in
* the failure exit path.
*/
- list_add(&root->root_list, &cgroup_roots);
+ list_add_rcu(&root->root_list, &cgroup_roots);
cgroup_root_count++;
/*
--
2.45.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 2/2] cgroup: Move rcu_head up near the top of cgroup_root
2024-12-02 10:11 ` Siddh Raman Pant
@ 2024-12-02 10:11 ` Siddh Raman Pant
2024-12-02 10:17 ` [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe Greg Kroah-Hartman
1 sibling, 0 replies; 29+ messages in thread
From: Siddh Raman Pant @ 2024-12-02 10:11 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sasha Levin, stable, linux-kernel, cgroups, Shivani Agarwal
From: Waiman Long <longman@redhat.com>
commit a7fb0423c201ba12815877a0b5a68a6a1710b23a upstream.
Commit d23b5c577715 ("cgroup: Make operations on the cgroup root_list RCU
safe") adds a new rcu_head to the cgroup_root structure and kvfree_rcu()
for freeing the cgroup_root.
The current implementation of kvfree_rcu(), however, has the limitation
that the offset of the rcu_head structure within the larger data
structure must be less than 4096 or the compilation will fail. See the
macro definition of __is_kvfree_rcu_offset() in include/linux/rcupdate.h
for more information.
By putting rcu_head below the large cgroup structure, any change to the
cgroup structure that makes it larger run the risk of causing build
failure under certain configurations. Commit 77070eeb8821 ("cgroup:
Avoid false cacheline sharing of read mostly rstat_cpu") happens to be
the last straw that breaks it. Fix this problem by moving the rcu_head
structure up before the cgroup structure.
Fixes: d23b5c577715 ("cgroup: Make operations on the cgroup root_list RCU safe")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Closes: https://lore.kernel.org/lkml/20231207143806.114e0a74@canb.auug.org.au/
Signed-off-by: Waiman Long <longman@redhat.com>
Acked-by: Yafang Shao <laoar.shao@gmail.com>
Reviewed-by: Yosry Ahmed <yosryahmed@google.com>
Reviewed-by: Michal Koutný <mkoutny@suse.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
[Shivani: Modified to apply on v5.4.y]
Signed-off-by: Shivani Agarwal <shivani.agarwal@broadcom.com>
Reviewed-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
include/linux/cgroup-defs.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index c64f11674850..f0798d98be8e 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -506,6 +506,10 @@ struct cgroup_root {
/* Unique id for this hierarchy. */
int hierarchy_id;
+ /* A list running through the active hierarchies */
+ struct list_head root_list;
+ struct rcu_head rcu;
+
/* The root cgroup. Root is destroyed on its release. */
struct cgroup cgrp;
@@ -515,10 +519,6 @@ struct cgroup_root {
/* Number of cgroups in the hierarchy, used only for /proc/cgroups */
atomic_t nr_cgrps;
- /* A list running through the active hierarchies */
- struct list_head root_list;
- struct rcu_head rcu;
-
/* Hierarchy-specific flags */
unsigned int flags;
--
2.45.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe
2024-12-02 10:11 ` Siddh Raman Pant
2024-12-02 10:11 ` [PATCH 2/2] cgroup: Move rcu_head up near the top of cgroup_root Siddh Raman Pant
@ 2024-12-02 10:17 ` Greg Kroah-Hartman
2024-12-02 10:26 ` Siddh Raman Pant
1 sibling, 1 reply; 29+ messages in thread
From: Greg Kroah-Hartman @ 2024-12-02 10:17 UTC (permalink / raw)
To: Siddh Raman Pant
Cc: Sasha Levin, stable, linux-kernel, cgroups, Shivani Agarwal
On Mon, Dec 02, 2024 at 03:41:01PM +0530, Siddh Raman Pant wrote:
> From: Yafang Shao <laoar.shao@gmail.com>
>
> commit d23b5c577715892c87533b13923306acc6243f93 upstream.
>
> At present, when we perform operations on the cgroup root_list, we must
> hold the cgroup_mutex, which is a relatively heavyweight lock. In reality,
> we can make operations on this list RCU-safe, eliminating the need to hold
> the cgroup_mutex during traversal. Modifications to the list only occur in
> the cgroup root setup and destroy paths, which should be infrequent in a
> production environment. In contrast, traversal may occur frequently.
> Therefore, making it RCU-safe would be beneficial.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> [fp: adapt to 5.10 mainly because of changes made by e210a89f5b07
> ("cgroup.c: add helper __cset_cgroup_from_root to cleanup duplicated
> codes")]
> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> [Shivani: Modified to apply on v5.4.y]
> Signed-off-by: Shivani Agarwal <shivani.agarwal@broadcom.com>
> Reviewed-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
I'm confused. You do know what signed-off-by means, right? When
sending a patch on, you MUST sign off on it.
Please work with other developers of the Oracle Linux kernel team to get
this right and send it as a whole new series, properly versioned, and
tested in in a format that we can take this in.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe
2024-12-02 10:17 ` [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe Greg Kroah-Hartman
@ 2024-12-02 10:26 ` Siddh Raman Pant
2024-12-02 10:30 ` gregkh
0 siblings, 1 reply; 29+ messages in thread
From: Siddh Raman Pant @ 2024-12-02 10:26 UTC (permalink / raw)
To: gregkh@linuxfoundation.org
Cc: sashal@kernel.org, stable@vger.kernel.org,
linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
shivani.agarwal@broadcom.com
[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]
On Mon, Dec 02 2024 at 15:47:00 +0530, Greg Kroah-Hartman wrote:
> On Mon, Dec 02, 2024 at 03:41:01PM +0530, Siddh Raman Pant wrote:
> > From: Yafang Shao <laoar.shao@gmail.com>
> >
> > commit d23b5c577715892c87533b13923306acc6243f93 upstream.
> >
> > At present, when we perform operations on the cgroup root_list, we must
> > hold the cgroup_mutex, which is a relatively heavyweight lock. In reality,
> > we can make operations on this list RCU-safe, eliminating the need to hold
> > the cgroup_mutex during traversal. Modifications to the list only occur in
> > the cgroup root setup and destroy paths, which should be infrequent in a
> > production environment. In contrast, traversal may occur frequently.
> > Therefore, making it RCU-safe would be beneficial.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > Signed-off-by: Tejun Heo <tj@kernel.org>
> > [fp: adapt to 5.10 mainly because of changes made by e210a89f5b07
> > ("cgroup.c: add helper __cset_cgroup_from_root to cleanup duplicated
> > codes")]
> > Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> > [Shivani: Modified to apply on v5.4.y]
> > Signed-off-by: Shivani Agarwal <shivani.agarwal@broadcom.com>
> > Reviewed-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
>
> I'm confused. You do know what signed-off-by means, right? When
> sending a patch on, you MUST sign off on it.
Even if I'm just *forwarding* the patch already posted on the mailing
list? I just added an r-b for the patch because I reviewed it, and did
no changes.
I'm sorry if I mistook the convention. In that case, the previous
patche emails I had sent has the signoff. I had thought that was
incorrect.
Thanks,
Siddh
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 1/2] cgroup: Make operations on the cgroup root_list RCU safe
2024-12-02 10:26 ` Siddh Raman Pant
@ 2024-12-02 10:30 ` gregkh
0 siblings, 0 replies; 29+ messages in thread
From: gregkh @ 2024-12-02 10:30 UTC (permalink / raw)
To: Siddh Raman Pant
Cc: sashal@kernel.org, stable@vger.kernel.org,
linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
shivani.agarwal@broadcom.com
On Mon, Dec 02, 2024 at 10:26:54AM +0000, Siddh Raman Pant wrote:
> On Mon, Dec 02 2024 at 15:47:00 +0530, Greg Kroah-Hartman wrote:
> > On Mon, Dec 02, 2024 at 03:41:01PM +0530, Siddh Raman Pant wrote:
> > > From: Yafang Shao <laoar.shao@gmail.com>
> > >
> > > commit d23b5c577715892c87533b13923306acc6243f93 upstream.
> > >
> > > At present, when we perform operations on the cgroup root_list, we must
> > > hold the cgroup_mutex, which is a relatively heavyweight lock. In reality,
> > > we can make operations on this list RCU-safe, eliminating the need to hold
> > > the cgroup_mutex during traversal. Modifications to the list only occur in
> > > the cgroup root setup and destroy paths, which should be infrequent in a
> > > production environment. In contrast, traversal may occur frequently.
> > > Therefore, making it RCU-safe would be beneficial.
> > >
> > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > > Signed-off-by: Tejun Heo <tj@kernel.org>
> > > [fp: adapt to 5.10 mainly because of changes made by e210a89f5b07
> > > ("cgroup.c: add helper __cset_cgroup_from_root to cleanup duplicated
> > > codes")]
> > > Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> > > [Shivani: Modified to apply on v5.4.y]
> > > Signed-off-by: Shivani Agarwal <shivani.agarwal@broadcom.com>
> > > Reviewed-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
> >
> > I'm confused. You do know what signed-off-by means, right? When
> > sending a patch on, you MUST sign off on it.
>
> Even if I'm just *forwarding* the patch already posted on the mailing
> list?
Yes.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4.19] cgroup: Make operations on the cgroup root_list RCU safe
2024-09-19 8:47 ` Fedor Pchelkin
` (2 preceding siblings ...)
2024-09-20 9:28 ` Shivani Agarwal
@ 2024-09-20 9:29 ` Shivani Agarwal
2024-09-20 9:29 ` [PATCH v4.19] cgroup: Move rcu_head up near the top of cgroup_root Shivani Agarwal
` (2 subsequent siblings)
6 siblings, 0 replies; 29+ messages in thread
From: Shivani Agarwal @ 2024-09-20 9:29 UTC (permalink / raw)
To: pchelkin, gregkh, stable
Cc: chenridong, gthelen, lvc-project, mkoutny, shivani.agarwal, tj,
lizefan.x, cgroups, ajay.kaher, alexey.makhalov,
vasavi.sirnapalli, Yafang Shao
From: Yafang Shao <laoar.shao@gmail.com>
commit d23b5c577715892c87533b13923306acc6243f93 upstream.
At present, when we perform operations on the cgroup root_list, we must
hold the cgroup_mutex, which is a relatively heavyweight lock. In reality,
we can make operations on this list RCU-safe, eliminating the need to hold
the cgroup_mutex during traversal. Modifications to the list only occur in
the cgroup root setup and destroy paths, which should be infrequent in a
production environment. In contrast, traversal may occur frequently.
Therefore, making it RCU-safe would be beneficial.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
[fp: adapt to 5.10 mainly because of changes made by e210a89f5b07
("cgroup.c: add helper __cset_cgroup_from_root to cleanup duplicated
codes")]
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
[Shivani: Modified to apply on v4.19.y]
Signed-off-by: Shivani Agarwal <shivani.agarwal@broadcom.com>
---
include/linux/cgroup-defs.h | 1 +
kernel/cgroup/cgroup-internal.h | 2 +-
kernel/cgroup/cgroup.c | 23 ++++++++++++++++-------
3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 56442d3b651d..1803c222e204 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -478,6 +478,7 @@ struct cgroup_root {
/* A list running through the active hierarchies */
struct list_head root_list;
+ struct rcu_head rcu;
/* Hierarchy-specific flags */
unsigned int flags;
diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index 4168e7d97e87..b96bbbc4b19c 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -151,7 +151,7 @@ extern struct list_head cgroup_roots;
/* iterate across the hierarchies */
#define for_each_root(root) \
- list_for_each_entry((root), &cgroup_roots, root_list)
+ list_for_each_entry_rcu((root), &cgroup_roots, root_list)
/**
* for_each_subsys - iterate all enabled cgroup subsystems
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 30c058806702..39f5c00cca29 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1246,7 +1246,7 @@ void cgroup_free_root(struct cgroup_root *root)
{
if (root) {
idr_destroy(&root->cgroup_idr);
- kfree(root);
+ kfree_rcu(root, rcu);
}
}
@@ -1280,7 +1280,7 @@ static void cgroup_destroy_root(struct cgroup_root *root)
spin_unlock_irq(&css_set_lock);
if (!list_empty(&root->root_list)) {
- list_del(&root->root_list);
+ list_del_rcu(&root->root_list);
cgroup_root_count--;
}
@@ -1333,7 +1333,6 @@ static struct cgroup *cset_cgroup_from_root(struct css_set *cset,
{
struct cgroup *res = NULL;
- lockdep_assert_held(&cgroup_mutex);
lockdep_assert_held(&css_set_lock);
if (cset == &init_css_set) {
@@ -1353,13 +1352,23 @@ static struct cgroup *cset_cgroup_from_root(struct css_set *cset,
}
}
- BUG_ON(!res);
+ /*
+ * If cgroup_mutex is not held, the cgrp_cset_link will be freed
+ * before we remove the cgroup root from the root_list. Consequently,
+ * when accessing a cgroup root, the cset_link may have already been
+ * freed, resulting in a NULL res_cgroup. However, by holding the
+ * cgroup_mutex, we ensure that res_cgroup can't be NULL.
+ * If we don't hold cgroup_mutex in the caller, we must do the NULL
+ * check.
+ */
return res;
}
/*
* Return the cgroup for "task" from the given hierarchy. Must be
- * called with cgroup_mutex and css_set_lock held.
+ * called with css_set_lock held to prevent task's groups from being modified.
+ * Must be called with either cgroup_mutex or rcu read lock to prevent the
+ * cgroup root from being destroyed.
*/
struct cgroup *task_cgroup_from_root(struct task_struct *task,
struct cgroup_root *root)
@@ -1922,7 +1931,7 @@ void init_cgroup_root(struct cgroup_root *root, struct cgroup_sb_opts *opts)
{
struct cgroup *cgrp = &root->cgrp;
- INIT_LIST_HEAD(&root->root_list);
+ INIT_LIST_HEAD_RCU(&root->root_list);
atomic_set(&root->nr_cgrps, 1);
cgrp->root = root;
init_cgroup_housekeeping(cgrp);
@@ -2004,7 +2013,7 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask, int ref_flags)
* care of subsystems' refcounts, which are explicitly dropped in
* the failure exit path.
*/
- list_add(&root->root_list, &cgroup_roots);
+ list_add_rcu(&root->root_list, &cgroup_roots);
cgroup_root_count++;
/*
--
2.39.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4.19] cgroup: Move rcu_head up near the top of cgroup_root
2024-09-19 8:47 ` Fedor Pchelkin
` (3 preceding siblings ...)
2024-09-20 9:29 ` [PATCH v4.19] " Shivani Agarwal
@ 2024-09-20 9:29 ` Shivani Agarwal
2024-09-20 9:30 ` [PATCH v5.4] cgroup: Make operations on the cgroup root_list RCU safe Shivani Agarwal
2024-09-20 9:33 ` [PATCH v5.4] cgroup: Move rcu_head up near the top of cgroup_root Shivani Agarwal
6 siblings, 0 replies; 29+ messages in thread
From: Shivani Agarwal @ 2024-09-20 9:29 UTC (permalink / raw)
To: pchelkin, gregkh, stable
Cc: chenridong, gthelen, lvc-project, mkoutny, shivani.agarwal, tj,
lizefan.x, cgroups, ajay.kaher, alexey.makhalov,
vasavi.sirnapalli, Waiman Long, Stephen Rothwell, Yafang Shao,
Yosry Ahmed
From: Waiman Long <longman@redhat.com>
commit a7fb0423c201ba12815877a0b5a68a6a1710b23a upstream.
Commit d23b5c577715 ("cgroup: Make operations on the cgroup root_list RCU
safe") adds a new rcu_head to the cgroup_root structure and kvfree_rcu()
for freeing the cgroup_root.
The current implementation of kvfree_rcu(), however, has the limitation
that the offset of the rcu_head structure within the larger data
structure must be less than 4096 or the compilation will fail. See the
macro definition of __is_kvfree_rcu_offset() in include/linux/rcupdate.h
for more information.
By putting rcu_head below the large cgroup structure, any change to the
cgroup structure that makes it larger run the risk of causing build
failure under certain configurations. Commit 77070eeb8821 ("cgroup:
Avoid false cacheline sharing of read mostly rstat_cpu") happens to be
the last straw that breaks it. Fix this problem by moving the rcu_head
structure up before the cgroup structure.
Fixes: d23b5c577715 ("cgroup: Make operations on the cgroup root_list RCU safe")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Closes: https://lore.kernel.org/lkml/20231207143806.114e0a74@canb.auug.org.au/
Signed-off-by: Waiman Long <longman@redhat.com>
Acked-by: Yafang Shao <laoar.shao@gmail.com>
Reviewed-by: Yosry Ahmed <yosryahmed@google.com>
Reviewed-by: Michal Koutný <mkoutny@suse.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
[Shivani: Modified to apply on v4.19.y]
Signed-off-by: Shivani Agarwal <shivani.agarwal@broadcom.com>
---
include/linux/cgroup-defs.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 1803c222e204..4042d9e509a6 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -467,6 +467,10 @@ struct cgroup_root {
/* Unique id for this hierarchy. */
int hierarchy_id;
+ /* A list running through the active hierarchies */
+ struct list_head root_list;
+ struct rcu_head rcu;
+
/* The root cgroup. Root is destroyed on its release. */
struct cgroup cgrp;
@@ -476,10 +480,6 @@ struct cgroup_root {
/* Number of cgroups in the hierarchy, used only for /proc/cgroups */
atomic_t nr_cgrps;
- /* A list running through the active hierarchies */
- struct list_head root_list;
- struct rcu_head rcu;
-
/* Hierarchy-specific flags */
unsigned int flags;
--
2.39.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v5.4] cgroup: Make operations on the cgroup root_list RCU safe
2024-09-19 8:47 ` Fedor Pchelkin
` (4 preceding siblings ...)
2024-09-20 9:29 ` [PATCH v4.19] cgroup: Move rcu_head up near the top of cgroup_root Shivani Agarwal
@ 2024-09-20 9:30 ` Shivani Agarwal
2024-09-20 9:33 ` [PATCH v5.4] cgroup: Move rcu_head up near the top of cgroup_root Shivani Agarwal
6 siblings, 0 replies; 29+ messages in thread
From: Shivani Agarwal @ 2024-09-20 9:30 UTC (permalink / raw)
To: pchelkin, gregkh, stable
Cc: chenridong, gthelen, lvc-project, mkoutny, shivani.agarwal, tj,
lizefan.x, cgroups, ajay.kaher, alexey.makhalov,
vasavi.sirnapalli, Yafang Shao
From: Yafang Shao <laoar.shao@gmail.com>
commit d23b5c577715892c87533b13923306acc6243f93 upstream.
At present, when we perform operations on the cgroup root_list, we must
hold the cgroup_mutex, which is a relatively heavyweight lock. In reality,
we can make operations on this list RCU-safe, eliminating the need to hold
the cgroup_mutex during traversal. Modifications to the list only occur in
the cgroup root setup and destroy paths, which should be infrequent in a
production environment. In contrast, traversal may occur frequently.
Therefore, making it RCU-safe would be beneficial.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
[fp: adapt to 5.10 mainly because of changes made by e210a89f5b07
("cgroup.c: add helper __cset_cgroup_from_root to cleanup duplicated
codes")]
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
[Shivani: Modified to apply on v5.4.y]
Signed-off-by: Shivani Agarwal <shivani.agarwal@broadcom.com>
---
include/linux/cgroup-defs.h | 1 +
kernel/cgroup/cgroup-internal.h | 3 ++-
kernel/cgroup/cgroup.c | 23 ++++++++++++++++-------
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index d15884957e7f..c64f11674850 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -517,6 +517,7 @@ struct cgroup_root {
/* A list running through the active hierarchies */
struct list_head root_list;
+ struct rcu_head rcu;
/* Hierarchy-specific flags */
unsigned int flags;
diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index 803989eae99e..bb85acc1114e 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -172,7 +172,8 @@ extern struct list_head cgroup_roots;
/* iterate across the hierarchies */
#define for_each_root(root) \
- list_for_each_entry((root), &cgroup_roots, root_list)
+ list_for_each_entry_rcu((root), &cgroup_roots, root_list, \
+ lockdep_is_held(&cgroup_mutex))
/**
* for_each_subsys - iterate all enabled cgroup subsystems
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 16ae86894121..dc6351095baf 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1314,7 +1314,7 @@ void cgroup_free_root(struct cgroup_root *root)
{
if (root) {
idr_destroy(&root->cgroup_idr);
- kfree(root);
+ kfree_rcu(root, rcu);
}
}
@@ -1348,7 +1348,7 @@ static void cgroup_destroy_root(struct cgroup_root *root)
spin_unlock_irq(&css_set_lock);
if (!list_empty(&root->root_list)) {
- list_del(&root->root_list);
+ list_del_rcu(&root->root_list);
cgroup_root_count--;
}
@@ -1401,7 +1401,6 @@ static struct cgroup *cset_cgroup_from_root(struct css_set *cset,
{
struct cgroup *res = NULL;
- lockdep_assert_held(&cgroup_mutex);
lockdep_assert_held(&css_set_lock);
if (cset == &init_css_set) {
@@ -1421,13 +1420,23 @@ static struct cgroup *cset_cgroup_from_root(struct css_set *cset,
}
}
- BUG_ON(!res);
+ /*
+ * If cgroup_mutex is not held, the cgrp_cset_link will be freed
+ * before we remove the cgroup root from the root_list. Consequently,
+ * when accessing a cgroup root, the cset_link may have already been
+ * freed, resulting in a NULL res_cgroup. However, by holding the
+ * cgroup_mutex, we ensure that res_cgroup can't be NULL.
+ * If we don't hold cgroup_mutex in the caller, we must do the NULL
+ * check.
+ */
return res;
}
/*
* Return the cgroup for "task" from the given hierarchy. Must be
- * called with cgroup_mutex and css_set_lock held.
+ * called with css_set_lock held to prevent task's groups from being modified.
+ * Must be called with either cgroup_mutex or rcu read lock to prevent the
+ * cgroup root from being destroyed.
*/
struct cgroup *task_cgroup_from_root(struct task_struct *task,
struct cgroup_root *root)
@@ -2012,7 +2021,7 @@ void init_cgroup_root(struct cgroup_fs_context *ctx)
struct cgroup_root *root = ctx->root;
struct cgroup *cgrp = &root->cgrp;
- INIT_LIST_HEAD(&root->root_list);
+ INIT_LIST_HEAD_RCU(&root->root_list);
atomic_set(&root->nr_cgrps, 1);
cgrp->root = root;
init_cgroup_housekeeping(cgrp);
@@ -2094,7 +2103,7 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask)
* care of subsystems' refcounts, which are explicitly dropped in
* the failure exit path.
*/
- list_add(&root->root_list, &cgroup_roots);
+ list_add_rcu(&root->root_list, &cgroup_roots);
cgroup_root_count++;
/*
--
2.39.4
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v5.4] cgroup: Move rcu_head up near the top of cgroup_root
2024-09-19 8:47 ` Fedor Pchelkin
` (5 preceding siblings ...)
2024-09-20 9:30 ` [PATCH v5.4] cgroup: Make operations on the cgroup root_list RCU safe Shivani Agarwal
@ 2024-09-20 9:33 ` Shivani Agarwal
6 siblings, 0 replies; 29+ messages in thread
From: Shivani Agarwal @ 2024-09-20 9:33 UTC (permalink / raw)
To: pchelkin, gregkh, stable
Cc: chenridong, gthelen, lvc-project, mkoutny, shivani.agarwal, tj,
lizefan.x, cgroups, ajay.kaher, alexey.makhalov,
vasavi.sirnapalli, Waiman Long, Stephen Rothwell, Yafang Shao,
Yosry Ahmed
From: Waiman Long <longman@redhat.com>
commit a7fb0423c201ba12815877a0b5a68a6a1710b23a upstream.
Commit d23b5c577715 ("cgroup: Make operations on the cgroup root_list RCU
safe") adds a new rcu_head to the cgroup_root structure and kvfree_rcu()
for freeing the cgroup_root.
The current implementation of kvfree_rcu(), however, has the limitation
that the offset of the rcu_head structure within the larger data
structure must be less than 4096 or the compilation will fail. See the
macro definition of __is_kvfree_rcu_offset() in include/linux/rcupdate.h
for more information.
By putting rcu_head below the large cgroup structure, any change to the
cgroup structure that makes it larger run the risk of causing build
failure under certain configurations. Commit 77070eeb8821 ("cgroup:
Avoid false cacheline sharing of read mostly rstat_cpu") happens to be
the last straw that breaks it. Fix this problem by moving the rcu_head
structure up before the cgroup structure.
Fixes: d23b5c577715 ("cgroup: Make operations on the cgroup root_list RCU safe")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Closes: https://lore.kernel.org/lkml/20231207143806.114e0a74@canb.auug.org.au/
Signed-off-by: Waiman Long <longman@redhat.com>
Acked-by: Yafang Shao <laoar.shao@gmail.com>
Reviewed-by: Yosry Ahmed <yosryahmed@google.com>
Reviewed-by: Michal Koutný <mkoutny@suse.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
[Shivani: Modified to apply on v5.4.y]
Signed-off-by: Shivani Agarwal <shivani.agarwal@broadcom.com>
---
include/linux/cgroup-defs.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 1803c222e204..4042d9e509a6 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -467,6 +467,10 @@ struct cgroup_root {
/* Unique id for this hierarchy. */
int hierarchy_id;
+ /* A list running through the active hierarchies */
+ struct list_head root_list;
+ struct rcu_head rcu;
+
/* The root cgroup. Root is destroyed on its release. */
struct cgroup cgrp;
@@ -476,10 +480,6 @@ struct cgroup_root {
/* Number of cgroups in the hierarchy, used only for /proc/cgroups */
atomic_t nr_cgrps;
- /* A list running through the active hierarchies */
- struct list_head root_list;
- struct rcu_head rcu;
-
/* Hierarchy-specific flags */
unsigned int flags;
--
2.39.4
^ permalink raw reply related [flat|nested] 29+ messages in thread