linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for-rc 0/4] AIP panic and hardening fixes
@ 2022-01-15 23:02 mike.marciniszyn
  2022-01-15 23:02 ` [PATCH for-rc 1/4] IB/hfi1: Fix panic with larger ipoib send_queue_size mike.marciniszyn
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: mike.marciniszyn @ 2022-01-15 23:02 UTC (permalink / raw)
  To: jgg; +Cc: linux-rdma, Mike Marciniszyn

From: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>

The first three patches fix serious stablity issues associated
with increasing the send_queue_size as relayed by ipoib.

The last fixes adds a missing alloc failure test in allocating
the per cpu stats structure.

Mike Marciniszyn (4):
  IB/hfi1: Fix panic with larger ipoib send_queue_size
  IB/hfi1: Fix alloc failure with larger txqueuelen
  IB/hfi1: Fix AIP early init panic
  IB/hfi1: Fix tstats alloc and dealloc

 drivers/infiniband/hw/hfi1/ipoib.h      |  2 +-
 drivers/infiniband/hw/hfi1/ipoib_main.c | 27 ++++++++++++-----------
 drivers/infiniband/hw/hfi1/ipoib_tx.c   | 38 ++++++++++++++++++++++-----------
 3 files changed, 42 insertions(+), 25 deletions(-)

-- 
1.8.3.1


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

* [PATCH for-rc 1/4] IB/hfi1: Fix panic with larger ipoib send_queue_size
  2022-01-15 23:02 [PATCH for-rc 0/4] AIP panic and hardening fixes mike.marciniszyn
@ 2022-01-15 23:02 ` mike.marciniszyn
  2022-01-15 23:02 ` [PATCH for-rc 2/4] IB/hfi1: Fix alloc failure with larger txqueuelen mike.marciniszyn
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: mike.marciniszyn @ 2022-01-15 23:02 UTC (permalink / raw)
  To: jgg; +Cc: linux-rdma, Mike Marciniszyn, stable

From: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>

When the ipoib send_queue_size is increased from the default
the following panic happens:

[  219.242960] RIP: 0010:hfi1_ipoib_drain_tx_ring+0x45/0xf0 [hfi1]
[  219.250708] Code: 31 e4 eb 0f 8b 85 c8 02 00 00 41 83 c4 01 44 39 e0 76 60 8b 8d cc 02 00 00 44 89 e3 be 01 00 00 00 d3 e3 48 03 9d c0 02 00 00 <c7> 83 18 01 00 00 00 00 00 00 48 8b bb 30 01 00 00 e8 25 af a7 e0
[  219.273764] RSP: 0018:ffffc9000798f4a0 EFLAGS: 00010286
[  219.280740] RAX: 0000000000008000 RBX: ffffc9000aa0f000 RCX: 000000000000000f
[  219.289842] RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000000
[  219.298864] RBP: ffff88810ff08000 R08: ffff88889476d900 R09: 0000000000000101
[  219.307907] R10: 0000000000000000 R11: ffffc90006590ff8 R12: 0000000000000200
[  219.317016] R13: ffffc9000798fba8 R14: 0000000000000000 R15: 0000000000000001
[  219.326100] FS:  00007fd0f79cc3c0(0000) GS:ffff88885fb00000(0000) knlGS:0000000000000000
[  219.336171] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  219.343639] CR2: ffffc9000aa0f118 CR3: 0000000889c84001 CR4: 00000000001706e0
[  219.352589] Call Trace:
[  219.356340]  <TASK>
[  219.359804]  hfi1_ipoib_napi_tx_disable+0x45/0x60 [hfi1]
[  219.366887]  hfi1_ipoib_dev_stop+0x18/0x80 [hfi1]
[  219.373313]  ipoib_ib_dev_stop+0x1d/0x40 [ib_ipoib]
[  219.379814]  ipoib_stop+0x48/0xc0 [ib_ipoib]
[  219.385604]  __dev_close_many+0x9e/0x110
[  219.391001]  __dev_change_flags+0xd9/0x210
[  219.396618]  dev_change_flags+0x21/0x60
[  219.401878]  do_setlink+0x31c/0x10f0
[  219.406841]  ? __nla_validate_parse+0x12d/0x1a0
[  219.412902]  ? __nla_parse+0x21/0x30
[  219.417844]  ? inet6_validate_link_af+0x5e/0xf0
[  219.423913]  ? cpumask_next+0x1f/0x20
[  219.428914]  ? __snmp6_fill_stats64.isra.53+0xbb/0x140
[  219.435648]  ? __nla_validate_parse+0x47/0x1a0
[  219.441564]  __rtnl_newlink+0x530/0x910
[  219.446818]  ? pskb_expand_head+0x73/0x300
[  219.452198]  ? __kmalloc_node_track_caller+0x109/0x280
[  219.458999]  ? __nla_put+0xc/0x20
[  219.463733]  ? cpumask_next_and+0x20/0x30
[  219.469166]  ? update_sd_lb_stats.constprop.144+0xd3/0x820
[  219.476325]  ? _raw_spin_unlock_irqrestore+0x25/0x37
[  219.482815]  ? __wake_up_common_lock+0x87/0xc0
[  219.488761]  ? kmem_cache_alloc_trace+0x3d/0x3d0
[  219.494917]  rtnl_newlink+0x43/0x60

The issue happens when the shift that should have been a function of
the txq item size mistakenly used the ring size.

Fix by using the item size.

Fixes: d47dfc2b00e6 ("IB/hfi1: Remove cache and embed txreq in ring")
Cc: stable@vger.kernel.org
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
---
 drivers/infiniband/hw/hfi1/ipoib_tx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/hfi1/ipoib_tx.c b/drivers/infiniband/hw/hfi1/ipoib_tx.c
index f401089..bf62956 100644
--- a/drivers/infiniband/hw/hfi1/ipoib_tx.c
+++ b/drivers/infiniband/hw/hfi1/ipoib_tx.c
@@ -731,7 +731,7 @@ int hfi1_ipoib_txreq_init(struct hfi1_ipoib_dev_priv *priv)
 			goto free_txqs;
 
 		txq->tx_ring.max_items = tx_ring_size;
-		txq->tx_ring.shift = ilog2(tx_ring_size);
+		txq->tx_ring.shift = ilog2(tx_item_size);
 		txq->tx_ring.avail = hfi1_ipoib_ring_hwat(txq);
 
 		netif_tx_napi_add(dev, &txq->napi,
-- 
1.8.3.1


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

* [PATCH for-rc 2/4] IB/hfi1: Fix alloc failure with larger txqueuelen
  2022-01-15 23:02 [PATCH for-rc 0/4] AIP panic and hardening fixes mike.marciniszyn
  2022-01-15 23:02 ` [PATCH for-rc 1/4] IB/hfi1: Fix panic with larger ipoib send_queue_size mike.marciniszyn
