[PATCH v4 0/2] mfd: loongson-se: Fix miscellaneous issues and add multi-node support

[PATCH v4 0/2] mfd: loongson-se: Fix miscellaneous issues and add multi-node support

[Qunqin Zhao]

- Patch 1 focuses entirely on structural and error-path hardening for the
  pre-existing baseline code. This includes resolving critical issues such as
  uninitialized stack memory writes, engines array out-of-bounds writes.

  v4: new patch

- Patch 2 introduces the multi-node platform support.

  v4: Safely handling shared interrupt handler returns.
  v3: Using shared interrupts (IRQF_SHARED) instead of manually
      iterating through all devices to check for interrupts.


Qunqin Zhao (2):
  mfd: loongson-se: Fix miscellaneous issues
  mfd: loongson-se: Add multi-node support

 drivers/mfd/loongson-se.c       | 32 ++++++++++++++++++++++++++------
 include/linux/mfd/loongson-se.h |  1 +
 2 files changed, 27 insertions(+), 6 deletions(-)


base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
-- 
2.47.2

[PATCH v4 1/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 for 'id' before accessing the fixed-size
  'engines' array in se_irq_handler() to prevent potential out-of-bounds
  memory writes.

- Switch the allocations from devm_kmalloc() to devm_kzalloc() to
  guarantee the descriptor structures are properly zero-initialized.

- Return an error code from loongson_se_probe() if devm_request_irq()
  fails, preventing an indefinite hang during completion waits.

- Add a reinit_completion() before waiting on the completions to ensure
  proper hardware synchronization.

- Prevent writing uninitialized stack memory to device registers in
  loongson_se_init() by properly initializing local command structures.

- Add a .remove callback to the platform driver to properly halt the
  hardware.

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

diff --git a/drivers/mfd/loongson-se.c b/drivers/mfd/loongson-se.c
index 3902ba377d..aa34e663c4 100644
--- a/drivers/mfd/loongson-se.c
+++ b/drivers/mfd/loongson-se.c
@@ -85,6 +85,8 @@ int loongson_se_send_engine_cmd(struct loongson_se_engine *engine)
 	if (err)
 		return err;
 
+	reinit_completion(&engine->completion);
+
 	return wait_for_completion_interruptible(&engine->completion);
 }
 EXPORT_SYMBOL_GPL(loongson_se_send_engine_cmd);
@@ -150,7 +152,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);
 	}
@@ -162,7 +165,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;
@@ -190,7 +193,7 @@ 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;
 
@@ -220,8 +223,10 @@ static int loongson_se_probe(struct platform_device *pdev)
 	for (i = 0; i < nr_irq; i++) {
 		irq = platform_get_irq(pdev, i);
 		err = devm_request_irq(dev, irq, se_irq_handler, 0, "loongson-se", se);
-		if (err)
+		if (err) {
 			dev_err(dev, "failed to request IRQ: %d\n", irq);
+			return err;
+		}
 	}
 
 	err = loongson_se_init(se, paddr, se->dmam_size);
@@ -232,6 +237,15 @@ static int loongson_se_probe(struct platform_device *pdev)
 				    ARRAY_SIZE(engines), NULL, 0, NULL);
 }
 
