[PATCH v5 2/2] mfd: loongson-se: Fix miscellaneous issues

[Qunqin Zhao]

Address multiple historical driver issues discovered by the Sashiko
Automation system within the loongson_se_probe() initialization flow
and the driver's interrupt service routines [1].

- Add an explicit bounds check in se_irq_handler() before accessing
  the engines array to prevent out-of-bounds memory writes.

- Switch from devm_kmalloc() to devm_kzalloc() and explicitly
  initialize all engine completion structures in probe() to avoid a
  kernel panic from complete() dereferencing a NULL wait head when a
  spurious interrupt fires before child drivers call
  loongson_se_init_engine().

- Replace engine_init_lock with a broader cmd_lock mutex that
  serializes all command submissions, and move the lock into
  loongson_se_send_controller_cmd() and
  loongson_se_send_engine_cmd() to cover the full register write +
  poll + wait sequence.

- Drop the spin_lock_irq from loongson_se_poll() so that interrupts
  are not disabled for up to 10 ms during the poll.  Keep the
  readl_relaxed_poll_timeout_atomic() busy-wait to avoid scheduling
  latency for fast hardware completions.

- Add reinit_completion() to loongson_se_send_controller_cmd() and
  loongson_se_send_engine_cmd() before waiting to prevent stale
  completions from falsely returning success after a signal
  interruption.

- Fix EPROBE_DEFER handling: propagate the error directly from
  platform_irq_count() instead of overwriting it with ENODEV so that
  probe deferral works when the interrupt provider is not yet ready.

- Validate dmam_size from firmware against the minimum required size
  to prevent command buffers from pointing outside the allocated
  DMA region.

- Return the error code from devm_request_irq() instead of silently
  continuing to prevent an indefinite hang.

- Disable hardware interrupts in the probe error path when
  loongson_se_init() fails to prevent an unhandled interrupt storm.

- Add a loongson_se_stop() cleanup handler registered with
  devm_add_action_or_reset() to send SE_CMD_STOP to the controller
  and mask all interrupts during device removal.  Using devres
  ensures that child MFD devices are unbound before the controller
  is stopped.  The STOP command uses a non-interruptible wait to
  avoid leaving hardware running while DMA buffers are freed.

- Zero-initialize the local controller command structure in
  loongson_se_init() to prevent uninitialized stack memory from
  being written to device registers.

- Add the SE_CMD_STOP command definition.

Link: https://lore.kernel.org/all/20260618095949.GB1672911@xxxxxxxxxx/ [1]
Fixes: e551fa3159e3 ("mfd: Add support for Loongson Security Engine chip controller")
Signed-off-by: Qunqin Zhao <zhaoqunqin@xxxxxxxxxxx>
---
 drivers/mfd/loongson-se.c       | 87 ++++++++++++++++++++++++++-------
 include/linux/mfd/loongson-se.h |  1 +
 2 files changed, 69 insertions(+), 19 deletions(-)

diff --git a/drivers/mfd/loongson-se.c b/drivers/mfd/loongson-se.c
index 7f552a8ee6..2c8afad1b0 100644
--- a/drivers/mfd/loongson-se.c
+++ b/drivers/mfd/loongson-se.c
@@ -28,7 +28,7 @@ struct loongson_se {
 	void *dmam_base;
 	int dmam_size;
 
-	struct mutex engine_init_lock;
+	struct mutex cmd_lock;
 	struct loongson_se_engine engines[SE_ENGINE_MAX];
 };
 
@@ -42,8 +42,6 @@ static int loongson_se_poll(struct loongson_se *se, u32 int_bit)
 	u32 status;
 	int err;
 
-	spin_lock_irq(&se->dev_lock);
-
 	/* Notify the controller that the engine needs to be started */
 	writel(int_bit, se->base + SE_L2SINT_SET);
 
@@ -52,8 +50,6 @@ static int loongson_se_poll(struct loongson_se *se, u32 int_bit)
 						!(status & int_bit),
 						1, LOONGSON_ENGINE_CMD_TIMEOUT_US);
 
