From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: [PATCH cgroup/for-3.12 1/2] cgroup: fix subsystem file accesses on the root cgroup
Date: Thu, 15 Aug 2013 11:42:36 -0400 [thread overview]
Message-ID: <20130815154236.GG14606@htj.dyndns.org> (raw)
From af8e64db96406f91746711260717c11584f90efd Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Date: Thu, 15 Aug 2013 11:37:54 -0400
105347ba5 ("cgroup: make cgroup_file_open() rcu_read_lock() around
cgroup_css() and add cfent->css") added cfent->css to cache the
associted cgroup_subsys_state across file operations.
A cfent is associated with single css throughout its lifetime and the
origimal commit initialized the cache pointer during cgroup_add_file()
and verified that it matches the actual one in cgroup_file_open().
While this works fine for !root cgroups, it's broken for root cgroups
as files in a root cgroup are created before the css's are associated
with the cgroup and thus cgroup_css() call in cgroup_add_file()
returns NULL associating all cfents in the root cgroup with NULL css.
This makes cgroup_file_open() trigger WARN and fail with -ENODEV for
all !core subsystem files in the root cgroups.
There's no reason to initialize cfent->css separately from
cgroup_add_file(). As the association never changes,
cgroup_file_open() can set it unconditionally every time and
containing the logic in cgroup_file_open() makes more sense anyway as
the only reason it's necessary is file->private_data being already
occupied.
Fix it by setting cfent->css unconditionally from cgroup_file_open().
Signed-off-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
kernel/cgroup.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 66d0107..ab2a23f 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2490,10 +2490,18 @@ static int cgroup_file_open(struct inode *inode, struct file *file)
}
rcu_read_unlock();
- /* css should match @cfe->css, see cgroup_add_file() for details */
- if (!css || WARN_ON_ONCE(css != cfe->css))
+ if (!css)
return -ENODEV;
+ /*
+ * @cfe->css is used by read/write/close to determine the
+ * associated css. @file->private_data would be a better place but
+ * that's already used by seqfile. Multiple accessors may use it
+ * simultaneously which is okay as the association never changes.
+ */
+ WARN_ON_ONCE(cfe->css && cfe->css != css);
+ cfe->css = css;
+
if (cft->read_map || cft->read_seq_string) {
file->f_op = &cgroup_seqfile_operations;
err = single_open(file, cgroup_seqfile_show, cfe);
@@ -2772,18 +2780,6 @@ static int cgroup_add_file(struct cgroup *cgrp, struct cftype *cft)
dentry->d_fsdata = cfe;
simple_xattrs_init(&cfe->xattrs);
- /*
- * cfe->css is used by read/write/close to determine the associated
- * css. file->private_data would be a better place but that's
- * already used by seqfile. Note that open will use the usual
- * cgroup_css() and css_tryget() to acquire the css and this
- * caching doesn't affect css lifetime management.
- */
- if (cft->ss)
- cfe->css = cgroup_css(cgrp, cft->ss->subsys_id);
- else
- cfe->css = &cgrp->dummy_css;
-
mode = cgroup_file_mode(cft);
error = cgroup_create_file(dentry, mode | S_IFREG, cgrp->root->sb);
if (!error) {
--
1.8.3.1
next reply other threads:[~2013-08-15 15:42 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-15 15:42 Tejun Heo [this message]
[not found] ` <20130815154236.GG14606-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2013-08-15 15:43 ` [PATCH cgroup/for-3.12 2/2] cgroup: fix cgroup_write_event_control() Tejun Heo
[not found] ` <20130815154315.GH14606-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2013-08-19 2:41 ` Li Zefan
[not found] ` <521185F3.6060806-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2013-08-19 13:57 ` Tejun Heo
2013-08-19 2:23 ` [PATCH cgroup/for-3.12 1/2] cgroup: fix subsystem file accesses on the root cgroup Li Zefan
[not found] ` <521181AD.7090700-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2013-08-19 13:57 ` Tejun Heo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130815154236.GG14606@htj.dyndns.org \
--to=tj-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
--cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
--cc=lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox