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

[Mirsad Goran Todorovac]

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