+static void loongson_se_remove(struct platform_device *pdev)
+{
+	struct loongson_se *se = dev_get_drvdata(&pdev->dev);
+	struct loongson_se_controller_cmd cmd = {0};
+
+	cmd.command_id = SE_CMD_STOP;
+	loongson_se_send_controller_cmd(se, &cmd);
+}
+
 static const struct acpi_device_id loongson_se_acpi_match[] = {
 	{ "LOON0011", 0 },
 	{ }
@@ -240,6 +254,7 @@ MODULE_DEVICE_TABLE(acpi, loongson_se_acpi_match);
 
 static struct platform_driver loongson_se_driver = {
 	.probe   = loongson_se_probe,
+	.remove  = loongson_se_remove,
 	.driver  = {
 		.name  = "loongson-se",
 		.acpi_match_table = loongson_se_acpi_match,
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.2

Re: [PATCH v4 1/2] mfd: loongson-se: Fix miscellaneous issues

[Lee Jones]

Oof, that's a lot of findings.  Please review and remedy each.

/* Sashiko Automation: Issues Found (10 Findings) */

On Mon, 29 Jun 2026, Qunqin Zhao wrote:

> 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 for 'id' before accessing the fixed-size
>   'engines' array in se_irq_handler() to prevent potential out-of-bounds
>   memory writes.
> 
> - Switch the allocations from devm_kmalloc() to devm_kzalloc() to
>   guarantee the descriptor structures are properly zero-initialized.
> 
> - Return an error code from loongson_se_probe() if devm_request_irq()
>   fails, preventing an indefinite hang during completion waits.
> 
> - Add a reinit_completion() before waiting on the completions to ensure
>   proper hardware synchronization.
> 
> - Prevent writing uninitialized stack memory to device registers in
>   loongson_se_init() by properly initializing local command structures.
> 
> - Add a .remove callback to the platform driver to properly halt the
>   hardware.
> 
> Link: https://lore.kernel.org/all/20260618095949.GB1672911@google.com/ [1]
> Fixes: e551fa3159e3 ("mfd: Add support for Loongson Security Engine chip controller")
> Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn>
> ---
>  drivers/mfd/loongson-se.c       | 23 +++++++++++++++++++----
>  include/linux/mfd/loongson-se.h |  1 +
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mfd/loongson-se.c b/drivers/mfd/loongson-se.c
> index 3902ba377d..aa34e663c4 100644
> --- a/drivers/mfd/loongson-se.c
> +++ b/drivers/mfd/loongson-se.c
> @@ -85,6 +85,8 @@ int loongson_se_send_engine_cmd(struct loongson_se_engine *engine)
>  	if (err)
>  		return err;
>  
> +	reinit_completion(&engine->completion);
> +

[Severity: High]
If a fast hardware completion occurs between loongson_se_poll() signaling the
hardware to start and this reinit_completion(), the IRQ handler runs and sets
the completion to done. Will this reinit_completion() then immediately clear
the done state, causing the subsequent wait_for_completion_interruptible()
to block indefinitely?


>  	return wait_for_completion_interruptible(&engine->completion);
>  }

[Severity: High]
This is a pre-existing issue, but controller commands also rely on
wait_for_completion_interruptible() without reinitializing the completion
structure. If loongson_se_send_controller_cmd() is interrupted by a signal,
it returns -ERESTARTSYS, and the hardware eventually sets the completion state
to done. Will the next controller command see the completion as already done
and return success without actually waiting?



[Severity: High]
This is a pre-existing issue, but does loongson_se_poll() hold an
irq-disabling spinlock for up to 10 milliseconds?

loongson_se_poll() acquires a spinlock with spin_lock_irq(), and then uses
readl_relaxed_poll_timeout_atomic() with a 10,000 us timeout. Could this
cause severe scheduling latency spikes or trigger hard lockup panics if the
hardware is slow?



>  EXPORT_SYMBOL_GPL(loongson_se_send_engine_cmd);
> @@ -150,7 +152,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);
>  	}
> @@ -162,7 +165,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;
> @@ -190,7 +193,7 @@ 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);

[Severity: Critical]
This is a pre-existing issue, but switching to devm_kzalloc() without calling
init_completion() for the engines array leaves the completion structs
improperly initialized.

A zeroed struct list_head inside swait_queue_head evaluates as non-empty in
list_empty(). If an interrupt fires before a child driver calls
loongson_se_init_engine(), se_irq_handler() will pass the new bounds check
and call complete(), which dereferences a NULL list head. Can this cause a
deterministic kernel panic on spurious interrupts?



