Re: [BUG] [BISECTED] [CORRECTION] systemd-devd triggers kernel memleak apparently in drivers/core/dd.c: driver_register()

From: Mirsad Goran Todorovac
Date: Wed Mar 29 2023 - 12:27:50 EST


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