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

[Eric Joyner <eric.joyner@amd.com>]

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