SELinux: Fix a RCU free problem with the netport cache

The netport cache doesn't free resources in a manner which is safe or orderly. This patch fixes this by adding in a missing call to rcu_dereference() in sel_netport_insert() as well as some general cleanup throughout the file. Signed-off-by: Paul Moore <paul.moore@hp.com> Signed-off-by: James Morris <jmorris@namei.org>

SELinux: Fix a RCU free problem with the netport cache
The netport cache doesn't free resources in a manner which is safe or orderly. This patch fixes this by adding in a missing call to rcu_dereference() in sel_netport_insert() as well as some general cleanup throughout the file. Signed-off-by: Paul Moore <paul.moore@hp.com> Signed-off-by: James Morris <jmorris@namei.org>
Paul Moore · James Morris
1 parent a639e7ca8e
Showing 1 changed file with 18 additions and 22 deletions Side-by-side Diff
security/selinux/netport.c
@@ -114,8 +114,7 @@
  
 	idx = sel_netport_hashfn(pnum);
 	list_for_each_entry_rcu(port, &sel_netport_hash[idx].list, list)
-		if (port->psec.port == pnum &&
-		    port->psec.protocol == protocol)
+		if (port->psec.port == pnum && port->psec.protocol == protocol)
 			return port;
  
 	return NULL;
  
@@ -126,11 +125,10 @@
  * @port: the new port record
  *
  * Description:
- * Add a new port record to the network address hash table.  Returns zero on
- * success, negative values on failure.
+ * Add a new port record to the network address hash table.
  *
  */
-static int sel_netport_insert(struct sel_netport *port)
+static void sel_netport_insert(struct sel_netport *port)
 {
 	unsigned int idx;
  
  
@@ -140,13 +138,13 @@
 	list_add_rcu(&port->list, &sel_netport_hash[idx].list);
 	if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
 		struct sel_netport *tail;
-		tail = list_entry(port->list.prev, struct sel_netport, list);
-		list_del_rcu(port->list.prev);
+		tail = list_entry(
+			rcu_dereference(sel_netport_hash[idx].list.prev),
+			struct sel_netport, list);
+		list_del_rcu(&tail->list);
 		call_rcu(&tail->rcu, sel_netport_free);
 	} else
 		sel_netport_hash[idx].size++;
