[PATCH net 0/7] ionic: Various bugfixes

[PATCH net 0/7] ionic: Various bugfixes

[Eric Joyner]

Brett's patches mostly fix issues around communication and configuration
between the firmware and the driver; the last two are related and were
found after an incorrectly operating firmware caused Admin Queue
commands to time out. This issue and the rest are fixes to issues that
we've observed internally.

Prabu's patch is critical for enabling PTP/HW timestamping to work
correctly when the completion queue entries are double-sized; the offset
into the descriptor to read the HW timestamp is incorrect when in that
mode.

Brett Creeley (6):
  ionic: Allow the first devcmd to trigger deferred probe
  ionic: Handle failures from ionic_reset() when relevant
  ionic: Fix unexpected dev_cmd failures
  ionic: Fix check in ionic_get_link_ext_stats
  ionic: fix adminq use-after-free on command timeout
  ionic: service adminq CQ before cancelling to avoid false timeouts

Prabu Thayalan (1):
  ionic: fix completion descriptor access with 2x desc size

 .../ethernet/pensando/ionic/ionic_bus_pci.c   |  8 +-
 .../ethernet/pensando/ionic/ionic_ethtool.c   |  6 +-
 .../net/ethernet/pensando/ionic/ionic_lif.c   |  4 +-
 .../net/ethernet/pensando/ionic/ionic_main.c  | 74 +++++++++++++++++--
 .../net/ethernet/pensando/ionic/ionic_txrx.c  | 27 +++----
 5 files changed, 96 insertions(+), 23 deletions(-)


base-commit: e728258debd553c95d2e70f9cd97c9fde27c7130
-- 
2.17.1

[PATCH net 1/7] ionic: Allow the first devcmd to trigger deferred probe

[Eric Joyner]

From: Brett Creeley <brett.creeley@amd.com>

The BAR signature can become visible before firmware is ready to process
device commands. In that window the first devcmd may fail even though
the device is present.

Treat a failure of the first devcmd as deferred probe and return
-EPROBE_DEFER, so probe is retried after firmware initialization
completes.

Also reduce log severity for reset-devcmd failures in this path: these
early failures are expected during firmware bring-up and should not emit
the standard devcmd failure messages.

A possible follow-up is to rework ionic_reset() to retry on -EAGAIN and
-EAGAIN/ETIMEDOUT style transient failures, but this change keeps the
current reset flow unchanged.

Fixes: fbfb8031533c ("ionic: Add hardware init and device commands")
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 8 ++++++--
 drivers/net/ethernet/pensando/ionic/ionic_main.c    | 8 ++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 05f19489ec5c..59ce35404e53 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -260,7 +260,8 @@ static int ionic_setup_one(struct ionic *ionic)
 	/* Configure the device */
 	err = ionic_setup(ionic);
 	if (err) {
-		dev_err(dev, "Cannot setup device: %d, aborting\n", err);
+		if (err != -EPROBE_DEFER)
+			dev_err(dev, "Cannot setup device: %d, aborting\n", err);
 		goto err_out_clear_pci;
 	}
 	pci_set_master(pdev);
@@ -335,8 +336,11 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 #endif
 
 	err = ionic_setup_one(ionic);
-	if (err)
+	if (err) {
+		if (err == -EPROBE_DEFER)
+			dev_info(dev, "Device isn't ready, deferring probe\n");
 		goto err_out;
+	}
 
 	/* Allocate and init the LIF */
 	err = ionic_lif_size(ionic);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 3c5200e2fdb7..91f89b9ff807 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -603,7 +603,11 @@ int ionic_setup(struct ionic *ionic)
 	err = ionic_dev_setup(ionic);
 	if (err)
 		return err;
-	ionic_reset(ionic);
+
+	err = ionic_reset(ionic);
+	/* firmware may not be ready to respond yet */
+	if (err == -EAGAIN || err == -ETIMEDOUT)
+		return -EPROBE_DEFER;
 
 	return 0;
 }
@@ -687,7 +691,7 @@ int ionic_reset(struct ionic *ionic)
 
 	mutex_lock(&ionic->dev_cmd_lock);
 	ionic_dev_cmd_reset(idev);