-	spin_unlock_irq(&se->dev_lock);
-
 	return err;
 }
 
@@ -63,24 +59,40 @@ static int loongson_se_send_controller_cmd(struct loongson_se *se,
 	u32 *send_cmd = (u32 *)cmd;
 	int err, i;
 
+	mutex_lock(&se->cmd_lock);
+
+	reinit_completion(&se->cmd_completion);
+
 	for (i = 0; i < SE_SEND_CMD_REG_LEN; i++)
 		writel(send_cmd[i], se->base + SE_SEND_CMD_REG + i * 4);
 
 	err = loongson_se_poll(se, SE_INT_CONTROLLER);
 	if (err)
-		return err;
+		goto out;
+
+	err = wait_for_completion_interruptible(&se->cmd_completion);
 
-	return wait_for_completion_interruptible(&se->cmd_completion);
+out:
+	mutex_unlock(&se->cmd_lock);
+	return err;
 }
 
 int loongson_se_send_engine_cmd(struct loongson_se_engine *engine)
 {
+	int err;
+
+	mutex_lock(&engine->se->cmd_lock);
+
+	reinit_completion(&engine->completion);
+
 	/*
 	 * After engine initialization, the controller already knows
 	 * where to obtain engine commands from. Now all we need to
 	 * do is notify the controller that the engine needs to be started.
 	 */
-	int err = loongson_se_poll(engine->se, BIT(engine->id));
+	err = loongson_se_poll(engine->se, BIT(engine->id));
+
+	mutex_unlock(&engine->se->cmd_lock);
 
 	if (err)
 		return err;
@@ -97,7 +109,7 @@ struct loongson_se_engine *loongson_se_init_engine(struct device *dev, int id)
 
 	engine->se = se;
 	engine->id = id;
-	init_completion(&engine->completion);
+	reinit_completion(&engine->completion);
 
 	/* Divide DMA memory equally among all engines */
 	engine->buffer_size = se->dmam_size / SE_ENGINE_MAX;
@@ -113,8 +125,6 @@ struct loongson_se_engine *loongson_se_init_engine(struct device *dev, int id)
 	engine->command = se->dmam_base + id * (2 * SE_ENGINE_CMD_SIZE);
 	engine->command_ret = engine->command + SE_ENGINE_CMD_SIZE;
 
-	mutex_lock(&se->engine_init_lock);
-
 	/* Tell the controller where to find engine command */
 	cmd.command_id = SE_CMD_SET_ENGINE_CMDBUF;
 	cmd.info[0] = id;
@@ -124,8 +134,6 @@ struct loongson_se_engine *loongson_se_init_engine(struct device *dev, int id)
 	if (loongson_se_send_controller_cmd(se, &cmd))
 		engine = NULL;
 
-	mutex_unlock(&se->engine_init_lock);
-
 	return engine;
 }
 EXPORT_SYMBOL_GPL(loongson_se_init_engine);
@@ -155,7 +163,8 @@ static irqreturn_t se_irq_handler(int irq, void *dev_id)
 	/* For engines */
 	while (int_status) {
 		id = __ffs(int_status);
-		complete(&se->engines[id].completion);
+		if (id < SE_ENGINE_MAX)
+			complete(&se->engines[id].completion);
 		int_status &= ~BIT(id);
 		writel(BIT(id), se->base + SE_S2LINT_CL);
 	}