-
-	return 0;
 }
  
 /**
@@ -163,7 +161,7 @@
  */
 static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
 {
-	int ret;
+	int ret = -ENOMEM;
 	struct sel_netport *port;
 	struct sel_netport *new = NULL;
  
  
  
  
  
@@ -171,23 +169,20 @@
 	port = sel_netport_find(protocol, pnum);
 	if (port != NULL) {
 		*sid = port->psec.sid;
-		ret = 0;
-		goto out;
+		spin_unlock_bh(&sel_netport_lock);
+		return 0;
 	}
 	new = kzalloc(sizeof(*new), GFP_ATOMIC);
-	if (new == NULL) {
-		ret = -ENOMEM;
+	if (new == NULL)
 		goto out;
-	}
-	ret = security_port_sid(protocol, pnum, &new->psec.sid);
+	ret = security_port_sid(protocol, pnum, sid);
 	if (ret != 0)
 		goto out;
+
 	new->psec.port = pnum;
 	new->psec.protocol = protocol;
-	ret = sel_netport_insert(new);
-	if (ret != 0)
-		goto out;
-	*sid = new->psec.sid;
+	new->psec.sid = *sid;
+	sel_netport_insert(new);
  
 out:
 	spin_unlock_bh(&sel_netport_lock);
  
@@ -239,11 +234,12 @@
 static void sel_netport_flush(void)
 {
 	unsigned int idx;
-	struct sel_netport *port;
+	struct sel_netport *port, *port_tmp;
  
 	spin_lock_bh(&sel_netport_lock);
 	for (idx = 0; idx < SEL_NETPORT_HASH_SIZE; idx++) {
-		list_for_each_entry(port, &sel_netport_hash[idx].list, list) {
+		list_for_each_entry_safe(port, port_tmp,
+					 &sel_netport_hash[idx].list, list) {
 			list_del_rcu(&port->list);
 			call_rcu(&port->rcu, sel_netport_free);
 		}
...	...	@@ -114,8 +114,7 @@
114	114
115	115	idx = sel_netport_hashfn(pnum);
116	116	list_for_each_entry_rcu(port, &sel_netport_hash[idx].list, list)
117		- if (port->psec.port == pnum &&
118		- port->psec.protocol == protocol)
	117	+ if (port->psec.port == pnum && port->psec.protocol == protocol)
119	118	return port;
120	119
121	120	return NULL;
122	121
...	...	@@ -126,11 +125,10 @@
126	125	* @port: the new port record
127	126	*
128	127	* Description:
129		- * Add a new port record to the network address hash table. Returns zero on
130		- * success, negative values on failure.
	128	+ * Add a new port record to the network address hash table.
131	129	*
132	130	*/
133		-static int sel_netport_insert(struct sel_netport *port)
	131	+static void sel_netport_insert(struct sel_netport *port)
134	132	{
135	133	unsigned int idx;
136	134
137	135
...	...	@@ -140,13 +138,13 @@
140	138	list_add_rcu(&port->list, &sel_netport_hash[idx].list);
141	139	if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
142	140	struct sel_netport *tail;
143		- tail = list_entry(port->list.prev, struct sel_netport, list);
144		- list_del_rcu(port->list.prev);
	141	+ tail = list_entry(
	142	+ rcu_dereference(sel_netport_hash[idx].list.prev),
	143	+ struct sel_netport, list);
	144	+ list_del_rcu(&tail->list);
145	145	call_rcu(&tail->rcu, sel_netport_free);
146	146	} else
147	147	sel_netport_hash[idx].size++;
148		-
149		- return 0;
150	148	}
151	149
152	150	/**
...	...	@@ -163,7 +161,7 @@
163	161	*/
164	162	static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
165	163	{
166		- int ret;
	164	+ int ret = -ENOMEM;
167	165	struct sel_netport *port;
168	166	struct sel_netport *new = NULL;
169	167
170	168
171	169
172	170
173	171
...	...	@@ -171,23 +169,20 @@
171	169	port = sel_netport_find(protocol, pnum);
172	170	if (port != NULL) {
173	171	*sid = port->psec.sid;
174		- ret = 0;
175		- goto out;
	172	+ spin_unlock_bh(&sel_netport_lock);
	173	+ return 0;
176	174	}
177	175	new = kzalloc(sizeof(*new), GFP_ATOMIC);
178		- if (new == NULL) {
179		- ret = -ENOMEM;
	176	+ if (new == NULL)
180	177	goto out;
181		- }
182		- ret = security_port_sid(protocol, pnum, &new->psec.sid);
	178	+ ret = security_port_sid(protocol, pnum, sid);
183	179	if (ret != 0)
184	180	goto out;
	181	+
185	182	new->psec.port = pnum;
186	183	new->psec.protocol = protocol;
187		- ret = sel_netport_insert(new);
188		- if (ret != 0)
189		- goto out;
190		- *sid = new->psec.sid;
	184	+ new->psec.sid = *sid;
	185	+ sel_netport_insert(new);
191	186
192	187	out:
193	188	spin_unlock_bh(&sel_netport_lock);
194	189
...	...	@@ -239,11 +234,12 @@
239	234	static void sel_netport_flush(void)
240	235	{
241	236	unsigned int idx;
242		- struct sel_netport *port;
	237	+ struct sel_netport port, port_tmp;
243	238
244	239	spin_lock_bh(&sel_netport_lock);
245	240	for (idx = 0; idx < SEL_NETPORT_HASH_SIZE; idx++) {
246		- list_for_each_entry(port, &sel_netport_hash[idx].list, list) {
	241	+ list_for_each_entry_safe(port, port_tmp,
	242	+ &sel_netport_hash[idx].list, list) {
247	243	list_del_rcu(&port->list);
248	244	call_rcu(&port->rcu, sel_netport_free);
249	245	}