[PATCH v2] rtc: pcf85363: Add error checking to regmap calls in probe()

[PATCH v2] rtc: pcf85363: Add error checking to regmap calls in probe()

[Cosmo Chou]

The probe() function ignores errors returned by regmap operations.
If an I2C transport error occurs (e.g., -ENXIO), the driver continues
probing and may register a non-functional RTC device.

Propagate errors from all unchecked regmap calls in probe() using
dev_err_probe().

Fixes: fd9a6a13949a ("rtc: pcf85363: add support for the quartz-load-femtofarads property")
Signed-off-by: Cosmo Chou <chou.cosmo@gmail.com>
---
Changes in v2 [1]:
- Update commit message to reflect the new scope of the fix
- Add error checking to all remaining regmap calls in probe()
  (regmap_write to CTRL_FLAGS and regmap_update_bits to CTRL_PIN_IO)

[1] https://lore.kernel.org/linux-rtc/20260716125142.1801599-1-chou.cosmo@gmail.com/

 drivers/rtc/rtc-pcf85363.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index 540042b9eec8..ccc7834e5759 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -426,8 +426,8 @@ static int pcf85363_probe(struct i2c_client *client)
 
 	err = pcf85363_load_capacitance(pcf85363, client->dev.of_node);
 	if (err < 0)
-		dev_warn(&client->dev, "failed to set xtal load capacitance: %d",
-			 err);
+		return dev_err_probe(&client->dev, err,
+				     "failed to set xtal load capacitance\n");
 
 	pcf85363->rtc->ops = &rtc_ops;
 	pcf85363->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
@@ -436,9 +436,16 @@ static int pcf85363_probe(struct i2c_client *client)
 	wakeup_source = device_property_read_bool(&client->dev,
 						  "wakeup-source");
 	if (client->irq > 0 || wakeup_source) {
-		regmap_write(pcf85363->regmap, CTRL_FLAGS, 0);
-		regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
-				   PIN_IO_INTAPM, PIN_IO_INTA_OUT);
+		err = regmap_write(pcf85363->regmap, CTRL_FLAGS, 0);
+		if (err)
+			return dev_err_probe(&client->dev, err,
+					     "failed to clear flags\n");
+
+		err = regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
+					 PIN_IO_INTAPM, PIN_IO_INTA_OUT);
+		if (err)
+			return dev_err_probe(&client->dev, err,
+					     "failed to set interrupt pin mode\n");
 	}
 
 	if (client->irq > 0) {
-- 
2.43.0

Re: [PATCH v2] rtc: pcf85363: Add error checking to regmap calls in probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Concurrent probes modify a shared static nvmem_config array without synchronization, which can lead to memory corruption or use-after-free vulnerabilities.
--

--- Patch [1]: [PATCH v2] rtc: pcf85363: Add error checking to regmap calls in probe() ---
commit 5bd0f6fb9671f5eeb0aea5f9e2154781ed668ff5
Author: Cosmo Chou <chou.cosmo@gmail.com>

rtc: pcf85363: Add error checking to regmap calls in probe()

This commit propagates errors from unchecked regmap operations in the
probe() function using dev_err_probe() to prevent registering a
non-functional RTC device on I2C transport errors.

Link: https://lore.kernel.org/linux-rtc/20260716125142.1801599-1-chou.cosmo@gmail.com/

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index 540042b9eec8f..ccc7834e57599 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -426,8 +426,8 @@ static int pcf85363_probe(struct i2c_client *client)
 
 	err = pcf85363_load_capacitance(pcf85363, client->dev.of_node);
 	if (err < 0)
-		dev_warn(&client->dev, "failed to set xtal load capacitance: %d",
-			 err);
+		return dev_err_probe(&client->dev, err,
+				     "failed to set xtal load capacitance\n");

[Severity: High]
This is a pre-existing issue, but since you are modifying the probe function,
could we look at a potential data race with the nvmem configuration?

In drivers/rtc/rtc-pcf85363.c:pcf85363_probe(), the nvmem_cfg array is
declared as static:

static int pcf85363_probe(struct i2c_client *client)
{
        ...
        static struct nvmem_config nvmem_cfg[] = {
                {
                        .name = "pcf85x63-",
                        ...
                },
        };

Later in the function, this globally shared array is mutated to set a 
per-instance priv pointer:

        for (i = 0; i < config->num_nvram; i++) {
                nvmem_cfg[i].priv = pcf85363;
                devm_rtc_nvmem_register(pcf85363->rtc, &nvmem_cfg[i]);
        }

If multiple instances of this device probe concurrently, can one instance
overwrite the priv and dev pointers of another instance? This might result
in one NVMEM device silently using the regmap and parent device of the
other instance, potentially leading to use-after-free and memory corruption
if the other device is unbound.

>  
>  	pcf85363->rtc->ops = &rtc_ops;
>  	pcf85363->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717193705.2003175-1-chou.cosmo@gmail.com?part=1