Re: [PATCH 2/3] power: supply: core: Read the charging temperature range and alert settings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] power: supply: core: out-of-bounds read in
  power_supply_ocv2cap_simple() [1]
--

commit adb164988f0d04b6bb830ddbcbc6a92311698583
Author: Alexey Charkov <alchark@flipper.net>

power: supply: core: Read the charging temperature range and alert settings

This commit reads the charge-temp-range-celsius and alert-*-temp-charge-*
firmware properties into struct power_supply_battery_info, falling back to
the operating temperature range when charge-temp-range-celsius is absent.

> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index de15314ef9549..3bfc4fb8687ef 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -855,6 +855,14 @@ int power_supply_get_battery_info(struct power_supply *psy,
>  			     &info->constant_charge_voltage_max_uv);
>  	fwnode_property_read_u32(fwnode, "factory-internal-resistance-micro-ohms",
>  			     &info->factory_internal_resistance_uohm);
> +	fwnode_property_read_u32(fwnode, "alert-low-temp-charge-current-microamp",
> +			     &info->alert_low_temp_charge_current_ua);
> +	fwnode_property_read_u32(fwnode, "alert-low-temp-charge-voltage-microvolt",
> +			     &info->alert_low_temp_charge_voltage_uv);
> +	fwnode_property_read_u32(fwnode, "alert-high-temp-charge-current-microamp",
> +			     &info->alert_high_temp_charge_current_ua);
> +	fwnode_property_read_u32(fwnode, "alert-high-temp-charge-voltage-microvolt",
> +			     &info->alert_high_temp_charge_voltage_uv);
>  
>  	if (!fwnode_property_read_u32_array(fwnode, "ambient-celsius",
>  					    min_max, ARRAY_SIZE(min_max))) {
> @@ -871,6 +879,14 @@ int power_supply_get_battery_info(struct power_supply *psy,
>  		info->temp_min = min_max[0];
>  		info->temp_max = min_max[1];
>  	}
> +	if (!fwnode_property_read_u32_array(fwnode, "charge-temp-range-celsius",
> +					    min_max, ARRAY_SIZE(min_max))) {
> +		info->temp_charge_min = min_max[0];
> +		info->temp_charge_max = min_max[1];
> +	} else {
> +		info->temp_charge_min = info->temp_min;
> +		info->temp_charge_max = info->temp_max;
> +	}
>  
>  	len = fwnode_property_count_u32(fwnode, "ocv-capacity-celsius");
>  	if (len < 0 && len != -EINVAL) {

[Severity: High]
This problem wasn't introduced by this patch, but can an empty
ocv-capacity-table-N property in power_supply_get_battery_info() [1]
cause an out-of-bounds read in power_supply_ocv2cap_simple()?

drivers/power/supply/power_supply_core.c:power_supply_get_battery_info() {
    ...
		proplen = fwnode_property_count_u32(fwnode, propname);
		if (proplen < 0 || proplen % 2 != 0) {
			dev_err(&psy->dev, "failed to get %s\n", propname);
			power_supply_put_battery_info(psy, info);
			err = -EINVAL;
			goto out_put_node;
		}
    ...
		tab_len = proplen / 2;
		info->ocv_table_size[index] = tab_len;

		info->ocv_table[index] = table =
			devm_kcalloc(&psy->dev, tab_len, sizeof(*table), GFP_KERNEL);
    ...
}

When ocv-capacity-table-N is present but empty,
fwnode_property_count_u32() returns 0, which passes the proplen < 0 ||
proplen % 2 != 0 check.  devm_kcalloc() then returns ZERO_SIZE_PTR with
ocv_table_size[index] set to 0, and power_supply_ocv2cap_simple() later
dereferences table[0], crashing the kernel.  Rejecting proplen <= 0 (similar
to resistance-temp-table below) would avoid this.

[1] https://sashiko.dev/#/bug/linux-4a0a5d5f-629d-4b0d-b4be-07415bcb018e

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-battery-charge-temp-range-v1-0-a967abd8b80c@flipper.net?part=2