Patchwork [net,1/2] caif: Bugfix list_del_rcu race in cfmuxl_ctrlcmd.

login
register
mail settings
Submitter sjur.brandeland@stericsson.com
Date Feb. 2, 2012, 11:21 a.m.
Message ID <1328181663-13853-1-git-send-email-sjur.brandeland@stericsson.com>
Download mbox | patch
Permalink /patch/139120/
State Accepted
Delegated to: David Miller
Headers show

Comments

sjur.brandeland@stericsson.com - Feb. 2, 2012, 11:21 a.m.
Always use cfmuxl_remove_uplayer when removing a up-layer.
cfmuxl_ctrlcmd() can be called independently and in parallel with
cfmuxl_remove_uplayer(). The race between them could cause list_del_rcu
to be called on a node which has been already taken out from the list.
That lead to a (rare) crash on accessing poisoned node->prev inside
list_del_rcu.

This fix ensures that deletion are done holding the same lock.

Reported-by: Dmitry Tarnyagin <dmitry.tarnyagin@stericsson.com>
Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 net/caif/cfmuxl.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)
David Miller - Feb. 2, 2012, 7:31 p.m.
From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Thu,  2 Feb 2012 12:21:02 +0100

> Always use cfmuxl_remove_uplayer when removing a up-layer.
> cfmuxl_ctrlcmd() can be called independently and in parallel with
> cfmuxl_remove_uplayer(). The race between them could cause list_del_rcu
> to be called on a node which has been already taken out from the list.
> That lead to a (rare) crash on accessing poisoned node->prev inside
> list_del_rcu.
> 
> This fix ensures that deletion are done holding the same lock.
> 
> Reported-by: Dmitry Tarnyagin <dmitry.tarnyagin@stericsson.com>
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/caif/cfmuxl.c b/net/caif/cfmuxl.c
index b36f24a..94b0861 100644
--- a/net/caif/cfmuxl.c
+++ b/net/caif/cfmuxl.c
@@ -248,7 +248,6 @@  static void cfmuxl_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
 {
 	struct cfmuxl *muxl = container_obj(layr);
 	struct cflayer *layer;
-	int idx;
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(layer, &muxl->srvl_list, node) {
@@ -257,14 +256,9 @@  static void cfmuxl_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
 
 			if ((ctrl == _CAIF_CTRLCMD_PHYIF_DOWN_IND ||
 				ctrl == CAIF_CTRLCMD_REMOTE_SHUTDOWN_IND) &&
-					layer->id != 0) {
-
-				idx = layer->id % UP_CACHE_SIZE;
-				spin_lock_bh(&muxl->receive_lock);
-				RCU_INIT_POINTER(muxl->up_cache[idx], NULL);
-				list_del_rcu(&layer->node);
-				spin_unlock_bh(&muxl->receive_lock);
-			}
+					layer->id != 0)
+				cfmuxl_remove_uplayer(layr, layer->id);
+
 			/* NOTE: ctrlcmd is not allowed to block */
 			layer->ctrlcmd(layer, ctrl, phyid);
 		}