All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: arcnet: Fix a possible concurrency use-after-free bug in arcnet_reply_tasklet()
@ 2018-12-26 14:22 Jia-Ju Bai
  2018-12-26 18:58 ` kbuild test robot
  0 siblings, 1 reply; 2+ messages in thread
From: Jia-Ju Bai @ 2018-12-26 14:22 UTC (permalink / raw)
  To: m.grzeschik, davem; +Cc: netdev, linux-kernel, Jia-Ju Bai

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>
---
 drivers/net/arcnet/arcnet.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/arcnet/arcnet.c b/drivers/net/arcnet/arcnet.c
index 8459115d9d4e..c5e943d01d66 100644
--- a/drivers/net/arcnet/arcnet.c
+++ b/drivers/net/arcnet/arcnet.c
@@ -426,10 +426,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);
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] net: arcnet: Fix a possible concurrency use-after-free bug in arcnet_reply_tasklet()
  2018-12-26 14:22 [PATCH] net: arcnet: Fix a possible concurrency use-after-free bug in arcnet_reply_tasklet() Jia-Ju Bai
@ 2018-12-26 18:58 ` kbuild test robot
  0 siblings, 0 replies; 2+ messages in thread
From: kbuild test robot @ 2018-12-26 18:58 UTC (permalink / raw)
  To: Jia-Ju Bai
  Cc: kbuild-all, m.grzeschik, davem, netdev, linux-kernel, Jia-Ju Bai

[-- Attachment #1: Type: text/plain, Size: 6532 bytes --]

Hi Jia-Ju,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]
[also build test ERROR on v4.20 next-20181224]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jia-Ju-Bai/net-arcnet-Fix-a-possible-concurrency-use-after-free-bug-in-arcnet_reply_tasklet/20181227-020417
config: x86_64-randconfig-x005-201851 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:13:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from drivers/net/arcnet/arcnet.c:46:
   drivers/net/arcnet/arcnet.c: In function 'arcnet_reply_tasklet':
>> drivers/net/arcnet/arcnet.c:429:31: error: 'flags' undeclared (first use in this function); did you mean 'class'?
     spin_lock_irqsave(&lp->lock, flags);
                                  ^
   include/linux/typecheck.h:11:9: note: in definition of macro 'typecheck'
     typeof(x) __dummy2; \
            ^
>> include/linux/spinlock.h:359:2: note: in expansion of macro 'raw_spin_lock_irqsave'
     raw_spin_lock_irqsave(spinlock_check(lock), flags); \
     ^~~~~~~~~~~~~~~~~~~~~
>> drivers/net/arcnet/arcnet.c:429:2: note: in expansion of macro 'spin_lock_irqsave'
     spin_lock_irqsave(&lp->lock, flags);
     ^~~~~~~~~~~~~~~~~
   drivers/net/arcnet/arcnet.c:429:31: note: each undeclared identifier is reported only once for each function it appears in
     spin_lock_irqsave(&lp->lock, flags);
                                  ^
   include/linux/typecheck.h:11:9: note: in definition of macro 'typecheck'
     typeof(x) __dummy2; \
            ^
>> include/linux/spinlock.h:359:2: note: in expansion of macro 'raw_spin_lock_irqsave'
     raw_spin_lock_irqsave(spinlock_check(lock), flags); \
     ^~~~~~~~~~~~~~~~~~~~~
>> drivers/net/arcnet/arcnet.c:429:2: note: in expansion of macro 'spin_lock_irqsave'
     spin_lock_irqsave(&lp->lock, flags);
     ^~~~~~~~~~~~~~~~~
>> include/linux/typecheck.h:12:18: warning: comparison of distinct pointer types lacks a cast
     (void)(&__dummy == &__dummy2); \
                     ^
>> include/linux/spinlock.h:240:3: note: in expansion of macro 'typecheck'
      typecheck(unsigned long, flags); \
      ^~~~~~~~~
>> include/linux/spinlock.h:359:2: note: in expansion of macro 'raw_spin_lock_irqsave'
     raw_spin_lock_irqsave(spinlock_check(lock), flags); \
     ^~~~~~~~~~~~~~~~~~~~~
>> drivers/net/arcnet/arcnet.c:429:2: note: in expansion of macro 'spin_lock_irqsave'
     spin_lock_irqsave(&lp->lock, flags);
     ^~~~~~~~~~~~~~~~~
