Bug 219939

Summary:	Missing null check in cros_ec_hwmon_probe_temp_sensors
Product:	Drivers	Reporter:	henry (bsdhenrymartin)
Component:	Hardware Monitoring	Assignee:	Jean Delvare (jdelvare)
Status:	NEW ---
Severity:	normal	CC:	linux
Priority:	P3
Hardware:	All
OS:	Linux
Kernel Version:		Subsystem:
Regression:	No	Bisected commit-id:

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!