netdev - [PATCH] net: nvidia: forcedeth: Fix two possible concurrency use-after-free bugs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Tue,  8 Jan 2019 20:45:18 +0800
From:   Jia-Ju Bai <baijiaju1990@...il.com>
To:     davem@...emloft.net, yanjun.zhu@...cle.com, keescook@...omium.org
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jia-Ju Bai <baijiaju1990@...il.com>
Subject: [PATCH] net: nvidia: forcedeth: Fix two possible concurrency use-after-free bugs

In drivers/net/ethernet/nvidia/forcedeth.c, the functions
nv_start_xmit() and nv_start_xmit_optimized() can be concurrently
executed with nv_poll_controller().

nv_start_xmit
  line 2321: prev_tx_ctx->skb = skb;

nv_start_xmit_optimized
  line 2479: prev_tx_ctx->skb = skb;

nv_poll_controller
  nv_do_nic_poll
    line 4134: spin_lock(&np->lock);
    nv_drain_rxtx
      nv_drain_tx
        nv_release_txskb
          line 2004: dev_kfree_skb_any(tx_skb->skb);

Thus, two possible concurrency use-after-free bugs may occur.

To fix these possible bugs, the calls to spin_lock_irqsave() in 
nv_start_xmit() and nv_start_xmit_optimized() are moved to the 
front of "prev_tx_ctx->skb = skb;"

Signed-off-by: Jia-Ju Bai <baijiaju1990@...il.com>
---
 drivers/net/ethernet/nvidia/forcedeth.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 1d9b0d44ddb6..48fa5a0bd2cb 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -2317,6 +2317,8 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* set last fragment flag  */
 	prev_tx->flaglen |= cpu_to_le32(tx_flags_extra);
 
+	spin_lock_irqsave(&np->lock, flags);
+
 	/* save skb in this slot's context area */
 	prev_tx_ctx->skb = skb;
 
@@ -2326,8 +2328,6 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		tx_flags_extra = skb->ip_summed == CHECKSUM_PARTIAL ?
 			 NV_TX2_CHECKSUM_L3 | NV_TX2_CHECKSUM_L4 : 0;
 
-	spin_lock_irqsave(&np->lock, flags);
-
 	/* set tx flags */
 	start_tx->flaglen |= cpu_to_le32(tx_flags | tx_flags_extra);
 
@@ -2475,6 +2475,8 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
 	/* set last fragment flag  */
 	prev_tx->flaglen |= cpu_to_le32(NV_TX2_LASTPACKET);
 
+	spin_lock_irqsave(&np->lock, flags);
+
 	/* save skb in this slot's context area */
 	prev_tx_ctx->skb = skb;
 
@@ -2491,8 +2493,6 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
 	else
 		start_tx->txvlan = 0;
 
-	spin_lock_irqsave(&np->lock, flags);
-
 	if (np->tx_limit) {
 		/* Limit the number of outstanding tx. Setup all fragments, but
 		 * do not set the VALID bit on the first descriptor. Save a pointer
-- 
2.17.0