-	err = ionic_dev_cmd_wait(ionic, DEVCMD_TIMEOUT);
+	err = ionic_dev_cmd_wait_nomsg(ionic, DEVCMD_TIMEOUT);
 	mutex_unlock(&ionic->dev_cmd_lock);
 
 	return err;
-- 
2.17.1

[PATCH net 2/7] ionic: Handle failures from ionic_reset() when relevant

[Eric Joyner]

From: Brett Creeley <brett.creeley@amd.com>

If ionic_reset() fails, then the device either wasn't ready to be
communicated with, the firmware is down, and/or the devcmd path is
already torn down. For teardown/remove cases we can ignore the
result of ionic_reset(). However, for any setup cases, we should
take the result seriously.

Note, older firmware always returns success for IONIC_CMD_RESET, so this
change will not break those. However, newer firmware may return failure
if the IONIC_CMD_RESET dev cmd fails.

Fixes: 8097a2f3d21a ("ionic: Reset LIF device while restarting LIF")
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 637e635bbf03..db4bbeda0b29 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -3473,7 +3473,9 @@ static void ionic_lif_handle_fw_up(struct ionic_lif *lif)
 	 * just need to reanimate it.
 	 */
 	ionic_init_devinfo(ionic);
-	ionic_reset(ionic);
+	err = ionic_reset(ionic);
+	if (err)
+		goto err_out;
 	err = ionic_identify(ionic);
 	if (err)
 		goto err_out;
-- 
2.17.1

[PATCH net 3/7] ionic: Fix unexpected dev_cmd failures

[Eric Joyner]

From: Brett Creeley <brett.creeley@amd.com>

When polling for a devcmd completion it's possible for the driver to
timeout the command even if the dev_cmd has completed. This can cause
unexpected failures and device probe to fail. Fix this by reading
the dev_cmd's done bit one last time after breaking out of the poll
loop.

Fixes: fbfb8031533c ("ionic: Add hardware init and device commands")
Suggested-by: Neel Patel <neel.patel@amd.com>
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 91f89b9ff807..810cef0fec93 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -541,6 +541,10 @@ static int __ionic_dev_cmd_wait(struct ionic *ionic, unsigned long max_seconds,
 	}
 	duration = jiffies - start_time;
 
+	/* one final check to prevent unexpected timeout */
+	if (!done)
+		done = ionic_dev_cmd_done(idev);
+
 	dev_dbg(ionic->dev, "DEVCMD %s (%d) done=%d took %ld secs (%ld jiffies)\n",
 		ionic_opcode_to_str(opcode), opcode,
 		done, duration / HZ, duration);
-- 
2.17.1

[PATCH net 4/7] ionic: Fix check in ionic_get_link_ext_stats

[Eric Joyner]

From: Brett Creeley <brett.creeley@amd.com>

The current check will fail if SR-IOV is not initialized for the
physical function; this is because is_physfn is 0 if sriov_init() isn't
run or fails. Change the check that prevents getting the link down count
to use is_virtfn instead so that VFs don't get this functionality, which
was the original intent.

Fixes: 132b4ebfa090 ("ionic: add support for ethtool extended stat link_down_count")
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index 78a802eb159f..296f831a514d 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -116,8 +116,10 @@ static void ionic_get_link_ext_stats(struct net_device *netdev,
 {
 	struct ionic_lif *lif = netdev_priv(netdev);
 
-	if (lif->ionic->pdev->is_physfn)
-		stats->link_down_events = lif->link_down_count;
+	if (lif->ionic->pdev->is_virtfn)
+		return;
+
+	stats->link_down_events = lif->link_down_count;
 }
 
 static int ionic_get_link_ksettings(struct net_device *netdev,
-- 
2.17.1

[PATCH net 5/7] ionic: fix adminq use-after-free on command timeout

[Eric Joyner]

From: Brett Creeley <brett.creeley@amd.com>

When ionic_adminq_wait() times out or detects FW reset, it
returns an error to the caller, whose ionic_admin_ctx is typically
on the stack. However, desc_info->ctx in the adminq still points
to that ctx. If ionic_adminq_service() later runs in NAPI context,
it dereferences the stale pointer to copy the completion and call
complete_all(), causing a use-after-free.

The timeout path partially addressed this via ionic_adminq_flush()
in ionic_adminq_check_err(), which NULLs all pending desc_info->ctx
entries. But there is a race window between the timeout detection
and the flush where NAPI could fire and access the stale ctx. The
FW reset path had no protection at all and returned directly
without clearing desc_info->ctx.

Add ionic_adminq_cancel() which takes adminq_lock and NULLs
desc_info->ctx for the specific context being cancelled. This
coordinates with ionic_adminq_service() which also runs under the
same lock. Call it from both error paths in ionic_adminq_wait()
before returning.

Fixes: 938962d55229 ("ionic: Add adminq action")
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 .../net/ethernet/pensando/ionic/ionic_main.c  | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 810cef0fec93..0971ca4d6650 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -190,6 +190,32 @@ static const char *ionic_opcode_to_str(enum ionic_cmd_opcode opcode)
 	}
 }
 