>  	if (!se)
>  		return -ENOMEM;
>  
> @@ -220,8 +223,10 @@ static int loongson_se_probe(struct platform_device *pdev)
>  	for (i = 0; i < nr_irq; i++) {
>  		irq = platform_get_irq(pdev, i);

[Severity: High]
This is a pre-existing issue, but the probe function returns -ENODEV if
platform_irq_count() fails earlier:

    nr_irq = platform_irq_count(pdev);
    if (nr_irq <= 0)
        return -ENODEV;

Does overwriting the return value of platform_irq_count() break probe deferral
if it returns -EPROBE_DEFER when the interrupt provider is not yet available?



[Severity: High]
This is a pre-existing issue, but earlier in probe, writel(SE_INT_ALL, ...)
enables all device interrupts before devm_request_irq() registers the handler.
If an interrupt fires in this window, no handler is present.

Additionally, if probe fails after this point, interrupts are never disabled.
Can this leave the hardware asserting unhandled interrupts and cause a system
lockup?


>  		err = devm_request_irq(dev, irq, se_irq_handler, 0, "loongson-se", se);
> -		if (err)
> +		if (err) {
>  			dev_err(dev, "failed to request IRQ: %d\n", irq);
> +			return err;
> +		}
>  	}
>  
>  	err = loongson_se_init(se, paddr, se->dmam_size);

[Severity: High]
This is a pre-existing issue, but is the driver blindly trusting the
firmware's dmam_size property without minimum bound checks?

In loongson_se_init_engine(), command buffers are placed at fixed offsets:

    engine->command = se->dmam_base + id * (2 * SE_ENGINE_CMD_SIZE);

If the provided dmam_size is smaller than the required space for all engines,
could these calculations yield pointers outside the allocated DMA buffer and
cause memory corruption?


> @@ -232,6 +237,15 @@ static int loongson_se_probe(struct platform_device *pdev)
>  				    ARRAY_SIZE(engines), NULL, 0, NULL);
>  }

[Severity: High]
This is a pre-existing issue, but if the devm_mfd_add_devices() call fails,
probe returns an error without sending SE_CMD_STOP to the already-started
controller.

Since devres will free the DMA buffers, can the still-running hardware
continue to access the freed memory and cause corruption?


