Messages in this thread | | | Date | Mon, 25 Sep 2023 16:26:00 +0200 | Subject | Re: [External] [PATCH] platform/x86: think-lmi: Fix reference leak | From | Armin Wolf <> |
| |
Am 25.09.23 um 16:00 schrieb Mark Pearson:
> Thanks Armin, > > On 9/23/23 16:41, Armin Wolf wrote: >> If a duplicate attribute is found using kset_find_obj(), a reference >> to that attribute is returned which needs to be disposed accordingly >> using kobject_put(). Move the setting name validation into a separate >> function to allow for this change without having to duplicate the >> cleanup code for this setting. >> As a side note, a very similar bug was fixed in >> commit 7295a996fdab ("platform/x86: dell-sysman: Fix reference leak"), >> so it seems that the bug was copied from that driver. >> >> Compile-tested only. >> >> Fixes: 1bcad8e510b2 ("platform/x86: think-lmi: Fix issues with >> duplicate attributes") >> Signed-off-by: Armin Wolf <W_Armin@gmx.de> >> --- >> drivers/platform/x86/think-lmi.c | 24 ++++++++++++++++++++---- >> 1 file changed, 20 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/platform/x86/think-lmi.c >> b/drivers/platform/x86/think-lmi.c >> index 4be6f28d4600..3a396b763c49 100644 >> --- a/drivers/platform/x86/think-lmi.c >> +++ b/drivers/platform/x86/think-lmi.c >> @@ -1344,6 +1344,24 @@ static void tlmi_release_attr(void) >> kset_unregister(tlmi_priv.authentication_kset); >> } >> >> +static int tlmi_validate_setting_name(struct kset *attribute_kset, >> char *name) >> +{ >> + struct kobject *duplicate; >> + >> + if (!strcmp(name, "Reserved")) >> + return -EINVAL; >> + >> + duplicate = kset_find_obj(attribute_kset, name); >> + if (duplicate) { >> + pr_debug("Duplicate attribute name found - %s\n", name); >> + /* kset_find_obj() returns a reference */ >> + kobject_put(duplicate); >> + return -EBUSY; >> + } >> + >> + return 0; >> +} >> + >> static int tlmi_sysfs_init(void) >> { >> int i, ret; >> @@ -1372,10 +1390,8 @@ static int tlmi_sysfs_init(void) >> continue; >> >> /* check for duplicate or reserved values */ >> - if (kset_find_obj(tlmi_priv.attribute_kset, >> tlmi_priv.setting[i]->display_name) || >> - !strcmp(tlmi_priv.setting[i]->display_name, "Reserved")) { >> - pr_debug("duplicate or reserved attribute name found - >> %s\n", >> - tlmi_priv.setting[i]->display_name); >> + if (tlmi_validate_setting_name(tlmi_priv.attribute_kset, >> + tlmi_priv.setting[i]->display_name) < 0) { >> kfree(tlmi_priv.setting[i]->possible_values); >> kfree(tlmi_priv.setting[i]); >> tlmi_priv.setting[i] = NULL; >> -- >> 2.39.2 >> > > Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca> > > Do you have any reports of our platforms where these are seen? If so > I'd like to get it fixed in FW too (and I can get it tested on HW if > that helps) > > Mark > No, i do not have any reports regarding this issue. I stumbled upon this bug by chance, after fixing a similar bug in dell-wmi-sysman. I suspect that the bug was copied from there to the other drivers, like think-lmi and hp-bioscfg.
Armin Wolf
| |