+static void ionic_adminq_cancel(struct ionic_lif *lif,
+				struct ionic_admin_ctx *ctx)
+{
+	struct ionic_admin_desc_info *desc_info;
+	unsigned long irqflags;
+	struct ionic_queue *q;
+	int i;
+
+	spin_lock_irqsave(&lif->adminq_lock, irqflags);
+	if (!lif->adminqcq) {
+		spin_unlock_irqrestore(&lif->adminq_lock, irqflags);
+		return;
+	}
+
+	q = &lif->adminqcq->q;
+
+	for (i = 0; i < q->num_descs; i++) {
+		desc_info = &q->admin_info[i];
+		if (desc_info->ctx == ctx) {
+			desc_info->ctx = NULL;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&lif->adminq_lock, irqflags);
+}
+
 static void ionic_adminq_flush(struct ionic_lif *lif)
 {
 	struct ionic_admin_desc_info *desc_info;
@@ -448,6 +474,7 @@ int ionic_adminq_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx,
 			if (do_msg)
 				netdev_warn(netdev, "%s (%d) interrupted, FW in reset\n",
 					    name, ctx->cmd.cmd.opcode);
+			ionic_adminq_cancel(lif, ctx);
 			ctx->comp.comp.status = IONIC_RC_ERROR;
 			return -ENXIO;
 		}
@@ -458,6 +485,9 @@ int ionic_adminq_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx,
 	dev_dbg(lif->ionic->dev, "%s: elapsed %d msecs\n",
 		__func__, jiffies_to_msecs(time_done - time_start));
 
+	if (time_after_eq(time_done, time_limit))
+		ionic_adminq_cancel(lif, ctx);
+
 	return ionic_adminq_check_err(lif, ctx,
 				      time_after_eq(time_done, time_limit),
 				      do_msg);
-- 
2.17.1

[PATCH net 6/7] ionic: service adminq CQ before cancelling to avoid false timeouts

[Eric Joyner]

From: Brett Creeley <brett.creeley@amd.com>

When ionic_adminq_wait() hits its timeout, it's possible the
firmware has already written the completion to the CQ but
ionic_adminq_service() was never scheduled via NAPI to process it.
In this case the command is falsely reported as timed out even
though it completed successfully.

Fix this by renaming ionic_adminq_cancel() to
ionic_adminq_service_or_cancel() and having it call
ionic_cq_service() under the adminq_lock before checking whether
the context needs to be cancelled.  ionic_cq_service() invokes
ionic_adminq_service() which will process any pending CQ entries,
copy the completion data, call complete_all(), and clear
desc_info->ctx for completed commands.

After servicing, check completion_done() on the ctx.  If the
completion was found and processed, return false (not cancelled)
so ionic_adminq_wait() can use the actual firmware result instead
of reporting a false timeout.  If the completion was not found,
fall through to the existing cancel path that NULLs desc_info->ctx
and return true (cancelled).

Fixes: 938962d55229 ("ionic: Add adminq action")
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 .../net/ethernet/pensando/ionic/ionic_main.c  | 42 +++++++++++++++----
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 0971ca4d6650..708c7e4c578b 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -190,8 +190,8 @@ static const char *ionic_opcode_to_str(enum ionic_cmd_opcode opcode)
 	}
 }
 
