* [PATCH net 1/2] caif: Bugfix list_del_rcu race in cfmuxl_ctrlcmd.
@ 2012-02-02 11:21 Sjur Brændeland
2012-02-02 11:21 ` [PATCH net 2/2] caif: Bugfix double kfree_skb upon xmit failure Sjur Brændeland
2012-02-02 19:31 ` [PATCH net 1/2] caif: Bugfix list_del_rcu race in cfmuxl_ctrlcmd David Miller
0 siblings, 2 replies; 4+ messages in thread
From: Sjur Brændeland @ 2012-02-02 11:21 UTC (permalink / raw)
To: netdev, davem; +Cc: sjurbren, Sjur Brændeland
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(-)
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);
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH net 2/2] caif: Bugfix double kfree_skb upon xmit failure
2012-02-02 11:21 [PATCH net 1/2] caif: Bugfix list_del_rcu race in cfmuxl_ctrlcmd Sjur Brændeland
@ 2012-02-02 11:21 ` Sjur Brændeland
2012-02-02 19:31 ` David Miller
2012-02-02 19:31 ` [PATCH net 1/2] caif: Bugfix list_del_rcu race in cfmuxl_ctrlcmd David Miller
1 sibling, 1 reply; 4+ messages in thread
From: Sjur Brændeland @ 2012-02-02 11:21 UTC (permalink / raw)
To: netdev, davem; +Cc: sjurbren, Dmitry Tarnyagin, Sjur Brændeland
From: Dmitry Tarnyagin <dmitry.tarnyagin@stericsson.com>
SKB is freed twice upon send error. The Network stack consumes SKB even
when it returns error code.
Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
net/caif/caif_socket.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index a986280..a97d97a 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -539,8 +539,10 @@ static int transmit_skb(struct sk_buff *skb, struct caifsock *cf_sk,
pkt = cfpkt_fromnative(CAIF_DIR_OUT, skb);
memset(skb->cb, 0, sizeof(struct caif_payload_info));
- if (cf_sk->layer.dn == NULL)
+ if (cf_sk->layer.dn == NULL) {
+ kfree_skb(skb);
return -EINVAL;
+ }
return cf_sk->layer.dn->transmit(cf_sk->layer.dn, pkt);
}
@@ -683,10 +685,10 @@ static int caif_stream_sendmsg(struct kiocb *kiocb, struct socket *sock,
}
err = transmit_skb(skb, cf_sk,
msg->msg_flags&MSG_DONTWAIT, timeo);
- if (err < 0) {
- kfree_skb(skb);
+ if (err < 0)
+ /* skb is already freed */
goto pipe_err;
- }
+
sent += size;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net 1/2] caif: Bugfix list_del_rcu race in cfmuxl_ctrlcmd.
2012-02-02 11:21 [PATCH net 1/2] caif: Bugfix list_del_rcu race in cfmuxl_ctrlcmd Sjur Brændeland
2012-02-02 11:21 ` [PATCH net 2/2] caif: Bugfix double kfree_skb upon xmit failure Sjur Brændeland
@ 2012-02-02 19:31 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2012-02-02 19:31 UTC (permalink / raw)
To: sjur.brandeland; +Cc: netdev, sjurbren
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-02-02 19:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-02 11:21 [PATCH net 1/2] caif: Bugfix list_del_rcu race in cfmuxl_ctrlcmd Sjur Brændeland
2012-02-02 11:21 ` [PATCH net 2/2] caif: Bugfix double kfree_skb upon xmit failure Sjur Brændeland
2012-02-02 19:31 ` David Miller
2012-02-02 19:31 ` [PATCH net 1/2] caif: Bugfix list_del_rcu race in cfmuxl_ctrlcmd David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).