On 29.3.2023. 17:46, Hans de Goede wrote:
So, the "tlmi_setting" memory leak appears to be fixed by this diff. The next step is to add Armin-suggested patch: diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c index c816646eb661..1e77ecb0cba8 100644 --- a/drivers/platform/x86/think-lmi.c +++ b/drivers/platform/x86/think-lmi.c @@ -929,8 +929,10 @@ static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *a /* validate and split from `item,value` -> `value` */ value = strpbrk(item, ","); - if (!value || value == item || !strlen(value + 1)) + if (!value || value == item || !strlen(value + 1)) { + kfree(item); return -EINVAL; + } ret = sysfs_emit(buf, "%s\n", value + 1); kfree(item); and Thomas' correction for the return type of the tlmi_setting() function: diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c index 86b33b74519be..c924e9e4a6a5b 100644 --- a/drivers/platform/x86/think-lmi.c +++ b/drivers/platform/x86/think-lmi.c @@ -1353,7 +1353,6 @@ static struct tlmi_pwd_setting *tlmi_create_auth(const char *pwd_type, static int tlmi_analyze(void) { - acpi_status status; int i, ret; if (wmi_has_guid(LENOVO_SET_BIOS_SETTINGS_GUID) && @@ -1390,8 +1389,8 @@ static int tlmi_analyze(void) char *p; tlmi_priv.setting[i] = NULL; - status = tlmi_setting(i, &item, LENOVO_BIOS_SETTING_GUID); - if (ACPI_FAILURE(status)) + ret = tlmi_setting(i, &item, LENOVO_BIOS_SETTING_GUID); + if (ret) break; if (!item) break; A build on top of 6.3-rc4+ fcd476ea6a88 commit is on the way, with all three included.Good work on catching these issues, thank you all for your work on this.
Not at all. It was a very interesting problem and a great session of brainstorming with Greg, Armin and Thomas.
I assume that these fixes will be posted as a proper 3 patch patch-series (one patch per fix) once you are done testing?
This is for others to decide. Armin gave a great hint with the one patch, and I sort of had a race condition of the one with Thomas that was the final fix ;-) The tlmi_setting return value fix by Thomas is already committed to the for-next tree, I got it from there. I do not have any authorship pretensions, this was a collaboration, so I don't know what is due in the Code of Conduct for such circumstances. I trust you guys will do the right thing. The 6.3.0-rc4-00034-gfcd476ea6a88 kernel apparently has a stackdump: WARNING: CPU: 4 PID: 746 at drivers/thermal/thermal_sysfs.c:879 cooling_device_stats_setup+0xb4/0xc0 but with our without the patch, and it apparently isn't related. Best regards, Mirsad -- Mirsad Todorovac System engineer Faculty of Graphic Arts | Academy of Fine Arts University of Zagreb Republic of Croatia, the European Union Sistem inženjer Grafički fakultet | Akademija likovnih umjetnosti Sveučilište u Zagrebu
