[PATCH 0/2] hwmon: chipcap2: fix uninitialized symbols

[PATCH 0/2] hwmon: chipcap2: fix uninitialized symbols

[Javier Carrasco]

Two static checker warnings have been found with Smatch[1] when checking
the chipcap2 driver. Two variables might be used uninitialized under
certain circumstances (explained in the commit messages).

This series fixes the two warnings and optimizes the error paths involved.

The fixes have been tested with Smatch (including cross function database),
and the bugs could not be reproduced anymore.

[1] https://lore.kernel.org/linux-hwmon/294e4634-89d4-415e-a723-b208d8770d7c@gmail.com/T/#t

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
Javier Carrasco (2):
      hwmon: chipcap2: fix uninitialized variable in cc2_get_reg_val()
      hwmon: chipcap2: fix return path in cc2_request_alarm_irqs()

 drivers/hwmon/chipcap2.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)
---
base-commit: 65f976a4299c5de2d2c9162c0337f95b7447243d
change-id: 20240207-chipcap2_init_vars-b39f8ca470be

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>

[PATCH 1/2] hwmon: chipcap2: fix uninitialized variable in cc2_get_reg_val()

[Javier Carrasco]

The reg_val variable in cc2_get_reg_val() might be used without a known
value if cc2_read_reg() fails. That leads to a useless data conversion
because the returned error means the read operation failed and the data is
not relevant.

That makes its initial value irrelevant as well, so skip the data
conversion instead. If no error happens, a value is assigned to reg_val
and the data conversion is required.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/linux-hwmon/294e4634-89d4-415e-a723-b208d8770d7c@gmail.com/T/#t
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/hwmon/chipcap2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/chipcap2.c b/drivers/hwmon/chipcap2.c
index a62c507b1042..3b604fc5d8ae 100644
--- a/drivers/hwmon/chipcap2.c
+++ b/drivers/hwmon/chipcap2.c
@@ -324,7 +324,9 @@ static int cc2_get_reg_val(struct cc2_data *data, u8 reg, long *val)
 	int ret;
 
 	ret = cc2_read_reg(data, reg, &reg_val);
-	*val = cc2_rh_convert(reg_val);
+	if (!ret)
+		*val = cc2_rh_convert(reg_val);
+
 	cc2_disable(data);
 
 	return ret;

-- 
2.40.1

[PATCH 2/2] hwmon: chipcap2: fix return path in cc2_request_alarm_irqs()

[Javier Carrasco]

The return path can be improved by returning upon first failure. The
current implementation would try to register the second interrupt even
if the first one failed, which is unnecessary.

Moreover, if no irqs are available, the return value should be zero
(the driver supports the use case with no interrupts). Currently the
initial value is unassigned and that may lead to returning an unknown
value if stack variables are not automatically set to zero and no irqs
were provided.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/linux-hwmon/294e4634-89d4-415e-a723-b208d8770d7c@gmail.com/T/#t
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/hwmon/chipcap2.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/chipcap2.c b/drivers/hwmon/chipcap2.c
index 3b604fc5d8ae..6ccceae21f70 100644
--- a/drivers/hwmon/chipcap2.c
+++ b/drivers/hwmon/chipcap2.c
@@ -670,7 +670,7 @@ static int cc2_request_ready_irq(struct cc2_data *data, struct device *dev)
 
 static int cc2_request_alarm_irqs(struct cc2_data *data, struct device *dev)
 {
-	int ret;
+	int ret = 0;
 
 	data->irq_low = fwnode_irq_get_byname(dev_fwnode(dev), "low");
 	if (data->irq_low > 0) {
@@ -679,8 +679,10 @@ static int cc2_request_alarm_irqs(struct cc2_data *data, struct device *dev)
 						IRQF_ONESHOT |
 						IRQF_TRIGGER_RISING,
 						dev_name(dev), data);
-		if (!ret)
-			data->rh_alarm.low_alarm_visible = true;
+		if (ret)
+			return ret;
+
+		data->rh_alarm.low_alarm_visible = true;
 	}
 
 	data->irq_high = fwnode_irq_get_byname(dev_fwnode(dev), "high");
@@ -690,8 +692,10 @@ static int cc2_request_alarm_irqs(struct cc2_data *data, struct device *dev)
 						IRQF_ONESHOT |
 						IRQF_TRIGGER_RISING,
 						dev_name(dev), data);
-		if (!ret)
-			data->rh_alarm.high_alarm_visible = true;
+		if (ret)
+			return ret;
+
+		data->rh_alarm.high_alarm_visible = true;
 	}
 
 	return ret;

-- 
2.40.1

Re: [PATCH 1/2] hwmon: chipcap2: fix uninitialized variable in cc2_get_reg_val()

[Guenter Roeck]

On Wed, Feb 07, 2024 at 10:17:08PM +0100, Javier Carrasco wrote:
> The reg_val variable in cc2_get_reg_val() might be used without a known
> value if cc2_read_reg() fails. That leads to a useless data conversion
> because the returned error means the read operation failed and the data is
> not relevant.
> 
> That makes its initial value irrelevant as well, so skip the data
> conversion instead. If no error happens, a value is assigned to reg_val
> and the data conversion is required.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/linux-hwmon/294e4634-89d4-415e-a723-b208d8770d7c@gmail.com/T/#t
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

Applied.

Thanks,
Guenter

Re: [PATCH 2/2] hwmon: chipcap2: fix return path in cc2_request_alarm_irqs()

[Guenter Roeck]

On Wed, Feb 07, 2024 at 10:17:09PM +0100, Javier Carrasco wrote:
> The return path can be improved by returning upon first failure. The
> current implementation would try to register the second interrupt even
> if the first one failed, which is unnecessary.
> 
> Moreover, if no irqs are available, the return value should be zero
> (the driver supports the use case with no interrupts). Currently the
> initial value is unassigned and that may lead to returning an unknown
> value if stack variables are not automatically set to zero and no irqs
> were provided.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/linux-hwmon/294e4634-89d4-415e-a723-b208d8770d7c@gmail.com/T/#t
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

Applied.

Thanks,
Guenter