All of lore.kernel.org
 help / color / mirror / Atom feed
From: Li Zefan <lizf@cn.fujitsu.com>
To: David Miller <davem@davemloft.net>
Cc: netdev <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Cgroups <cgroups@vger.kernel.org>,
	Neil Horman <nhorman@tuxdriver.com>
Subject: [PATCH 3/6] netprio_cgroup: fix wrong memory access when NETPRIO_CGROUP=m
Date: Wed, 01 Feb 2012 14:55:46 +0800	[thread overview]
Message-ID: <4F28E1F2.70805@cn.fujitsu.com> (raw)
In-Reply-To: <4F28E1D1.900@cn.fujitsu.com>

When the netprio_cgroup module is not loaded, net_prio_subsys_id
is -1, and so sock_update_prioidx() accesses cgroup_subsys array
with negative index subsys[-1].

Make the code resembles cls_cgroup code, which is bug free.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 include/net/netprio_cgroup.h |   48 +++++++++++++++++++++++++++++++++++-------
 net/core/sock.c              |    7 +----
 2 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/include/net/netprio_cgroup.h b/include/net/netprio_cgroup.h
index 7b2d431..d58fdec 100644
--- a/include/net/netprio_cgroup.h
+++ b/include/net/netprio_cgroup.h
@@ -37,19 +37,51 @@ extern int net_prio_subsys_id;
 
 extern void sock_update_netprioidx(struct sock *sk);
 
-static inline struct cgroup_netprio_state
-		*task_netprio_state(struct task_struct *p)
+#if IS_BUILTIN(CONFIG_NETPRIO_CGROUP)
+
+static inline u32 task_netprioidx(struct task_struct *p)
 {
-#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
-	return container_of(task_subsys_state(p, net_prio_subsys_id),
-			    struct cgroup_netprio_state, css);
-#else
-	return NULL;
-#endif
+	struct cgroup_netprio_state *state;
+	u32 idx;
+
+	rcu_read_lock();
+	state = container_of(task_subsys_state(p, net_prio_subsys_id),
+			     struct cgroup_netprio_state, css);
+	idx = state->prioidx;
+	rcu_read_unlock();
+	return idx;
+}
+
+#elif IS_MODULE(CONFIG_NETPRIO_CGROUP)
+
+static inline u32 task_netprioidx(struct task_struct *p)
+{
+	struct cgroup_netprio_state *state;
+	int subsys_id;
+	u32 idx = 0;
+
+	rcu_read_lock();
+	subsys_id = rcu_dereference_index_check(net_prio_subsys_id,
+						rcu_read_lock_held());
+	if (subsys_id >= 0) {
+		state = container_of(task_subsys_state(p, subsys_id),
+				     struct cgroup_netprio_state, css);
+		idx = state->prioidx;
+	}
+	rcu_read_unlock();
+	return idx;
 }
 
 #else
 
+static inline u32 task_netprioidx(struct task_struct *p)
+{
+	return 0;
+}
+
+#endif /* CONFIG_NETPRIO_CGROUP */
+
+#else
 #define sock_update_netprioidx(sk)
 #endif
 
diff --git a/net/core/sock.c b/net/core/sock.c
index 3e81fd2..02f8dfe 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1171,13 +1171,10 @@ EXPORT_SYMBOL(sock_update_classid);
 
 void sock_update_netprioidx(struct sock *sk)
 {
-	struct cgroup_netprio_state *state;
 	if (in_interrupt())
 		return;
-	rcu_read_lock();
-	state = task_netprio_state(current);
-	sk->sk_cgrp_prioidx = state ? state->prioidx : 0;
-	rcu_read_unlock();
+
+	sk->sk_cgrp_prioidx = task_netprioidx(current);
 }
 EXPORT_SYMBOL_GPL(sock_update_netprioidx);
 #endif
-- 
1.7.3.1

  reply	other threads:[~2012-02-01  6:55 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-01  6:55 [PATCH 1/6] netprio_cgroup: fix an off-by-one bug Li Zefan
2012-02-01  6:55 ` Li Zefan
2012-02-01  6:55 ` Li Zefan [this message]
2012-02-01  6:56 ` [PATCH 4/6] netprio_cgroup: use IS_ENABLED() and family Li Zefan
     [not found]   ` <4F28E203.10502-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2012-02-01  6:59     ` David Miller
2012-02-01  6:59       ` David Miller
     [not found]       ` <20120201.015954.2098592611164231843.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-01  7:06         ` Li Zefan
2012-02-01  7:06           ` Li Zefan
     [not found]           ` <4F28E48D.3040505-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2012-02-01  7:07             ` David Miller
2012-02-01  7:07               ` David Miller
2012-02-01  7:22               ` Li Zefan
     [not found]                 ` <4F28E853.7000104-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2012-02-01 12:02                   ` Neil Horman
2012-02-01 12:02                     ` Neil Horman
     [not found] ` <4F28E1D1.900-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2012-02-01  6:55   ` [PATCH 2/6] netprio_cgroup: don't allocate prio table when a device is registered Li Zefan
2012-02-01  6:55     ` Li Zefan
2012-02-01  6:56   ` [PATCH 5/6] cls_cgroup: use IS_ENABLED() and family Li Zefan
2012-02-01  6:56     ` Li Zefan
2012-02-01  6:56 ` [PATCH 6/6] cls_cgroup: remove redundant rcu_read_lock/unlock Li Zefan
     [not found]   ` <4F28E22A.703-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2012-02-01  7:07     ` Eric Dumazet
2012-02-01  7:07       ` Eric Dumazet
2012-02-01  7:10       ` David Miller
2012-02-01  7:10         ` David Miller
2012-02-01  7:20       ` Li Zefan
2012-02-01  7:20         ` Li Zefan
     [not found]         ` <4F28E7A0.6000309-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2012-02-01  7:23           ` Herbert Xu
2012-02-01  7:23             ` Herbert Xu

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=4F28E1F2.70805@cn.fujitsu.com \
    --to=lizf@cn.fujitsu.com \
    --cc=cgroups@vger.kernel.org \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.