| Message ID | 20210628095230.319726-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86: think-lmi: Move kfree(setting->possible_values) to tlmi_attr_setting_release() | expand |
Hi, On 6/28/21 11:52 AM, Hans de Goede wrote: > We must not free the possible_values string before we have called > sysfs_remove_group(kobj, &tlmi_attr_group) otherwise there is a race > where a sysfs read of possible_values could reference the free-ed > memory. > > Move the kfree(setting->possible_values) together with the free of the > actual tlmi_attr_setting struct to avoid this race. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> I've merged this into my review-hans branch. Regards, Hans > --- > drivers/platform/x86/think-lmi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c > index 4cab341a3538..3671b5d20613 100644 > --- a/drivers/platform/x86/think-lmi.c > +++ b/drivers/platform/x86/think-lmi.c > @@ -626,6 +626,7 @@ static void tlmi_attr_setting_release(struct kobject *kobj) > { > struct tlmi_attr_setting *setting = to_tlmi_attr_setting(kobj); > > + kfree(setting->possible_values); > kfree(setting); > } > > @@ -654,7 +655,6 @@ static void tlmi_release_attr(void) > /* Attribute structures */ > for (i = 0; i < TLMI_SETTINGS_COUNT; i++) { > if (tlmi_priv.setting[i]) { > - kfree(tlmi_priv.setting[i]->possible_values); > sysfs_remove_group(&tlmi_priv.setting[i]->kobj, &tlmi_attr_group); > kobject_put(&tlmi_priv.setting[i]->kobj); > } >
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c index 4cab341a3538..3671b5d20613 100644 --- a/drivers/platform/x86/think-lmi.c +++ b/drivers/platform/x86/think-lmi.c @@ -626,6 +626,7 @@ static void tlmi_attr_setting_release(struct kobject *kobj) { struct tlmi_attr_setting *setting = to_tlmi_attr_setting(kobj); + kfree(setting->possible_values); kfree(setting); } @@ -654,7 +655,6 @@ static void tlmi_release_attr(void) /* Attribute structures */ for (i = 0; i < TLMI_SETTINGS_COUNT; i++) { if (tlmi_priv.setting[i]) { - kfree(tlmi_priv.setting[i]->possible_values); sysfs_remove_group(&tlmi_priv.setting[i]->kobj, &tlmi_attr_group); kobject_put(&tlmi_priv.setting[i]->kobj); }
We must not free the possible_values string before we have called sysfs_remove_group(kobj, &tlmi_attr_group) otherwise there is a race where a sysfs read of possible_values could reference the free-ed memory. Move the kfree(setting->possible_values) together with the free of the actual tlmi_attr_setting struct to avoid this race. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/platform/x86/think-lmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)