Re: [PATCH v1 1/2] Revert "thermal: hwmon: Use extra_groups for adding temperature attributes"

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Adding sysfs attributes via `device_create_file()` after device registration creates a race condition with userspace uevents.
--

--- Patch [1]: [PATCH v1 1/2] Revert "thermal: hwmon: Use extra_groups for adding temperature attributes" ---
commit fd1537461eb002e63315e0164251174bf300f7cc
Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Revert "thermal: hwmon: Use extra_groups for adding temperature attributes"

This reverts commit cfb5dc0f60fb because it was depended on by another commit
that turned out to be problematic.

> diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
> index 386dfb9f559e3..223ae1571655b 100644
> --- a/drivers/thermal/thermal_hwmon.c
> +++ b/drivers/thermal/thermal_hwmon.c
[ ... ]
> @@ -120,13 +114,35 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
>  	scnprintf(hwmon->name, THERMAL_HWMON_NAME_LENGTH, "%s_%d", tz->type, tz->id);
>  	strreplace(hwmon->name, '-', '_');
>  	hwmon->device = hwmon_device_register_for_thermal(&tz->device,
> -							  hwmon->name, hwmon,
> -							  thermal_hwmon_groups);
> +							  hwmon->name, hwmon);
>  	if (IS_ERR(hwmon->device)) {
> -		int result = PTR_ERR(hwmon->device);
> +		result = PTR_ERR(hwmon->device);
> +		goto free_mem;
> +	}
>  
> -		kfree(hwmon);
> -		return result;
> +	temp = &hwmon->tz_temp;
> +
> +	temp->tz = tz;
> +
> +	temp->temp_input.attr.attr.name = "temp1_input";
> +	temp->temp_input.attr.attr.mode = 0444;
> +	temp->temp_input.attr.show = temp_input_show;
> +	sysfs_attr_init(&temp->temp_input.attr.attr);
> +	result = device_create_file(hwmon->device, &temp->temp_input.attr);

[Severity: Medium]
Does this sequence create a race condition with userspace uevents? 

When thermal_add_hwmon_sysfs() calls hwmon_device_register_for_thermal(),
the device is registered and the KOBJ_ADD uevent fires immediately.

If userspace monitoring daemons or udev rules respond to this uevent and 
read the hwmon device directory right away, they might query it before 
device_create_file() finishes adding the temp1_input and temp1_crit 
attributes.

Could this lead to missing sensor attributes in userspace during device
initialization?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/6319276.lOV4Wx5bFT@rafael.j.wysocki?part=1