* [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