>  
> +static void loongson_se_remove(struct platform_device *pdev)
> +{
> +	struct loongson_se *se = dev_get_drvdata(&pdev->dev);
> +	struct loongson_se_controller_cmd cmd = {0};
> +
> +	cmd.command_id = SE_CMD_STOP;
> +	loongson_se_send_controller_cmd(se, &cmd);

[Severity: High]
Because the child devices were registered via devm_mfd_add_devices(), their
unbind sequence happens later in the devres teardown phase, after this remove
callback returns.

If this halts the hardware controller before its child MFD devices are
unbound, will the child drivers still be fully active? Could any hardware
access by the children during their own remove callbacks fail or hang?



[Severity: High]
loongson_se_send_controller_cmd() waits using
wait_for_completion_interruptible(). If the thread executing the removal
receives a signal, the wait aborts immediately and returns -ERESTARTSYS.

Since this remove function ignores the return value, can devres free the
DMA buffers while the hardware is still processing the STOP command, leading
to DMA memory corruption?


> +}
> +
>  static const struct acpi_device_id loongson_se_acpi_match[] = {
>  	{ "LOON0011", 0 },
>  	{ }
> @@ -240,6 +254,7 @@ MODULE_DEVICE_TABLE(acpi, loongson_se_acpi_match);
>  
>  static struct platform_driver loongson_se_driver = {
>  	.probe   = loongson_se_probe,
> +	.remove  = loongson_se_remove,
>  	.driver  = {
>  		.name  = "loongson-se",
>  		.acpi_match_table = loongson_se_acpi_match,
> 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.2
> 

-- 
Lee Jones

[PATCH v4 2/2] mfd: loongson-se: Add multi-node support

[Qunqin Zhao]

On the Loongson platform, each node is equipped with a security engine
device. However, due to a hardware flaw, only the device on node 0 can
trigger interrupts. Therefore, interrupts from other nodes are forwarded
by node 0. We need to check in the interrupt handler of node 0 whether
this interrupt is intended for other nodes, this can be accomplished via
shared interrupt handling.

Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn>
---
 drivers/mfd/loongson-se.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/loongson-se.c b/drivers/mfd/loongson-se.c
index aa34e663c4..2f296faa97 100644
--- a/drivers/mfd/loongson-se.c
+++ b/drivers/mfd/loongson-se.c
@@ -142,6 +142,11 @@ static irqreturn_t se_irq_handler(int irq, void *dev_id)
 
 	int_status = readl(se->base + SE_S2LINT_STAT);
 
+	if (int_status == 0) {
+		spin_unlock(&se->dev_lock);
+		return IRQ_NONE;
+	}
+
 	/* For controller */
 	if (int_status & SE_INT_CONTROLLER) {
 		complete(&se->cmd_completion);
@@ -222,7 +227,7 @@ static int loongson_se_probe(struct platform_device *pdev)
 
 	for (i = 0; i < nr_irq; i++) {
 		irq = platform_get_irq(pdev, i);
-		err = devm_request_irq(dev, irq, se_irq_handler, 0, "loongson-se", se);
+		err = devm_request_irq(dev, irq, se_irq_handler, IRQF_SHARED, "loongson-se", se);
 		if (err) {
 			dev_err(dev, "failed to request IRQ: %d\n", irq);
 			return err;
@@ -233,7 +238,7 @@ static int loongson_se_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
-	return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, engines,
+	return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, engines,
 				    ARRAY_SIZE(engines), NULL, 0, NULL);
 }
 
-- 
2.47.2

Re: [PATCH v4 2/2] mfd: loongson-se: Add multi-node support

[Lee Jones]

Same here please.

/* Sashiko Automation: Issues Found (6 Findings) */

On Mon, 29 Jun 2026, Qunqin Zhao wrote:

> On the Loongson platform, each node is equipped with a security engine
> device. However, due to a hardware flaw, only the device on node 0 can
> trigger interrupts. Therefore, interrupts from other nodes are forwarded
> by node 0. We need to check in the interrupt handler of node 0 whether
> this interrupt is intended for other nodes, this can be accomplished via
> shared interrupt handling.
> 
> Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn>
> ---
>  drivers/mfd/loongson-se.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/loongson-se.c b/drivers/mfd/loongson-se.c
> index aa34e663c4..2f296faa97 100644
> --- a/drivers/mfd/loongson-se.c
> +++ b/drivers/mfd/loongson-se.c
> @@ -142,6 +142,11 @@ static irqreturn_t se_irq_handler(int irq, void *dev_id)
>  
>  	int_status = readl(se->base + SE_S2LINT_STAT);
>  
> +	if (int_status == 0) {
> +		spin_unlock(&se->dev_lock);
> +		return IRQ_NONE;
> +	}
> +
>  	/* For controller */
>  	if (int_status & SE_INT_CONTROLLER) {
>  		complete(&se->cmd_completion);
> @@ -222,7 +227,7 @@ static int loongson_se_probe(struct platform_device *pdev)
>  
>  	for (i = 0; i < nr_irq; i++) {
>  		irq = platform_get_irq(pdev, i);
> -		err = devm_request_irq(dev, irq, se_irq_handler, 0, "loongson-se", se);
> +		err = devm_request_irq(dev, irq, se_irq_handler, IRQF_SHARED, "loongson-se", se);

[Severity: High]
Could unmasking interrupts before registering the shared handler create a
race condition?

Because writel(SE_INT_ALL, se->base + SE_S2LINT_EN) is called earlier in
loongson_se_probe() to unmask interrupts, registering a shared interrupt
handler here means that if node 1's interrupts are unmasked, it could
instantly assert the shared IRQ line.

Since node 1 hasn't fully registered its handler yet, node 0's handler might
return IRQ_NONE, causing the kernel to repeatedly receive an unhandled
interrupt and eventually disable the IRQ line.



[Severity: High]
This is a pre-existing issue, but might the new use of IRQF_SHARED expose an
uninitialized completion object in the interrupt handler?

The completion structures for the engines are zero-allocated via
devm_kzalloc() in loongson_se_probe() and are only initialized later in
loongson_se_init_engine().

If a pending or spurious engine interrupt fires on the shared line before
the child engine devices are fully initialized, the handler will execute:

se_irq_handler()
    ...
    while (int_status) {
        id = __ffs(int_status);
        if (id < SE_ENGINE_MAX)
            complete(&se->engines[id].completion);

Calling complete() here on a zeroed structure dereferences a NULL
wait.head.next pointer, which could cause a kernel panic.


>  		if (err) {
>  			dev_err(dev, "failed to request IRQ: %d\n", irq);
>  			return err;
> @@ -233,7 +238,7 @@ static int loongson_se_probe(struct platform_device *pdev)
>  	if (err)
>  		return err;
>  
> -	return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, engines,
> +	return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, engines,
>  				    ARRAY_SIZE(engines), NULL, 0, NULL);
>  }

[Severity: Critical]
This isn't a bug introduced by this patch, but does the probe error path leak
running hardware state?

loongson_se_init() configures the hardware to use the allocated DMA memory
and sends SE_CMD_START. If devm_mfd_add_devices() fails here, the probe
returns an error and devres frees the DMA memory.

However, the hardware is never sent SE_CMD_STOP in the error path. It
remains active and retains a pointer to the freed DMA memory, which could
allow it to corrupt system memory via DMA.



[Severity: High]
This is a pre-existing issue, but are hardware interrupts left globally
enabled if probe fails or the device is removed?

loongson_se_probe() unconditionally enables all hardware interrupts earlier
via writel(SE_INT_ALL, se->base + SE_S2LINT_EN). If devm_mfd_add_devices()
fails, the probe function returns an error and the IRQ handler is
unregistered, but the hardware interrupts are never masked.

Similarly, loongson_se_remove() stops the controller but fails to write 0
to the interrupt enable register.

Because the IRQ is shared, a pending interrupt from this device could
continuously trigger the shared IRQ line without a registered handler to
clear it, causing an unhandled interrupt storm that forces the kernel to
permanently disable the IRQ line.

While reviewing the completion handling in this driver, I noticed a couple
other pre-existing issues outside the scope of this diff:



[Severity: High]
This isn't a bug introduced by this patch, but does re-initializing the
completion variable after triggering the hardware create a race condition?

Looking at loongson_se_send_engine_cmd():

loongson_se_send_engine_cmd()
    ...
    loongson_se_poll() // triggers hardware and reenables interrupts
    ...
    reinit_completion(&engine->completion);
    ...
    wait_for_completion_interruptible(&engine->completion);

If the hardware completes the command quickly after loongson_se_poll()
unmasks interrupts, it triggers an interrupt. The handler will call
complete(), but then execution resumes in loongson_se_send_engine_cmd()
where reinit_completion() clears the completion state.

This would cause the subsequent wait_for_completion_interruptible() to hang
indefinitely since the completion event was lost.



[Severity: High]
This is a pre-existing issue, but could missing completion re-initialization
cause controller commands to falsely return success?

Looking at loongson_se_send_controller_cmd():

loongson_se_send_controller_cmd()
    ...
    // hardware triggered
    return wait_for_completion_interruptible(&se->cmd_completion);

The function waits on the completion without first calling
reinit_completion() to clear any leftover state.

If a previous wait was interrupted by a signal (returning -ERESTARTSYS),
the hardware may still complete later, leaving a positive done count in
the completion structure. On the next call to
loongson_se_send_controller_cmd(), wait_for_completion_interruptible()
would immediately consume the leftover count and return success before the
new hardware command has actually finished. This could break serialization
and corrupt hardware or DMA state.

>  
> -- 
> 2.47.2
> 

-- 
Lee Jones