| Message ID | 20181227020142.18190-1-baijiaju1990@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2] net: arcnet: Fix a possible concurrency use-after-free bug in arcnet_reply_tasklet() | expand |
From: Jia-Ju Bai <baijiaju1990@gmail.com> Date: Thu, 27 Dec 2018 10:01:42 +0800 > @@ -401,6 +401,7 @@ static void arcnet_reply_tasklet(unsigned long data) > struct sock_exterr_skb *serr; > struct sock *sk; > int ret; > + unsigned long flags; > > local_irq_disable(); > skb = lp->outgoing.skb; > @@ -426,10 +427,14 @@ static void arcnet_reply_tasklet(unsigned long data) > serr->ee.ee_data = skb_shinfo(skb)->tskey; > serr->ee.ee_info = lp->reply_status; > > + spin_lock_irqsave(&lp->lock, flags); > + > /* finally erasing outgoing skb */ > dev_kfree_skb(lp->outgoing.skb); > lp->outgoing.skb = NULL; > > + spin_unlock_irqrestore(&lp->lock, flags); > + > ackskb->dev = lp->dev; > > ret = sock_queue_err_skb(sk, ackskb); This is not the correct fix. You need to instead replace the existing local_irq_*() calls in the function with the spinlock stuff.
diff --git a/drivers/net/arcnet/arcnet.c b/drivers/net/arcnet/arcnet.c index 8459115d9d4e..1e1b12650964 100644 --- a/drivers/net/arcnet/arcnet.c +++ b/drivers/net/arcnet/arcnet.c @@ -401,6 +401,7 @@ static void arcnet_reply_tasklet(unsigned long data) struct sock_exterr_skb *serr; struct sock *sk; int ret; + unsigned long flags; local_irq_disable(); skb = lp->outgoing.skb; @@ -426,10 +427,14 @@ static void arcnet_reply_tasklet(unsigned long data) serr->ee.ee_data = skb_shinfo(skb)->tskey; serr->ee.ee_info = lp->reply_status; + spin_lock_irqsave(&lp->lock, flags); + /* finally erasing outgoing skb */ dev_kfree_skb(lp->outgoing.skb); lp->outgoing.skb = NULL; + spin_unlock_irqrestore(&lp->lock, flags); + ackskb->dev = lp->dev; ret = sock_queue_err_skb(sk, ackskb);
In drivers/net/arcnet/arcnet.c, the functions arcnet_reply_tasklet() and arcnet_send_packet() may be concurrently executed. arcnet_reply_tasklet() line 430: dev_kfree_skb(lp->outgoing.skb); arcnet_send_packet() line 682: spin_lock_irqsave(); line 690: lp->outgoing.skb = skb; line 691: proto->prepare_tx(..., skb->len, ...) Thus, a possible concurrency use-after-free bugs may occur. To fix this bug, the calls to spin_lock_irqsave() and spin_unlock_irqrestore() are added in arcnet_reply_tasklet() to protect dev_kfree_skb(lp->outgoing.skb). Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> --- v2: * Add the definition of flags to fix the compilation error. --- drivers/net/arcnet/arcnet.c | 5 +++++ 1 file changed, 5 insertions(+)