netdev - [PATCH net-next] bnxt_en: fix kernel panic in queue api functions

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]

Message-Id: <20240704074153.1508825-1-ap420073@gmail.com>
Date: Thu,  4 Jul 2024 07:41:53 +0000
From: Taehee Yoo <ap420073@...il.com>
To: davem@...emloft.net,
	kuba@...nel.org,
	pabeni@...hat.com,
	edumazet@...gle.com,
	michael.chan@...adcom.com,
	netdev@...r.kernel.org
Cc: ap420073@...il.com,
	somnath.kotur@...adcom.com,
	dw@...idwei.uk,
	horms@...nel.org
Subject: [PATCH net-next] bnxt_en: fix kernel panic in queue api functions

bnxt_queue_{mem_alloc,start,stop} access bp->rx_ring array and this is
initialized while an interface is being up.
The rings are initialized as a number of channels.

The queue API functions access rx_ring without checking both null and
ring size.
So, if the queue API functions are called when interface status is down,
they access an uninitialized rx_ring array.
Also if the queue index parameter value is larger than a ring, it
would also access an uninitialized rx_ring.

 BUG: kernel NULL pointer dereference, address: 0000000000000000
 #PF: supervisor read access in kernel mode
 #PF: error_code(0x0000) - not-present page
 PGD 0 P4D 0
 Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
 CPU: 1 PID: 1697 Comm: ncdevmem Not tainted 6.10.0-rc5+ #34
 RIP: 0010:bnxt_queue_mem_alloc+0x38/0x410 [bnxt_en]
 Code: 49 89 f5 41 54 4d 89 c4 4d 69 c0 c0 05 00 00 55 48 8d af 40 0a 00 00 53 48 89 fb 48 83 ec 05
 RSP: 0018:ffffa1ad0449ba48 EFLAGS: 00010246
 RAX: ffffffffc04c7710 RBX: ffff9b88aee48000 RCX: 0000000000000000
 RDX: 0000000000000000 RSI: ffff9b8884ba0000 RDI: ffff9b8884ba0008
 RBP: ffff9b88aee48a40 R08: 0000000000000000 R09: ffff9b8884ba6000
 R10: ffffa1ad0449ba88 R11: ffff9b8884ba6000 R12: 0000000000000000
 R13: ffff9b8884ba0000 R14: ffff9b8884ba0000 R15: ffff9b8884ba6000
 FS:  00007f7b2a094740(0000) GS:ffff9b8f9f680000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000000000000000 CR3: 000000015f394000 CR4: 00000000007506f0
 PKRU: 55555554
 Call Trace:
  <TASK>
  ? __die+0x20/0x70
  ? page_fault_oops+0x15a/0x460
  ? __vmalloc_node_range_noprof+0x4f7/0x8e0
  ? exc_page_fault+0x6e/0x180
  ? asm_exc_page_fault+0x22/0x30
  ? __pfx_bnxt_queue_mem_alloc+0x10/0x10 [bnxt_en 2b2843e995211f081639d5c0e74fe1cce7fed534]
  ? bnxt_queue_mem_alloc+0x38/0x410 [bnxt_en 2b2843e995211f081639d5c0e74fe1cce7fed534]
  netdev_rx_queue_restart+0xa9/0x1c0
  net_devmem_bind_dmabuf_to_queue+0xcb/0x100
  netdev_nl_bind_rx_doit+0x2f6/0x350
  genl_family_rcv_msg_doit+0xd9/0x130
  genl_rcv_msg+0x184/0x2b0
  ? __pfx_netdev_nl_bind_rx_doit+0x10/0x10
  ? __pfx_genl_rcv_msg+0x10/0x10
  netlink_rcv_skb+0x54/0x100
  genl_rcv+0x24/0x40
  netlink_unicast+0x243/0x370
  netlink_sendmsg+0x1bb/0x3e0

Fixes: 2d694c27d32e ("bnxt_en: implement netdev_queue_mgmt_ops")
Signed-off-by: Taehee Yoo <ap420073@...il.com>
---

The branch is not net because the commit 2d694c27d32e is not
yet merged into net branch.

devmem TCP causes this problem, but it is not yet merged.
So, to test this patch, please patch the current devmem TCP.
The /tools/testing/selftests/net/ncdevmem will immediately reproduce 
this problem.

 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 6fc34ccb86e3..e270fb6b2866 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -15022,6 +15022,9 @@ static int bnxt_queue_mem_alloc(struct net_device *dev, void *qmem, int idx)
 	struct bnxt_ring_struct *ring;
 	int rc;
 
+	if (!bp->rx_ring || idx >= bp->rx_nr_rings)
+		return -EINVAL;
+
 	rxr = &bp->rx_ring[idx];
 	clone = qmem;
 	memcpy(clone, rxr, sizeof(*rxr));
@@ -15156,6 +15159,9 @@ static int bnxt_queue_start(struct net_device *dev, void *qmem, int idx)
 	struct bnxt_cp_ring_info *cpr;
 	int rc;
 
+	if (!bp->rx_ring || idx >= bp->rx_nr_rings)
+		return -EINVAL;
+
 	rxr = &bp->rx_ring[idx];
 	clone = qmem;
 
@@ -15195,6 +15201,9 @@ static int bnxt_queue_stop(struct net_device *dev, void *qmem, int idx)
 	struct bnxt *bp = netdev_priv(dev);
 	struct bnxt_rx_ring_info *rxr;
 
+	if (!bp->rx_ring || idx >= bp->rx_nr_rings)
+		return -EINVAL;
+
 	rxr = &bp->rx_ring[idx];
 	napi_disable(&rxr->bnapi->napi);
 	bnxt_hwrm_rx_ring_free(bp, rxr, false);
-- 
2.34.1