@ 2022-01-15 23:02 ` mike.marciniszyn
  2022-01-15 23:02 ` [PATCH for-rc 3/4] IB/hfi1: Fix AIP early init panic mike.marciniszyn
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: mike.marciniszyn @ 2022-01-15 23:02 UTC (permalink / raw)
  To: jgg; +Cc: linux-rdma, Mike Marciniszyn, stable

From: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>

The following allocation with large txqueuelen will result in the
following warning:

[  136.166367] Call Trace:
[  136.169661]  __alloc_pages_nodemask+0x283/0x2c0
[  136.175273]  kmalloc_large_node+0x3c/0xa0
[  136.180289]  __kmalloc_node+0x22a/0x2f0
[  136.185110]  ? __kmalloc_node+0x22a/0x2f0
[  136.190169]  hfi1_ipoib_txreq_init+0x19f/0x330 [hfi1]
[  136.196453]  hfi1_ipoib_setup_rn+0xd3/0x1a0 [hfi1]
[  136.202396]  rdma_init_netdev+0x5a/0x80 [ib_core]
[  136.208210]  ? hfi1_ipoib_set_id+0x30/0x30 [hfi1]
[  136.213995]  ipoib_intf_init+0x6c/0x350 [ib_ipoib]
[  136.219873]  ipoib_intf_alloc+0x5c/0xc0 [ib_ipoib]
[  136.225751]  ipoib_add_one+0xbe/0x300 [ib_ipoib]
[  136.231563]  add_client_context+0x12c/0x1a0 [ib_core]
[  136.237739]  ib_register_client+0x147/0x190 [ib_core]
[  136.243906]  ? 0xffffffffc0570000
[  136.248123]  ipoib_init_module+0xdd/0x132 [ib_ipoib]
[  136.254212]  do_one_initcall+0x46/0x1c3
[  136.259136]  ? do_init_module+0x22/0x220
[  136.264043]  ? kmem_cache_alloc_trace+0x131/0x270
[  136.269813]  do_init_module+0x5a/0x220
[  136.274547]  load_module+0x14c5/0x17f0
[  136.279246]  ? __do_sys_init_module+0x13b/0x180
[  136.284810]  __do_sys_init_module+0x13b/0x180
[  136.290295]  do_syscall_64+0x5b/0x1a0
[  136.294914]  entry_SYSCALL_64_after_hwframe+0x65/0xca
[  136.301070] RIP: 0033:0x7f3eacd0d80e