-static void ionic_adminq_cancel(struct ionic_lif *lif,
-				struct ionic_admin_ctx *ctx)
+static bool ionic_adminq_service_or_cancel(struct ionic_lif *lif,
+					   struct ionic_admin_ctx *ctx)
 {
 	struct ionic_admin_desc_info *desc_info;
 	unsigned long irqflags;
@@ -201,9 +201,29 @@ static void ionic_adminq_cancel(struct ionic_lif *lif,
 	spin_lock_irqsave(&lif->adminq_lock, irqflags);
 	if (!lif->adminqcq) {
 		spin_unlock_irqrestore(&lif->adminq_lock, irqflags);
-		return;
+		return true;
+	}
+
+	/* Service the CQ to pick up any completions that the FW has
+	 * sent but NAPI hasn't processed yet.  This will call
+	 * complete_all() on any matching contexts, including ours.
+	 */
+	ionic_cq_service(&lif->adminqcq->cq, lif->adminqcq->cq.num_descs,
+			 ionic_adminq_service, NULL, NULL);
+
+	/* If the completion was serviced above, the ctx will have been
+	 * completed and its desc_info->ctx cleared by
+	 * ionic_adminq_service().  Check and return not-cancelled.
+	 */
+	if (completion_done(&ctx->work)) {
+		spin_unlock_irqrestore(&lif->adminq_lock, irqflags);
+		return false;
 	}
 
+	/* The command is still pending, cancel it by clearing
+	 * desc_info->ctx so ionic_adminq_service() won't touch
+	 * the caller's ctx after we return.
+	 */
 	q = &lif->adminqcq->q;
 
 	for (i = 0; i < q->num_descs; i++) {
@@ -214,6 +234,8 @@ static void ionic_adminq_cancel(struct ionic_lif *lif,
 		}
 	}
 	spin_unlock_irqrestore(&lif->adminq_lock, irqflags);
+
+	return true;
 }
 
 static void ionic_adminq_flush(struct ionic_lif *lif)
@@ -444,6 +466,7 @@ int ionic_adminq_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx,
 	unsigned long time_start;
 	unsigned long time_done;
 	unsigned long remaining;
+	bool timed_out = false;
 	const char *name;
 
 	name = ionic_opcode_to_str(ctx->cmd.cmd.opcode);
@@ -474,7 +497,7 @@ int ionic_adminq_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx,
 			if (do_msg)
 				netdev_warn(netdev, "%s (%d) interrupted, FW in reset\n",
 					    name, ctx->cmd.cmd.opcode);
-			ionic_adminq_cancel(lif, ctx);
+			ionic_adminq_service_or_cancel(lif, ctx);
 			ctx->comp.comp.status = IONIC_RC_ERROR;
 			return -ENXIO;
 		}
@@ -485,12 +508,15 @@ int ionic_adminq_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx,
 	dev_dbg(lif->ionic->dev, "%s: elapsed %d msecs\n",
 		__func__, jiffies_to_msecs(time_done - time_start));
 
+	/* If the wait timed out, attempt to service the CQ and cancel
+	 * the ctx.  If ionic_adminq_service() completed the ctx between
+	 * timeout detection and taking the lock, cancel returns false
+	 * and we avoid a false timeout.
+	 */
 	if (time_after_eq(time_done, time_limit))
-		ionic_adminq_cancel(lif, ctx);
+		timed_out = ionic_adminq_service_or_cancel(lif, ctx);
 
