On Mon, Nov 08, 2021 at 11:03:57AM -0700, Alex Williamson wrote:
> A Lenovo ThinkStation S20 (4157CTO BIOS 60KT41AUS) fails to boot on
> recent kernels including the think-lmi driver, due to the fact that
> errors returned by the tlmi_analyze() function are ignored by
> tlmi_probe(), where tlmi_sysfs_init() is called unconditionally.
> This results in making use of an array of already freed, non-null
> pointers and other uninitialized globals, causing all sorts of nasty
> kobject and memory faults.
>
> Make use of the analyze function return value, free a couple leaked
> allocations, and remove the settings_count field, which is incremented
> but never consumed.
part of me would like to see this split into 2 patches, one to fix the memory
leaks, and one to fix the boot issue. But, its so small its hard to argue for
splitting up.
Looks good enough to me:
Reviewed-by: Mark Gross <[email protected]>
>
> Fixes: a40cd7ef22fb ("platform/x86: think-lmi: Add WMI interface support on Lenovo platforms")
> Signed-off-by: Alex Williamson <[email protected]>
> ---
> drivers/platform/x86/think-lmi.c | 13 ++++++++++---
> drivers/platform/x86/think-lmi.h | 1 -
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
> index 9472aae72df2..c4d9c45350f7 100644
> --- a/drivers/platform/x86/think-lmi.c
> +++ b/drivers/platform/x86/think-lmi.c
> @@ -888,8 +888,10 @@ static int tlmi_analyze(void)
> break;
> if (!item)
> break;
> - if (!*item)
> + if (!*item) {
> + kfree(item);
> continue;
> + }
>
> /* It is not allowed to have '/' for file name. Convert it into '\'. */
> strreplace(item, '/', '\\');
> @@ -902,6 +904,7 @@ static int tlmi_analyze(void)
> setting = kzalloc(sizeof(*setting), GFP_KERNEL);
> if (!setting) {
> ret = -ENOMEM;
> + kfree(item);
> goto fail_clear_attr;
> }
> setting->index = i;
> @@ -916,7 +919,6 @@ static int tlmi_analyze(void)
> }
> kobject_init(&setting->kobj, &tlmi_attr_setting_ktype);
> tlmi_priv.setting[i] = setting;
> - tlmi_priv.settings_count++;
> kfree(item);
> }
>
> @@ -983,7 +985,12 @@ static void tlmi_remove(struct wmi_device *wdev)
>
> static int tlmi_probe(struct wmi_device *wdev, const void *context)
> {
> - tlmi_analyze();
> + int ret;
> +
> + ret = tlmi_analyze();
> + if (ret)
> + return ret;
> +
> return tlmi_sysfs_init();
> }
>
> diff --git a/drivers/platform/x86/think-lmi.h b/drivers/platform/x86/think-lmi.h
> index f8e26823075f..2ce5086a5af2 100644
> --- a/drivers/platform/x86/think-lmi.h
> +++ b/drivers/platform/x86/think-lmi.h
> @@ -55,7 +55,6 @@ struct tlmi_attr_setting {
> struct think_lmi {
> struct wmi_device *wmi_device;
>
> - int settings_count;
> bool can_set_bios_settings;
> bool can_get_bios_selections;
> bool can_set_bios_password;
>
>