For ipoib, the txqueuelen is modified with the module parameter
send_queue_size.

Fix by changing to use kv versions of the same allocator to handle
the large allocations.  The allocation embeds a hdr struct that
is dma mapped.  Change that struct to a pointer to a kzalloced struct.

Fixes: d99dc602e2a5 ("IB/hfi1: Add functions to transmit datagram ipoib packets")
Cc: stable@vger.kernel.org
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
---
 drivers/infiniband/hw/hfi1/ipoib.h    |  2 +-
 drivers/infiniband/hw/hfi1/ipoib_tx.c | 36 ++++++++++++++++++++++++-----------
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/ipoib.h b/drivers/infiniband/hw/hfi1/ipoib.h
index 9091229..aec60d4 100644
--- a/drivers/infiniband/hw/hfi1/ipoib.h
+++ b/drivers/infiniband/hw/hfi1/ipoib.h
@@ -55,7 +55,7 @@
  */
 struct ipoib_txreq {
 	struct sdma_txreq           txreq;
-	struct hfi1_sdma_header     sdma_hdr;
+	struct hfi1_sdma_header     *sdma_hdr;
 	int                         sdma_status;
 	int                         complete;
 	struct hfi1_ipoib_dev_priv *priv;
diff --git a/drivers/infiniband/hw/hfi1/ipoib_tx.c b/drivers/infiniband/hw/hfi1/ipoib_tx.c
index bf62956..d6bbdb8 100644
--- a/drivers/infiniband/hw/hfi1/ipoib_tx.c
+++ b/drivers/infiniband/hw/hfi1/ipoib_tx.c
@@ -122,7 +122,7 @@ static void hfi1_ipoib_free_tx(struct ipoib_txreq *tx, int budget)
 		dd_dev_warn(priv->dd,
 			    "%s: Status = 0x%x pbc 0x%llx txq = %d sde = %d\n",
 			    __func__, tx->sdma_status,
-			    le64_to_cpu(tx->sdma_hdr.pbc), tx->txq->q_idx,
+			    le64_to_cpu(tx->sdma_hdr->pbc), tx->txq->q_idx,
 			    tx->txq->sde->this_idx);
 	}
 