--
   In file included from include/linux/kernel.h:13:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from drivers/net//arcnet/arcnet.c:46:
   drivers/net//arcnet/arcnet.c: In function 'arcnet_reply_tasklet':
   drivers/net//arcnet/arcnet.c:429:31: error: 'flags' undeclared (first use in this function); did you mean 'class'?
     spin_lock_irqsave(&lp->lock, flags);
                                  ^
   include/linux/typecheck.h:11:9: note: in definition of macro 'typecheck'
     typeof(x) __dummy2; \
            ^
>> include/linux/spinlock.h:359:2: note: in expansion of macro 'raw_spin_lock_irqsave'
     raw_spin_lock_irqsave(spinlock_check(lock), flags); \
     ^~~~~~~~~~~~~~~~~~~~~
   drivers/net//arcnet/arcnet.c:429:2: note: in expansion of macro 'spin_lock_irqsave'
     spin_lock_irqsave(&lp->lock, flags);
     ^~~~~~~~~~~~~~~~~
   drivers/net//arcnet/arcnet.c:429:31: note: each undeclared identifier is reported only once for each function it appears in
     spin_lock_irqsave(&lp->lock, flags);
                                  ^
   include/linux/typecheck.h:11:9: note: in definition of macro 'typecheck'
     typeof(x) __dummy2; \
            ^
>> include/linux/spinlock.h:359:2: note: in expansion of macro 'raw_spin_lock_irqsave'
     raw_spin_lock_irqsave(spinlock_check(lock), flags); \
     ^~~~~~~~~~~~~~~~~~~~~
   drivers/net//arcnet/arcnet.c:429:2: note: in expansion of macro 'spin_lock_irqsave'
     spin_lock_irqsave(&lp->lock, flags);
     ^~~~~~~~~~~~~~~~~
>> include/linux/typecheck.h:12:18: warning: comparison of distinct pointer types lacks a cast
     (void)(&__dummy == &__dummy2); \
                     ^
>> include/linux/spinlock.h:240:3: note: in expansion of macro 'typecheck'
      typecheck(unsigned long, flags); \
      ^~~~~~~~~
>> include/linux/spinlock.h:359:2: note: in expansion of macro 'raw_spin_lock_irqsave'
     raw_spin_lock_irqsave(spinlock_check(lock), flags); \
     ^~~~~~~~~~~~~~~~~~~~~
   drivers/net//arcnet/arcnet.c:429:2: note: in expansion of macro 'spin_lock_irqsave'
     spin_lock_irqsave(&lp->lock, flags);
     ^~~~~~~~~~~~~~~~~

vim +429 drivers/net/arcnet/arcnet.c

   395	
   396	static void arcnet_reply_tasklet(unsigned long data)
   397	{
   398		struct arcnet_local *lp = (struct arcnet_local *)data;
   399	
   400		struct sk_buff *ackskb, *skb;
   401		struct sock_exterr_skb *serr;
   402		struct sock *sk;
   403		int ret;
   404	
   405		local_irq_disable();
   406		skb = lp->outgoing.skb;
   407		if (!skb || !skb->sk) {
   408			local_irq_enable();
   409			return;
   410		}
   411	
   412		sock_hold(skb->sk);
   413		sk = skb->sk;
   414		ackskb = skb_clone_sk(skb);
   415		sock_put(skb->sk);
   416	
   417		if (!ackskb) {
   418			local_irq_enable();
   419			return;
   420		}
   421	
   422		serr = SKB_EXT_ERR(ackskb);
   423		memset(serr, 0, sizeof(*serr));
   424		serr->ee.ee_errno = ENOMSG;
   425		serr->ee.ee_origin = SO_EE_ORIGIN_TXSTATUS;
   426		serr->ee.ee_data = skb_shinfo(skb)->tskey;
   427		serr->ee.ee_info = lp->reply_status;
   428	
 > 429		spin_lock_irqsave(&lp->lock, flags);
   430	
   431		/* finally erasing outgoing skb */
   432		dev_kfree_skb(lp->outgoing.skb);
   433		lp->outgoing.skb = NULL;
   434	
   435		spin_unlock_irqrestore(&lp->lock, flags);
   436	
   437		ackskb->dev = lp->dev;
   438	
   439		ret = sock_queue_err_skb(sk, ackskb);
   440		if (ret)
   441			kfree_skb(ackskb);
   442	
   443		local_irq_enable();
   444	};
   445	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26420 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-12-26 19:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-26 14:22 [PATCH] net: arcnet: Fix a possible concurrency use-after-free bug in arcnet_reply_tasklet() Jia-Ju Bai
2018-12-26 18:58 ` kbuild test robot

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.