Message-ID: <20251029173456.1749436-1-a.shimko.dev@gmail.com>
Date: Wed, 29 Oct 2025 20:34:55 +0300
From: Artem Shimko <a.shimko.dev@...il.com>
To: Sudeep Holla <sudeep.holla@....com>,
	Cristian Marussi <cristian.marussi@....com>
Cc: a.shimko.dev@...il.com,
	arm-scmi@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] scmi: reset: validate number of reset domains

Add validation to reject zero reset domains during protocol initialization.
While the SCMI specification allows zero domains, the current driver
implementation cannot handle this case safely.

The fix adds an explicit check for zero domains in
scmi_reset_protocol_init(), returning -EINVAL early during protocol
initialization. This prevents the driver from proceeding with a
non-functional state and avoids potential kernel panics in functions
like scmi_reset_domain_reset() and scmi_reset_notify_supported() that
assume dom_info is always valid.

The change is minimal and safe, affecting only the error case while
preserving all existing functionality for valid configurations. The
existing -ENOMEM handling for memory allocation remains unchanged and
sufficient.

Signed-off-by: Artem Shimko <a.shimko.dev@...il.com>
---

Dear SCMI Maintainers,

This patch addresses an issue in the SCMI reset protocol initialization
where a zero value for num_domains could lead to a non-functional state
or potential NULL pointer dereferences.

Currently, if the platform reports zero reset domains, the driver
continues initialization but creates an inconsistent state:

    ret = scmi_reset_attributes_get(ph, pinfo);
    if (ret)
        return ret;

    /* When num_domains == 0: */
    pinfo->dom_info = devm_kcalloc(ph->dev, pinfo->num_domains,  /* 0 */
          sizeof(*pinfo->dom_info), GFP_KERNEL);
    /* Returns ZERO_SIZE_PTR (not NULL) */
    
    if (!pinfo->dom_info)  /* ZERO_SIZE_PTR != NULL, condition fails */
        return -ENOMEM;

    /* Execution continues! */
    return ph->set_priv(ph, pinfo, version);  /* Returns SUCCESS (0)! */

However, subsequent reset operations crash when accessing dom_info:

    static int scmi_reset_domain_reset(const struct scmi_protocol_handle *ph,
              u32 domain_id)
    {
        struct scmi_reset_info *pi = ph->get_priv(ph);
        struct reset_dom_info *dom = pi->dom_info + domain_id;  
        /* ZERO_SIZE_PTR + domain_id = INVALID POINTER! */
        /* KERNEL PANIC on dom-> access */
    }

The protocol appears to initialize successfully but is actually non-functional
and will crash on first usage.

The patch adds validation to reject zero domains during initialization,
ensuring fail-fast behavior and preventing hidden failures. This approach
maintains system stability by catching invalid configurations early.

Testing confirmed normal operation with positive num_domains values and
proper error handling with zero domains. The change is minimal and safe,
affecting only the error case while preserving all existing functionality
for valid configurations.

This patch fixes a potential crash scenario while maintaining full
backward compatibility with properly configured systems.

Case with num_domains == 0 before the fix:
[   25.617950] SCMI Notifications RESET - drivers/firmware/arm_scmi/reset.c scmi_reset_protocol_init
				num_domains == 0
				pinfo->dom_info == 0000000000000010
[   25.623655] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
[   25.625241] Oops [#1]
[   25.625585] Modules linked in:
[   25.626343] CPU: 1 UID: 0 PID: 62 Comm: kworker/u39:0 Not tainted 6.12.0-01121-g62b3fcf44d59-dirty #1
[   25.627257] Hardware name: YMP ELCT QEMU (DT) 
[   25.628276] Workqueue: events_unbound deferred_probe_work_func
[   25.629447] epc : scmi_reset_protocol_init+0x256/0x2de
[   25.630109]  ra : scmi_reset_protocol_init+0x248/0x2de
[   25.630616] epc : ffffffff8080d618 ra : ffffffff8080d60a sp : ffffffc600263350
[   25.631232]  gp : ffffffff81a4d9d8 tp : ffffffd60e834380 t0 : 0000000000000008
[   25.631897]  t1 : ffffffffffff0000 t2 : 69746f4e20494d43 s0 : ffffffc6002633f0
[   25.632570]  s1 : 0000000000000000 a0 : 0000000000000000 a1 : 0000000000000000
[   25.633364]  a2 : ffffffd60ece0740 a3 : 0000000000000000 a4 : 0000000000000001
[   25.633950]  a5 : ffffffd60f75c640 a6 : ffffffd60e834480 a7 : 0000000000000001
[   25.634496]  s2 : ffffffd60eee3440 s3 : ffffffd60e812a30 s4 : 0000000000000010
[   25.635037]  s5 : 0000000000030000 s6 : ffffffff80000000 s7 : 0000000000000000
[   25.635581]  s8 : 0000000020000000 s9 : 0000000000000002 s10: ffffffffffffffff
[   25.636127]  s11: 0000000000000048 t3 : 00000000000000ff t4 : 0000000000000000
[   25.636660]  t5 : 0000000000000001 t6 : 0000000000000008
[   25.637071] status: 0000000200000120 badaddr: 0000000000000010 cause: 000000000000000f
[   25.637836] [<ffffffff8080d618>] scmi_reset_protocol_init+0x256/0x2de
[   25.638472] [<ffffffff80805670>] scmi_get_protocol_instance+0x186/0x44e
[   25.638984] [<ffffffff8080597c>] scmi_devres_protocol_instance_get+0x44/0x88
[   25.639501] [<ffffffff808059d2>] scmi_devm_protocol_get+0x12/0x30
[   25.639917] [<ffffffff804e3978>] scmi_reset_probe+0x3c/0xce
[   25.640278] [<ffffffff808010cc>] scmi_dev_probe+0x18/0x26
[   25.640566] [<ffffffff8058541c>] really_probe+0x8a/0x30a
[   25.640830] [<ffffffff80585700>] __driver_probe_device+0x64/0x10a
[   25.641147] [<ffffffff80585864>] driver_probe_device+0x2c/0xb2
[   25.641450] [<ffffffff8058595e>] __device_attach_driver+0x74/0xd2

If this is a working case, I will check and supplement other protocols such as
sensor and power domain.

 drivers/firmware/arm_scmi/reset.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/firmware/arm_scmi/reset.c b/drivers/firmware/arm_scmi/reset.c
index 0aa82b96f41b..458b75fcc858 100644
--- a/drivers/firmware/arm_scmi/reset.c
+++ b/drivers/firmware/arm_scmi/reset.c
@@ -358,6 +358,9 @@ static int scmi_reset_protocol_init(const struct scmi_protocol_handle *ph)
 	if (ret)
 		return ret;
 
+	if (!pinfo->num_domains)
+		return -EINVAL;
+
 	pinfo->dom_info = devm_kcalloc(ph->dev, pinfo->num_domains,
 				       sizeof(*pinfo->dom_info), GFP_KERNEL);
 	if (!pinfo->dom_info)
-- 
2.43.0

Powered by blists - more mailing lists