-	return ionic_adminq_check_err(lif, ctx,
-				      time_after_eq(time_done, time_limit),
-				      do_msg);
+	return ionic_adminq_check_err(lif, ctx, timed_out, do_msg);
 }
 
 static int __ionic_adminq_post_wait(struct ionic_lif *lif,
-- 
2.17.1

[PATCH net 7/7] ionic: fix completion descriptor access with 2x desc size

[Eric Joyner]

From: Prabu Thayalan <prabu.ponrajthayalan@amd.com>

The old ionic_rx_service() and ionic_tx_service() used array
indexing to access completion descriptors:

    comp = &((struct ionic_rxq_comp *)cq->base)[cq->tail_idx];

This assumes the stride is sizeof(struct ionic_rxq_comp) = 16 bytes.
However, when the IONIC_Q_F_2X_CQ_DESC flag is set, the actual
completion descriptor size is 32 bytes (2 * sizeof(comp)), and the
completion itself is located at the end of that 32-byte slot. Array
indexing with a 16-byte stride would access the wrong offset.

Use pointer arithmetic that accounts for the actual descriptor size
from cq->desc_size:

    comp = cq->base +
           cq->desc_size * cq->tail_idx +
           cq->desc_size - sizeof(*comp);

This correctly calculates the completion location regardless of
descriptor size. For the common case where desc_size equals
sizeof(*comp), use array indexing in a likely() fast path to avoid
performance regression.

Fixes: 0ec9f6669a7d ("ionic: add handling of larger descriptors")
Signed-off-by: Prabu Thayalan <prabu.ponrajthayalan@amd.com>
Signed-off-by: Eric Joyner <eric.joyner@amd.com>
---
 .../net/ethernet/pensando/ionic/ionic_txrx.c  | 27 ++++++++++---------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index 301ebee2fdc5..27a113d63d28 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -701,11 +701,7 @@ static void ionic_rx_clean(struct ionic_queue *q,
 		__le64 *cq_desc_hwstamp;
 		u64 hwstamp;
 
-		cq_desc_hwstamp =
-			(void *)comp +
-			qcq->cq.desc_size -
-			sizeof(struct ionic_rxq_comp) -
-			IONIC_HWSTAMP_CQ_NEGOFFSET;
+		cq_desc_hwstamp = (void *)comp - IONIC_HWSTAMP_CQ_NEGOFFSET;
 
 		hwstamp = le64_to_cpu(*cq_desc_hwstamp);
 
@@ -729,7 +725,12 @@ static bool __ionic_rx_service(struct ionic_cq *cq, struct bpf_prog *xdp_prog)
 	struct ionic_queue *q = cq->bound_q;
 	struct ionic_rxq_comp *comp;
 
-	comp = &((struct ionic_rxq_comp *)cq->base)[cq->tail_idx];
+	if (likely(cq->desc_size == sizeof(*comp)))
+		comp = &((struct ionic_rxq_comp *)cq->base)[cq->tail_idx];
+	else
+		comp = cq->base +
+		       cq->desc_size * cq->tail_idx +
+		       cq->desc_size - sizeof(*comp);
 
 	if (!color_match(comp->pkt_type_color, cq->done_color))
 		return false;
@@ -1180,7 +1181,6 @@ static void ionic_tx_clean(struct ionic_queue *q,
 			   bool in_napi)
 {
 	struct ionic_tx_stats *stats = q_to_tx_stats(q);
-	struct ionic_qcq *qcq = q_to_qcq(q);
 	struct sk_buff *skb;
 
 	if (desc_info->xdpf) {
@@ -1205,11 +1205,7 @@ static void ionic_tx_clean(struct ionic_queue *q,
 			__le64 *cq_desc_hwstamp;
 			u64 hwstamp;
 
-			cq_desc_hwstamp =
-				(void *)comp +
-				qcq->cq.desc_size -
-				sizeof(struct ionic_txq_comp) -
-				IONIC_HWSTAMP_CQ_NEGOFFSET;
+			cq_desc_hwstamp = (void *)comp - IONIC_HWSTAMP_CQ_NEGOFFSET;
 
 			hwstamp = le64_to_cpu(*cq_desc_hwstamp);
 
@@ -1244,7 +1240,12 @@ static bool ionic_tx_service(struct ionic_cq *cq,
 	unsigned int pkts = 0;
 	u16 index;
 
-	comp = &((struct ionic_txq_comp *)cq->base)[cq->tail_idx];
+	if (likely(cq->desc_size == sizeof(*comp)))
+		comp = &((struct ionic_txq_comp *)cq->base)[cq->tail_idx];
+	else
+		comp = cq->base +
+		       cq->desc_size * cq->tail_idx +
+		       cq->desc_size - sizeof(*comp);
 
 	if (!color_match(comp->color, cq->done_color))
 		return false;
-- 
2.17.1

Re: [PATCH net 5/7] ionic: fix adminq use-after-free on command timeout

[Eric Joyner]

On 4/29/2026 2:00 PM, Joyner, Eric wrote:
> From: Brett Creeley <brett.creeley@amd.com>
> 
> When ionic_adminq_wait() times out or detects FW reset, it
> returns an error to the caller, whose ionic_admin_ctx is typically
> on the stack. However, desc_info->ctx in the adminq still points
> to that ctx. If ionic_adminq_service() later runs in NAPI context,
> it dereferences the stale pointer to copy the completion and call
> complete_all(), causing a use-after-free.
> 
> The timeout path partially addressed this via ionic_adminq_flush()
> in ionic_adminq_check_err(), which NULLs all pending desc_info->ctx
> entries. But there is a race window between the timeout detection
> and the flush where NAPI could fire and access the stale ctx. The
> FW reset path had no protection at all and returned directly
> without clearing desc_info->ctx.
> 
> Add ionic_adminq_cancel() which takes adminq_lock and NULLs
> desc_info->ctx for the specific context being cancelled. This
> coordinates with ionic_adminq_service() which also runs under the
> same lock. Call it from both error paths in ionic_adminq_wait()
> before returning.
> 
> Fixes: 938962d55229 ("ionic: Add adminq action")
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Brett Creeley <brett.creeley@amd.com>
> Signed-off-by: Eric Joyner <eric.joyner@amd.com>
> ---
>  .../net/ethernet/pensando/ionic/ionic_main.c  | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> index 810cef0fec93..0971ca4d6650 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> @@ -190,6 +190,32 @@ static const char *ionic_opcode_to_str(enum ionic_cmd_opcode opcode)
>  	}
>  }
>  
> +static void ionic_adminq_cancel(struct ionic_lif *lif,
> +				struct ionic_admin_ctx *ctx)
> +{
> +	struct ionic_admin_desc_info *desc_info;
> +	unsigned long irqflags;
> +	struct ionic_queue *q;
> +	int i;
> +
> +	spin_lock_irqsave(&lif->adminq_lock, irqflags);
> +	if (!lif->adminqcq) {
> +		spin_unlock_irqrestore(&lif->adminq_lock, irqflags);
> +		return;
> +	}
> +
> +	q = &lif->adminqcq->q;
> +
> +	for (i = 0; i < q->num_descs; i++) {
> +		desc_info = &q->admin_info[i];
> +		if (desc_info->ctx == ctx) {
> +			desc_info->ctx = NULL;
> +			break;
> +		}
> +	}
> +	spin_unlock_irqrestore(&lif->adminq_lock, irqflags);
> +}
> +
>  static void ionic_adminq_flush(struct ionic_lif *lif)
>  {
>  	struct ionic_admin_desc_info *desc_info;
> @@ -448,6 +474,7 @@ int ionic_adminq_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx,
>  			if (do_msg)
>  				netdev_warn(netdev, "%s (%d) interrupted, FW in reset\n",
>  					    name, ctx->cmd.cmd.opcode);
> +			ionic_adminq_cancel(lif, ctx);
>  			ctx->comp.comp.status = IONIC_RC_ERROR;
>  			return -ENXIO;
>  		}
> @@ -458,6 +485,9 @@ int ionic_adminq_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx,
>  	dev_dbg(lif->ionic->dev, "%s: elapsed %d msecs\n",
>  		__func__, jiffies_to_msecs(time_done - time_start));
>  
> +	if (time_after_eq(time_done, time_limit))
> +		ionic_adminq_cancel(lif, ctx);
> +
>  	return ionic_adminq_check_err(lif, ctx,
>  				      time_after_eq(time_done, time_limit),
>  				      do_msg);

I took a look at the Sashiko output for patches 5 and 6, and it echoed concerns that we found
internally around ionic_adminq_cancel() and ionic_adminq_flush(). We might need to rework at
least those two.

https://sashiko.dev/#/message/20260429210007.40015-7-eric.joyner%40amd.com

- Eric