Bug 219939 - Missing null check in cros_ec_hwmon_probe_temp_sensors
Summary: Missing null check in cros_ec_hwmon_probe_temp_sensors
Status: NEW
Alias: None
Product: Drivers
Classification: Unclassified
Component: Hardware Monitoring (show other bugs)
Hardware: All Linux
: P3 normal
Assignee: Jean Delvare
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-03-28 08:45 UTC by henry
Modified: 2025-03-31 14:23 UTC (History)
1 user (show)

See Also:
Kernel Version:
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description henry 2025-03-28 08:45:05 UTC
hwmon: cros_ec_hwmon: Add NULL check in cros_ec_hwmon_probe_temp_sensors

devm_kasprintf() can return a NULL pointer on failure, but "priv->temp_sensor_names[i]" returned value in cros_ec_hwmon_probe_temp_sensors is not checked.

Add NULL check in cros_ec_hwmon_probe_temp_sensors, to handle kernel NULL pointer dereference error.
Comment 1 Jean Delvare 2025-03-31 14:16:31 UTC
There's no actual NULL pointer dereference. If the memory allocation failed, cros_ec_hwmon_is_visible() will set the attributes permissions to 0, which means the attribute will not be created. This means cros_ec_hwmon_read_string(), the only function which makes use of this pointer, will never be called for this attribute.

We could report the memory shortage, but I seem to recall the low-level memory allocation functions are already doing that so it would be redundant. We could also catch the error and fail the device probing altogether, but I can't see how this would improve the situation.

So I believe the best course of action is to leave the code as is.
Comment 2 henry 2025-03-31 14:23:31 UTC
(In reply to Jean Delvare from comment #1)
> There's no actual NULL pointer dereference. If the memory allocation failed,
> cros_ec_hwmon_is_visible() will set the attributes permissions to 0, which
> means the attribute will not be created. This means
> cros_ec_hwmon_read_string(), the only function which makes use of this
> pointer, will never be called for this attribute.
> 
> We could report the memory shortage, but I seem to recall the low-level
> memory allocation functions are already doing that so it would be redundant.
> We could also catch the error and fail the device probing altogether, but I
> can't see how this would improve the situation.
> 
> So I believe the best course of action is to leave the code as is.

Thank you for the detailed explanation. I'll drop this patch and mark it as "Not Needed" in my records.

Appreciate your time reviewing this!

Note You need to log in before you can comment on or make changes to this bug.