| Message ID | 20230213182215.53703-1-W_Armin@gmx.de (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [1/2] platform/x86: dell-ddv: Fix cache invalidation on resume | expand |
Hi Armin, On 2/13/23 19:22, Armin Wolf wrote: > If one or both sensor buffers could not be initialized, either > due to missing hardware support or due to some error during probing, > the resume handler will encounter undefined behaviour when > attempting to lock buffers then protected by an uninitialized or > destroyed mutex. > Fix this by introducing a "active" flag which is set during probe, > and only invalidate buffers which where flaged as "active". > > Tested on a Dell Inspiron 3505. > > Fixes: 3b7eeff93d29 ("platform/x86: dell-ddv: Add hwmon support") > Signed-off-by: Armin Wolf <W_Armin@gmx.de> Can you move the "if (...active)" check to inside dell_wmi_ddv_hwmon_cache_invalidate() please ? Otherwise this patch as well as patch 2/2 look good to me. Regards, Hans > --- > drivers/platform/x86/dell/dell-wmi-ddv.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c > index d547c9d09725..58f996b3b374 100644 > --- a/drivers/platform/x86/dell/dell-wmi-ddv.c > +++ b/drivers/platform/x86/dell/dell-wmi-ddv.c > @@ -96,6 +96,7 @@ struct combined_chip_info { > }; > > struct dell_wmi_ddv_sensors { > + bool active; > struct mutex lock; /* protect caching */ > unsigned long timestamp; > union acpi_object *obj; > @@ -530,6 +531,7 @@ static void dell_wmi_ddv_hwmon_cache_destroy(void *data) > { > struct dell_wmi_ddv_sensors *sensors = data; > > + sensors->active = false; > mutex_destroy(&sensors->lock); > kfree(sensors->obj); > } > @@ -549,6 +551,7 @@ static struct hwmon_channel_info *dell_wmi_ddv_channel_init(struct wmi_device *w > return ERR_PTR(ret); > > mutex_init(&sensors->lock); > + sensors->active = true; > > ret = devm_add_action_or_reset(&wdev->dev, dell_wmi_ddv_hwmon_cache_destroy, sensors); > if (ret < 0) > @@ -852,9 +855,12 @@ static int dell_wmi_ddv_resume(struct device *dev) > { > struct dell_wmi_ddv_data *data = dev_get_drvdata(dev); > > - /* Force re-reading of all sensors */ > - dell_wmi_ddv_hwmon_cache_invalidate(&data->fans); > - dell_wmi_ddv_hwmon_cache_invalidate(&data->temps); > + /* Force re-reading of all active sensors */ > + if (data->fans.active) > + dell_wmi_ddv_hwmon_cache_invalidate(&data->fans); > + > + if (data->temps.active) > + dell_wmi_ddv_hwmon_cache_invalidate(&data->temps); > > return 0; > } > -- > 2.30.2 >
diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c index d547c9d09725..58f996b3b374 100644 --- a/drivers/platform/x86/dell/dell-wmi-ddv.c +++ b/drivers/platform/x86/dell/dell-wmi-ddv.c @@ -96,6 +96,7 @@ struct combined_chip_info { }; struct dell_wmi_ddv_sensors { + bool active; struct mutex lock; /* protect caching */ unsigned long timestamp; union acpi_object *obj; @@ -530,6 +531,7 @@ static void dell_wmi_ddv_hwmon_cache_destroy(void *data) { struct dell_wmi_ddv_sensors *sensors = data; + sensors->active = false; mutex_destroy(&sensors->lock); kfree(sensors->obj); } @@ -549,6 +551,7 @@ static struct hwmon_channel_info *dell_wmi_ddv_channel_init(struct wmi_device *w return ERR_PTR(ret); mutex_init(&sensors->lock); + sensors->active = true; ret = devm_add_action_or_reset(&wdev->dev, dell_wmi_ddv_hwmon_cache_destroy, sensors); if (ret < 0) @@ -852,9 +855,12 @@ static int dell_wmi_ddv_resume(struct device *dev) { struct dell_wmi_ddv_data *data = dev_get_drvdata(dev); - /* Force re-reading of all sensors */ - dell_wmi_ddv_hwmon_cache_invalidate(&data->fans); - dell_wmi_ddv_hwmon_cache_invalidate(&data->temps); + /* Force re-reading of all active sensors */ + if (data->fans.active) + dell_wmi_ddv_hwmon_cache_invalidate(&data->fans); + + if (data->temps.active) + dell_wmi_ddv_hwmon_cache_invalidate(&data->temps); return 0; }
If one or both sensor buffers could not be initialized, either due to missing hardware support or due to some error during probing, the resume handler will encounter undefined behaviour when attempting to lock buffers then protected by an uninitialized or destroyed mutex. Fix this by introducing a "active" flag which is set during probe, and only invalidate buffers which where flaged as "active". Tested on a Dell Inspiron 3505. Fixes: 3b7eeff93d29 ("platform/x86: dell-ddv: Add hwmon support") Signed-off-by: Armin Wolf <W_Armin@gmx.de> --- drivers/platform/x86/dell/dell-wmi-ddv.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) -- 2.30.2