@@ -231,7 +231,7 @@ static int hfi1_ipoib_build_tx_desc(struct ipoib_txreq *tx,
 {
 	struct hfi1_devdata *dd = txp->dd;
 	struct sdma_txreq *txreq = &tx->txreq;
-	struct hfi1_sdma_header *sdma_hdr = &tx->sdma_hdr;
+	struct hfi1_sdma_header *sdma_hdr = tx->sdma_hdr;
 	u16 pkt_bytes =
 		sizeof(sdma_hdr->pbc) + (txp->hdr_dwords << 2) + tx->skb->len;
 	int ret;
@@ -256,7 +256,7 @@ static void hfi1_ipoib_build_ib_tx_headers(struct ipoib_txreq *tx,
 					   struct ipoib_txparms *txp)
 {
 	struct hfi1_ipoib_dev_priv *priv = tx->txq->priv;
-	struct hfi1_sdma_header *sdma_hdr = &tx->sdma_hdr;
+	struct hfi1_sdma_header *sdma_hdr = tx->sdma_hdr;
 	struct sk_buff *skb = tx->skb;
 	struct hfi1_pportdata *ppd = ppd_from_ibp(txp->ibp);
 	struct rdma_ah_attr *ah_attr = txp->ah_attr;
@@ -483,7 +483,7 @@ static int hfi1_ipoib_send_dma_single(struct net_device *dev,
 	if (likely(!ret)) {
 tx_ok:
 		trace_sdma_output_ibhdr(txq->priv->dd,
-					&tx->sdma_hdr.hdr,
+					&tx->sdma_hdr->hdr,
 					ib_is_sc5(txp->flow.sc5));
 		hfi1_ipoib_check_queue_depth(txq);
 		return NETDEV_TX_OK;
@@ -547,7 +547,7 @@ static int hfi1_ipoib_send_dma_list(struct net_device *dev,
 	hfi1_ipoib_check_queue_depth(txq);
 
 	trace_sdma_output_ibhdr(txq->priv->dd,
-				&tx->sdma_hdr.hdr,
+				&tx->sdma_hdr->hdr,
 				ib_is_sc5(txp->flow.sc5));
 
 	if (!netdev_xmit_more())
@@ -683,7 +683,8 @@ int hfi1_ipoib_txreq_init(struct hfi1_ipoib_dev_priv *priv)
 {
 	struct net_device *dev = priv->netdev;
 	u32 tx_ring_size, tx_item_size;
-	int i;
+	struct hfi1_ipoib_circ_buf *tx_ring;
+	int i, j;
 
 	/*
 	 * Ring holds 1 less than tx_ring_size
@@ -701,7 +702,9 @@ int hfi1_ipoib_txreq_init(struct hfi1_ipoib_dev_priv *priv)
 
 	for (i = 0; i < dev->num_tx_queues; i++) {
 		struct hfi1_ipoib_txq *txq = &priv->txqs[i];
+		struct ipoib_txreq *tx;
 
+		tx_ring = &txq->tx_ring;
 		iowait_init(&txq->wait,
 			    0,
 			    hfi1_ipoib_flush_txq,
@@ -725,14 +728,19 @@ int hfi1_ipoib_txreq_init(struct hfi1_ipoib_dev_priv *priv)
 					     priv->dd->node);
 
 		txq->tx_ring.items =
-			kcalloc_node(tx_ring_size, tx_item_size,
-				     GFP_KERNEL, priv->dd->node);
+			kvzalloc_node(array_size(tx_ring_size, tx_item_size),
+				      GFP_KERNEL, priv->dd->node);
 		if (!txq->tx_ring.items)
 			goto free_txqs;
 
 		txq->tx_ring.max_items = tx_ring_size;
 		txq->tx_ring.shift = ilog2(tx_item_size);
 		txq->tx_ring.avail = hfi1_ipoib_ring_hwat(txq);
+		tx_ring = &txq->tx_ring;
+		for (j = 0; j < tx_ring_size; j++)
+			hfi1_txreq_from_idx(tx_ring, j)->sdma_hdr =
+				kzalloc_node(sizeof(*tx->sdma_hdr),
+					     GFP_KERNEL, priv->dd->node);
 
 		netif_tx_napi_add(dev, &txq->napi,
 				  hfi1_ipoib_poll_tx_ring,
@@ -746,7 +754,10 @@ int hfi1_ipoib_txreq_init(struct hfi1_ipoib_dev_priv *priv)
 		struct hfi1_ipoib_txq *txq = &priv->txqs[i];
 
 		netif_napi_del(&txq->napi);
-		kfree(txq->tx_ring.items);
+		tx_ring = &txq->tx_ring;
+		for (j = 0; j < tx_ring_size; j++)
+			kfree(hfi1_txreq_from_idx(tx_ring, j)->sdma_hdr);
+		kvfree(tx_ring->items);
 	}
 
 	kfree(priv->txqs);
@@ -780,17 +791,20 @@ static void hfi1_ipoib_drain_tx_list(struct hfi1_ipoib_txq *txq)
 
 void hfi1_ipoib_txreq_deinit(struct hfi1_ipoib_dev_priv *priv)
 {
-	int i;
+	int i, j;
 
 	for (i = 0; i < priv->netdev->num_tx_queues; i++) {
 		struct hfi1_ipoib_txq *txq = &priv->txqs[i];
+		struct hfi1_ipoib_circ_buf *tx_ring = &txq->tx_ring;
 
 		iowait_cancel_work(&txq->wait);
 		iowait_sdma_drain(&txq->wait);
 		hfi1_ipoib_drain_tx_list(txq);
 		netif_napi_del(&txq->napi);
 		hfi1_ipoib_drain_tx_ring(txq);
-		kfree(txq->tx_ring.items);
+		for (j = 0; j < tx_ring->max_items; j++)
+			kfree(hfi1_txreq_from_idx(tx_ring, j)->sdma_hdr);
+		kvfree(tx_ring->items);
 	}
 
 	kfree(priv->txqs);
-- 
1.8.3.1


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

* [PATCH for-rc 3/4] IB/hfi1: Fix AIP early init panic
  2022-01-15 23:02 [PATCH for-rc 0/4] AIP panic and hardening fixes mike.marciniszyn
  2022-01-15 23:02 ` [PATCH for-rc 1/4] IB/hfi1: Fix panic with larger ipoib send_queue_size mike.marciniszyn
  2022-01-15 23:02 ` [PATCH for-rc 2/4] IB/hfi1: Fix alloc failure with larger txqueuelen mike.marciniszyn
@ 2022-01-15 23:02 ` mike.marciniszyn
  2022-01-15 23:02 ` [PATCH for-rc 4/4] IB/hfi1: Fix tstats alloc and dealloc mike.marciniszyn
  2022-01-28 15:12 ` [PATCH for-rc 0/4] AIP panic and hardening fixes Jason Gunthorpe
  4 siblings, 0 replies; 6+ messages in thread
From: mike.marciniszyn @ 2022-01-15 23:02 UTC (permalink / raw)
  To: jgg; +Cc: linux-rdma, Mike Marciniszyn, stable

From: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>

An early failure in hfi1_ipoib_setup_rn() can lead to the following
panic:

[  355.625765] BUG: unable to handle kernel NULL pointer dereference at 00000000000001b0
[  355.634188] PGD 0 P4D 0
[  355.636731] Oops: 0002 [#1] SMP NOPTI
[  355.659994] Workqueue: events work_for_cpu_fn
[  355.664371] RIP: 0010:try_to_grab_pending+0x2b/0x140
[  355.669361] Code: 1f 44 00 00 41 55 41 54 55 48 89 d5 53 48 89 fb 9c 58 0f 1f 44 00 00 48 89 c2 fa 66 0f 1f 44 00 00 48 89 55 00 40 84 f6 75 77 <f0> 48 0f ba 2b 00 72 09 31 c0 5b 5d 41 5c 41 5d c3 48 89 df e8 6c
[  355.688238] RSP: 0018:ffffb6b3cf7cfa48 EFLAGS: 00010046
[  355.693491] RAX: 0000000000000246 RBX: 00000000000001b0 RCX: 0000000000000000
[  355.700664] RDX: 0000000000000246 RSI: 0000000000000000 RDI: 00000000000001b0
[  355.707836] RBP: ffffb6b3cf7cfa70 R08: 0000000000000f09 R09: 0000000000000001
[  355.715007] R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000000
[  355.722178] R13: ffffb6b3cf7cfa90 R14: ffffffff9b2fbfc0 R15: ffff8a4fdf244690
[  355.729351] FS:  0000000000000000(0000) GS:ffff8a527f400000(0000) knlGS:0000000000000000
[  355.737485] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  355.743260] CR2: 00000000000001b0 CR3: 00000017e2410003 CR4: 00000000007706f0
[  355.750434] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  355.757607] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  355.764780] PKRU: 55555554
[  355.767497] Call Trace:
[  355.769954]  __cancel_work_timer+0x42/0x190
[  355.774159]  ? dev_printk_emit+0x4e/0x70
[  355.778115]  iowait_cancel_work+0x15/0x30 [hfi1]
[  355.782768]  hfi1_ipoib_txreq_deinit+0x5a/0x220 [hfi1]
[  355.787933]  ? dev_err+0x6c/0x90
[  355.791188]  hfi1_ipoib_netdev_dtor+0x15/0x30 [hfi1]
[  355.796188]  hfi1_ipoib_setup_rn+0x10e/0x150 [hfi1]
[  355.801094]  rdma_init_netdev+0x5a/0x80 [ib_core]
[  355.805832]  ? hfi1_ipoib_free_rdma_netdev+0x20/0x20 [hfi1]
[  355.811434]  ipoib_intf_init+0x6c/0x350 [ib_ipoib]
[  355.816251]  ipoib_intf_alloc+0x5c/0xc0 [ib_ipoib]
[  355.821068]  ipoib_add_one+0xbe/0x300 [ib_ipoib]
[  355.825712]  add_client_context+0x12c/0x1a0 [ib_core]
[  355.830794]  enable_device_and_get+0xdc/0x1d0 [ib_core]
[  355.836049]  ib_register_device+0x572/0x6b0 [ib_core]
[  355.841128]  rvt_register_device+0x11b/0x220 [rdmavt]
[  355.846219]  hfi1_register_ib_device+0x6b4/0x770 [hfi1]
[  355.851486]  do_init_one.isra.20+0x3e3/0x680 [hfi1]
[  355.856389]  local_pci_probe+0x41/0x90
[  355.860154]  work_for_cpu_fn+0x16/0x20
[  355.863921]  process_one_work+0x1a7/0x360
[  355.867948]  ? create_worker+0x1a0/0x1a0
[  355.871888]  worker_thread+0x1cf/0x390
[  355.875655]  ? create_worker+0x1a0/0x1a0
[  355.879594]  kthread+0x116/0x130
[  355.882838]  ? kthread_flush_work_fn+0x10/0x10
[  355.887302]  ret_from_fork+0x1f/0x40
[  355.890893] Modules linked in: rpcrdma sunrpc rdma_ucm ib_srpt ib_isert acpi_cpufreq(-) iscsi_target_mod target_core_mod ib_iser libiscsi scsi_transport_iscsi ib_umad rdma_cm ib_ipoib iw_cm ib_cm mlx5_ib intel_rapl_msr intel_rapl_
common isst_if_common x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm irqbypass mlx5_core crct10dif_pclmul crc32_pclmul hfi1(OE+) tls ghash_clmulni_intel rdmavt(OE) mgag200 drm_kms_helper mlxfw mei_me syscopyarea sysfill
rect ib_uverbs sysimgblt fb_sys_fops rapl ioatdma intel_cstate tg3 i2c_algo_bit mei hpwdt ses drm ib_core pci_hyperv_intf uas enclosure hpilo pcspkr intel_uncore wmi lpc_ich dca acpi_tad ipmi_ssif acpi_power_meter binfmt_misc xpmem(O
) numatools(O) fuse ip_tables dm_mod xfs libcrc32c vfat fat ext4 mbcache jbd2 sd_mod t10_pi sg smartpqi ipmi_si scsi_transport_sas usb_storage ipmi_devintf ipmi_msghandler crc32c_intel [last unloaded: mlxfw]
[  355.970226] CR2: 00000000000001b0
[  355.973583]

The panic happens in hfi1_ipoib_txreq_deinit() because there is a NULL
deref when hfi1_ipoib_netdev_dtor() is called in this error case.

hfi1_ipoib_txreq_init() and hfi1_ipoib_rxq_init() are self unwinding so fix
by adjusting the error paths accordingly.

Other changes:
- hfi1_ipoib_free_rdma_netdev() is deleted including the free_netdev()
  since the netdev core code deletes calls free_netdev()
- The switch to the accelerated entrances is moved to the success path.

Fixes: d99dc602e2a5 ("IB/hfi1: Add functions to transmit datagram ipoib packets")
Cc: stable@vger.kernel.org
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
---
 drivers/infiniband/hw/hfi1/ipoib_main.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/ipoib_main.c b/drivers/infiniband/hw/hfi1/ipoib_main.c
index e1a2b02..8306ed5 100644
--- a/drivers/infiniband/hw/hfi1/ipoib_main.c
+++ b/drivers/infiniband/hw/hfi1/ipoib_main.c
@@ -168,12 +168,6 @@ static void hfi1_ipoib_netdev_dtor(struct net_device *dev)
 	free_percpu(dev->tstats);
 }
 
-static void hfi1_ipoib_free_rdma_netdev(struct net_device *dev)
-{
-	hfi1_ipoib_netdev_dtor(dev);
-	free_netdev(dev);
-}
-
 static void hfi1_ipoib_set_id(struct net_device *dev, int id)
 {
 	struct hfi1_ipoib_dev_priv *priv = hfi1_ipoib_priv(dev);
@@ -211,24 +205,23 @@ static int hfi1_ipoib_setup_rn(struct ib_device *device,
 	priv->port_num = port_num;
 	priv->netdev_ops = netdev->netdev_ops;
 
-	netdev->netdev_ops = &hfi1_ipoib_netdev_ops;
-
 	ib_query_pkey(device, port_num, priv->pkey_index, &priv->pkey);
 
 	rc = hfi1_ipoib_txreq_init(priv);
 	if (rc) {
 		dd_dev_err(dd, "IPoIB netdev TX init - failed(%d)\n", rc);
-		hfi1_ipoib_free_rdma_netdev(netdev);
 		return rc;
 	}
 
 	rc = hfi1_ipoib_rxq_init(netdev);
 	if (rc) {
 		dd_dev_err(dd, "IPoIB netdev RX init - failed(%d)\n", rc);
-		hfi1_ipoib_free_rdma_netdev(netdev);
+		hfi1_ipoib_txreq_deinit(priv);
 		return rc;
 	}
 
+	netdev->netdev_ops = &hfi1_ipoib_netdev_ops;
+
 	netdev->priv_destructor = hfi1_ipoib_netdev_dtor;
 	netdev->needs_free_netdev = true;
 
-- 
1.8.3.1


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

* [PATCH for-rc 4/4] IB/hfi1: Fix tstats alloc and dealloc
  2022-01-15 23:02 [PATCH for-rc 0/4] AIP panic and hardening fixes mike.marciniszyn
                   ` (2 preceding siblings ...)
  2022-01-15 23:02 ` [PATCH for-rc 3/4] IB/hfi1: Fix AIP early init panic mike.marciniszyn
@ 2022-01-15 23:02 ` mike.marciniszyn
  2022-01-28 15:12 ` [PATCH for-rc 0/4] AIP panic and hardening fixes Jason Gunthorpe
  4 siblings, 0 replies; 6+ messages in thread
From: mike.marciniszyn @ 2022-01-15 23:02 UTC (permalink / raw)
  To: jgg; +Cc: linux-rdma, Mike Marciniszyn

From: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>

The tstats allocation is done in the accelerated ndo_init
function but the allocation is not tested to succeed.

The deallocation is not done in the accelerated ndo_uninit
function.

Resolve issues by testing for an allocation failure and
adding the free_percpu in the uninit function.

Fixes: aa0616a9bd52 ("IB/hfi1: switch to core handling of rx/tx byte/packet counters")
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
---
 drivers/infiniband/hw/hfi1/ipoib_main.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/ipoib_main.c b/drivers/infiniband/hw/hfi1/ipoib_main.c
index 8306ed5..5d814af 100644
--- a/drivers/infiniband/hw/hfi1/ipoib_main.c
+++ b/drivers/infiniband/hw/hfi1/ipoib_main.c
@@ -22,26 +22,35 @@ static int hfi1_ipoib_dev_init(struct net_device *dev)
 	int ret;
 
 	dev->tstats = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
+	if (!dev->tstats)
+		return -ENOMEM;
 
 	ret = priv->netdev_ops->ndo_init(dev);
 	if (ret)
-		return ret;
+		goto out_ret;
 
 	ret = hfi1_netdev_add_data(priv->dd,
 				   qpn_from_mac(priv->netdev->dev_addr),
 				   dev);
 	if (ret < 0) {
 		priv->netdev_ops->ndo_uninit(dev);
-		return ret;
+		goto out_ret;
 	}
 
 	return 0;
+out_ret:
+	free_percpu(dev->tstats);
+	dev->tstats = NULL;
+	return ret;
 }
 
 static void hfi1_ipoib_dev_uninit(struct net_device *dev)
 {
 	struct hfi1_ipoib_dev_priv *priv = hfi1_ipoib_priv(dev);
 
+	free_percpu(dev->tstats);
+	dev->tstats = NULL;
+
 	hfi1_netdev_remove_data(priv->dd, qpn_from_mac(priv->netdev->dev_addr));
 
 	priv->netdev_ops->ndo_uninit(dev);
@@ -166,6 +175,7 @@ static void hfi1_ipoib_netdev_dtor(struct net_device *dev)
 	hfi1_ipoib_rxq_deinit(priv->netdev);
 
 	free_percpu(dev->tstats);
+	dev->tstats = NULL;
 }
 
 static void hfi1_ipoib_set_id(struct net_device *dev, int id)
-- 
1.8.3.1


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

* Re: [PATCH for-rc 0/4] AIP panic and hardening fixes
  2022-01-15 23:02 [PATCH for-rc 0/4] AIP panic and hardening fixes mike.marciniszyn
                   ` (3 preceding siblings ...)
  2022-01-15 23:02 ` [PATCH for-rc 4/4] IB/hfi1: Fix tstats alloc and dealloc mike.marciniszyn
@ 2022-01-28 15:12 ` Jason Gunthorpe
  4 siblings, 0 replies; 6+ messages in thread
From: Jason Gunthorpe @ 2022-01-28 15:12 UTC (permalink / raw)
  To: mike.marciniszyn; +Cc: linux-rdma

On Sat, Jan 15, 2022 at 06:02:32PM -0500, mike.marciniszyn@cornelisnetworks.com wrote:
> From: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
> 
> The first three patches fix serious stablity issues associated
> with increasing the send_queue_size as relayed by ipoib.
> 
> The last fixes adds a missing alloc failure test in allocating
> the per cpu stats structure.
> 
> Mike Marciniszyn (4):
>   IB/hfi1: Fix panic with larger ipoib send_queue_size
>   IB/hfi1: Fix alloc failure with larger txqueuelen
>   IB/hfi1: Fix AIP early init panic
>   IB/hfi1: Fix tstats alloc and dealloc

Applied to for-rc, thanks

Jason

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

end of thread, other threads:[~2022-01-28 15:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-15 23:02 [PATCH for-rc 0/4] AIP panic and hardening fixes mike.marciniszyn
2022-01-15 23:02 ` [PATCH for-rc 1/4] IB/hfi1: Fix panic with larger ipoib send_queue_size mike.marciniszyn
2022-01-15 23:02 ` [PATCH for-rc 2/4] IB/hfi1: Fix alloc failure with larger txqueuelen mike.marciniszyn
2022-01-15 23:02 ` [PATCH for-rc 3/4] IB/hfi1: Fix AIP early init panic mike.marciniszyn
2022-01-15 23:02 ` [PATCH for-rc 4/4] IB/hfi1: Fix tstats alloc and dealloc mike.marciniszyn
2022-01-28 15:12 ` [PATCH for-rc 0/4] AIP panic and hardening fixes Jason Gunthorpe

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).