Re: [PATCH net-next v6 11/15] bnxt_en: Add crypto MPC transmit/completion infrastructure

Responding to Sashiko comments:
https://sashiko.dev/#/patchset/20260810051358.1244418-1-michael.chan%40broadcom.com

On Sun, Aug 9, 2026 at 10:15 PM Michael Chan <[email protected]> wrote:
> +int bnxt_xmit_crypto_cmd(struct bnxt *bp, struct bnxt_tx_ring_info *txr,
> +                        void *cmd, unsigned int len, unsigned int tmo)

> +               spin_lock_bh(&txr->tx_lock);
> +               rc = bnxt_start_xmit_mpc(bp, txr, cmd, len, handle);
Sashiko:
Since bnxt_start_xmit_mpc() only checks txr->dev_state against
BNXT_DEV_STATE_CLOSING and lacks a check for the global BNXT_STATE_OPEN
flag, can this proceed after the device is closed?
When the device is closed, bnxt_close_nic() frees the MPC ring memory but
does not update dev_state to BNXT_DEV_STATE_CLOSING. If a caller
asynchronously invokes this while the device is being closed, could it end
up writing to the freed tx_desc_ring memory and cause a use-after-free?

Response:
BNXT_DEV_STATE_CLOSING is set during MPC timeout/reset, not during
close.  During close, it will wait for all pending kTLS MPC commands
to finish or abort before proceeding to free the MPC rings.

> +static bool bnxt_disable_mpc_ring(struct bnxt_mpc_info *mpc, int mpc_ring)
> +{
> +       struct bnxt_tx_ring_info *txr;
> +       bool disabled = false;
> +       int i;
> +
> +       for (i = 0; i < BNXT_MPC_TYPE_MAX; i++) {
> +               if (mpc_ring >= mpc->mpc_ring_count[i])
> +                       continue;
> +               txr = &mpc->mpc_rings[i][mpc_ring];
> +               spin_lock_bh(&txr->tx_lock);
> +               if (!READ_ONCE(txr->dev_state)) {
> +                       disabled = true;
> +                       WRITE_ONCE(txr->dev_state, BNXT_DEV_STATE_CLOSING);
> +               }
> +               spin_unlock_bh(&txr->tx_lock);
Sashiko:
If the first ring type (e.g., i=0) is already marked as CLOSING from a
previous timeout, the disabled variable will remain false. Does breaking
early here leave subsequent ring types active (dev_state == 0)?
Concurrently, the workqueue task bnxt_mpc_ring_reset_task() observes the
first ring as CLOSING and tears down all ring types sharing the same index
via bnxt_mpc_ring_stop().
Since subsequent ring types are still active, could this allow concurrent
calls to bnxt_xmit_crypto_cmd() to write to the ring memory while it is
being freed without lock synchronization?

Response:
This is indeed possible if we are in the middle here and a concurrent
workqueue is resetting another MPC ring and sees that this ring is in
the middle of being disabled.  I should be able to come up with a
simple fix for this.