@@ -167,7 +176,7 @@ static irqreturn_t se_irq_handler(int irq, void *dev_id)
 
 static int loongson_se_init(struct loongson_se *se, dma_addr_t addr, int size)
 {
-	struct loongson_se_controller_cmd cmd;
+	struct loongson_se_controller_cmd cmd = {0};
 	int err;
 
 	cmd.command_id = SE_CMD_START;
@@ -188,6 +197,30 @@ static const struct mfd_cell engines[] = {
 	{ .name = "tpm_loongson" },
 };
 
+static void loongson_se_stop(void *data)
+{
+	struct loongson_se *se = data;
+	struct loongson_se_controller_cmd cmd = {0};
+	u32 *send_cmd = (u32 *)&cmd;
+	int i;
+
+	mutex_lock(&se->cmd_lock);
+
+	cmd.command_id = SE_CMD_STOP;
+
+	reinit_completion(&se->cmd_completion);
+
+	for (i = 0; i < SE_SEND_CMD_REG_LEN; i++)
+		writel(send_cmd[i], se->base + SE_SEND_CMD_REG + i * 4);
+
+	if (!loongson_se_poll(se, SE_INT_CONTROLLER))
+		wait_for_completion(&se->cmd_completion);
+
+	writel(0, se->base + SE_S2LINT_EN);
+
+	mutex_unlock(&se->cmd_lock);
+}
+
 static int loongson_se_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -195,19 +228,25 @@ static int loongson_se_probe(struct platform_device *pdev)
 	int nr_irq, irq, err, i;
 	dma_addr_t paddr;
 
-	se = devm_kmalloc(dev, sizeof(*se), GFP_KERNEL);
+	se = devm_kzalloc(dev, sizeof(*se), GFP_KERNEL);
 	if (!se)
 		return -ENOMEM;
 
 	dev_set_drvdata(dev, se);
 	init_completion(&se->cmd_completion);
 	spin_lock_init(&se->dev_lock);
-	mutex_init(&se->engine_init_lock);
+	mutex_init(&se->cmd_lock);
+
+	for (i = 0; i < SE_ENGINE_MAX; i++)
+		init_completion(&se->engines[i].completion);
 
 	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
 	if (device_property_read_u32(dev, "dmam_size", &se->dmam_size))
 		return -ENODEV;
 
+	if (se->dmam_size < (SE_ENGINE_MAX * 2 * SE_ENGINE_CMD_SIZE))
+		return -EINVAL;
+
 	se->dmam_base = dmam_alloc_coherent(dev, se->dmam_size, &paddr, GFP_KERNEL);
 	if (!se->dmam_base)
 		return -ENOMEM;
@@ -217,21 +256,31 @@ static int loongson_se_probe(struct platform_device *pdev)
 		return PTR_ERR(se->base);
 
 	nr_irq = platform_irq_count(pdev);
-	if (nr_irq <= 0)
+	if (nr_irq == 0)
 		return -ENODEV;
+	if (nr_irq < 0)
+		return nr_irq;
 
 	writel(SE_INT_ALL, se->base + SE_S2LINT_CL);
 
 	for (i = 0; i < nr_irq; i++) {
 		irq = platform_get_irq(pdev, i);
 		err = devm_request_irq(dev, irq, se_irq_handler, IRQF_SHARED, "loongson-se", se);
-		if (err)
+		if (err) {
 			dev_err(dev, "failed to request IRQ: %d\n", irq);
+			return err;
+		}
 	}
 
 	writel(SE_INT_ALL, se->base + SE_S2LINT_EN);
 
 	err = loongson_se_init(se, paddr, se->dmam_size);
+	if (err) {
+		writel(0, se->base + SE_S2LINT_EN);
+		return err;
+	}
+
+	err = devm_add_action_or_reset(dev, loongson_se_stop, se);
 	if (err)
 		return err;
 
diff --git a/include/linux/mfd/loongson-se.h b/include/linux/mfd/loongson-se.h
index 07afa0c252..8237ccab7b 100644
--- a/include/linux/mfd/loongson-se.h
+++ b/include/linux/mfd/loongson-se.h
@@ -9,6 +9,7 @@
 #define SE_SEND_CMD_REG_LEN		0x8
 /* Controller command ID */
 #define SE_CMD_START			0x0
+#define SE_CMD_STOP			0x1
 #define SE_CMD_SET_DMA			0x3
 #define SE_CMD_SET_ENGINE_CMDBUF	0x4
 
-- 
2.47.3