[PATCH net v2] net/mlx5: Don't return firmware-owned command mailboxes to the DMA pool

[Danielle Costantino <dcostantino@meta.com>]

On a command timeout mlx5_cmd_comp_handler(forced) keeps the entry and its
queue slot, because firmware may still complete the command and write to
ent->lay. cmd_exec() and the callback path free the mailboxes anyway,
while ent->lay->{in_ptr,out_ptr} still point at them.

dma_pool keeps its free list node in the first 16 bytes of the block,
which for mlx5 is the start of the command payload, so a late firmware
write corrupts the allocator:

  Unable to handle kernel paging request at virtual address
  0007c830040001a0
  pc : dma_pool_alloc+0x48/0x430   lr : mlx5_alloc_cmd_msg+0x154/0x318
  Call trace:
   dma_pool_alloc+0x48/0x430 (P)
   mlx5_alloc_cmd_msg+0x154/0x318
   cmd_exec+0x24c/0xb28
   mlx5_access_reg+0xe8/0x1c8

Two crash dumps show the aliasing: 31 of 32 slots held an entry with
ret == -ETIMEDOUT, 28 of 31 shared one ent->lay->out_ptr while every
in_ptr was distinct, and pool->next_block held a non-kernel address.

Transfer mailbox ownership to the entry and release it on its final put,
on both the synchronous and the callback path. For a real completion that
put is normally the last one, so nothing moves in practice.

An entry firmware never completes keeps its mailboxes for the lifetime of
the device. dma_pool_destroy() then reports the pool busy and declines to
free its pages, which is the memory safe outcome: the pages stay mapped,
so a late write lands there rather than in memory handed back to the
allocator.

Fixes: 73dd3a4839c1 ("net/mlx5: Avoid using pending command interface slots")
Cc: stable@vger.kernel.org
Signed-off-by: Danielle Costantino <dcostantino@meta.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 32 +++++++++++++++----
 include/linux/mlx5/driver.h                   |  1 +
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
index 84583dc5eb1c0..4051f97b2ae12 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
@@ -185,6 +185,10 @@ static void cmd_free_index(struct mlx5_cmd *cmd, int idx)
 	set_bit(idx, &cmd->vars.bitmask);
 }
 
+static void free_msg(struct mlx5_core_dev *dev, struct mlx5_cmd_msg *msg);
+static void mlx5_free_cmd_msg(struct mlx5_core_dev *dev,
+			      struct mlx5_cmd_msg *msg);
+
 static void cmd_ent_get(struct mlx5_cmd_work_ent *ent)
 {
 	refcount_inc(&ent->refcnt);
@@ -193,8 +197,11 @@ static void cmd_ent_get(struct mlx5_cmd_work_ent *ent)
 static void cmd_ent_put(struct mlx5_cmd_work_ent *ent)
 {
 	struct mlx5_cmd *cmd = ent->cmd;
+	struct mlx5_core_dev *dev;
 	unsigned long flags;
 
+	dev = container_of(cmd, struct mlx5_core_dev, cmd);
+
 	spin_lock_irqsave(&cmd->alloc_lock, flags);
 	if (!refcount_dec_and_test(&ent->refcnt)) {
 		spin_unlock_irqrestore(&cmd->alloc_lock, flags);
@@ -207,6 +214,11 @@ static void cmd_ent_put(struct mlx5_cmd_work_ent *ent)
 	}
 	spin_unlock_irqrestore(&cmd->alloc_lock, flags);
 
+	if (ent->own_msgs) {
+		mlx5_free_cmd_msg(dev, ent->out);
+		free_msg(dev, ent->in);
+	}
+
 	cmd_free_ent(ent);
 }
 
@@ -958,10 +970,6 @@ static void cb_timeout_handler(struct work_struct *work)
 	cmd_ent_put(ent); /* for the cmd_ent_get() took on schedule delayed work */
 }
 
-static void free_msg(struct mlx5_core_dev *dev, struct mlx5_cmd_msg *msg);
-static void mlx5_free_cmd_msg(struct mlx5_core_dev *dev,
-			      struct mlx5_cmd_msg *msg);
-
 static bool opcode_allowed(struct mlx5_cmd *cmd, u16 opcode)
 {
 	if (cmd->allowed_opcode == CMD_ALLOWED_OPCODE_ALL)
@@ -1313,7 +1321,12 @@ static int mlx5_cmd_invoke(struct mlx5_core_dev *dev, struct mlx5_cmd_msg *in,
 		return 0; /* mlx5_cmd_comp_handler() will put(ent) */
 
 	err = wait_func(dev, ent);
-	if (err == -ETIMEDOUT || err == -ECANCELED || err == -EBUSY)
+	if (err == -ETIMEDOUT) {
+		/* firmware may still DMA into the mailboxes; keep them */
+		ent->own_msgs = true;
+		goto out_free;
+	}
+	if (err == -ECANCELED || err == -EBUSY)
 		goto out_free;
 
 	ds = ent->ts2 - ent->ts1;
@@ -1816,8 +1829,10 @@ static void mlx5_cmd_comp_handler(struct mlx5_core_dev *dev, u64 vec, bool force
 								 ent->out,
 								 ent->uout_size);
 
-				mlx5_free_cmd_msg(dev, ent->out);
-				free_msg(dev, ent->in);
+				/* firmware may still DMA into the mailboxes;
+				 * keep them
+				 */
+				ent->own_msgs = true;
 
 				/* final consumer is done, release ent */
 				cmd_ent_put(ent);
@@ -2012,6 +2027,9 @@ static int cmd_exec(struct mlx5_core_dev *dev, void *in, int in_size, void *out,
 	if (callback && !err)
 		return 0;
 
+	if (err == -ETIMEDOUT) /* the command entry owns the mailboxes now */
+		goto out_up;
+
 	if (err > 0) /* Failed in FW, command didn't execute */
 		err = deliv_status_to_err(err);
 
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 83d0a83bbfbca..9fcbc6070869f 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -855,6 +855,7 @@ struct mlx5_cmd_work_ent {
 	u64			ts2;
 	u16			op;
 	bool			polling;
+	bool			own_msgs;
 	/* Track the max comp handlers */
 	refcount_t              refcnt;
 };

base-commit: 9c572a83037a7dcd653ba3a9cc468c16b857d0c9