cgroup: avoid accessing modular cgroup subsys structure without locking
PATCH v2 1/1 cgroup: avoid accessing modular cgroup subsys structure without locking
subsys[i] is set to NULL in cgroup_unload_subsys() at modular unload, and that's protected by cgroup_mutex, and then the memory *subsys[i] resides will be freed.
So this is unsafe without any locking:
if (!ss || ss->module) ...
v2:
- add a comment for enum cgroup_subsys_id
- simplify the comment in cgroup_exit()
Signed-off-by Li Zefan <[email protected]>
--- include/linux/cgroup.h | 17 ++++++++++++++--- kernel/cgroup.c | 28 ++++++++++++++-------------- 2 files changed, 28 insertions(+), 17 deletions(-)
include/linux/cgroup.h +14 -3
@@ -44,14 +44,25 @@ extern void cgroup_unload_subsys(struct cgroup_subsys *ss);extern const struct file_operations proc_cgroup_operations; -/* Define the enumeration of all builtin cgroup subsystems */ +/* +* Define the enumeration of all cgroup subsystems. +* +* We define ids for builtin subsystems and then modular ones. +*/ #define SUBSYS (_x) _x ## _subsys_id, -#define IS_SUBSYS_ENABLED (option) IS_ENABLED(option) enum cgroup_subsys_id { +#define IS_SUBSYS_ENABLED(option) IS_BUILTIN(option) +#include <linux/cgroup_subsys.h> +#undef IS_SUBSYS_ENABLED +CGROUP_BUILTIN_SUBSYS_COUNT , + + __CGROUP_SUBSYS_TEMP_PLACEHOLDER =CGROUP_BUILTIN_SUBSYS_COUNT -1 , + +#define IS_SUBSYS_ENABLED(option) IS_MODULE(option)#include <linux/cgroup_subsys.h> +#undef IS_SUBSYS_ENABLED CGROUP_SUBSYS_COUNT, }; -#undef IS_SUBSYS_ENABLED#undef SUBSYS/* Per-subsystem/per-cgroup state maintained by the system. */
kernel/cgroup.c +14 -14
@@ -4940,17 +4940,17 @@ void cgroup_post_fork(struct task_struct *child)* and addition to css_set. */ if (need_forkexit_callback) { -for (i =0 ; i <CGROUP_SUBSYS_COUNT ; i++) { +/* +* fork/exit callbacks are supported only for builtin +* subsystems, and the builtin section of the subsys +* array is immutable, so we don't need to lock the +* subsys array here. On the other hand, modular section +* of the array can be freed at module unload, so we +* can't touch that. +*/ +for (i =0 ; i <CGROUP_BUILTIN_SUBSYS_COUNT ; i++) {struct cgroup_subsys *ss = subsys[i]; -/* -* fork/exit callbacks are supported only for -* builtin subsystems and we don't need further -* synchronization as they never go away. -*/ -if (!ss || ss->module) -continue ; -if (ss->fork) ss->fork (child); }
@@ -5015,13 +5015,13 @@ void cgroup_exit(struct task_struct *tsk, int run_callbacks) tsk->cgroups = &init_css_set;if (run_callbacks && need_forkexit_callback) { -for (i =0 ; i <CGROUP_SUBSYS_COUNT ; i++) { +/* +* fork/exit callbacks are supported only for builtin +* subsystems, see cgroup_post_fork() for details. +*/ +for (i =0 ; i <CGROUP_BUILTIN_SUBSYS_COUNT ; i++) {struct cgroup_subsys *ss = subsys[i]; -/* modular subsystems can't use callbacks */ -if (!ss || ss->module) -continue ; -if (ss->exit) {struct cgroup *old_cgrp =rcu_dereference_raw (cg->subsys[i])->cgroup;
Signature · 1 line
1.8.0.2
Quoted 6 lines · Li Zefan
subsys[i] is set to NULL in
cgroup_unload_subsys()at modular unload, and that's protected bycgroup_mutex, and then the memory *subsys[i] resides will be freed.So this is unsafe without any locking:
Quoted 8 lines
if (!ss || ss->module) ...v2:
- add a comment for enumcgroup_subsys_id
- simplify the comment incgroup_exit()Signed-off-by Li Zefan <[email protected]>
Applied to cgroup/for-3.10.
Thanks.
Signature · 1 line
tejun