* [PATCH v2 1/2] platform/x86: think-lmi: Fix memory leaks when parsing ThinkStation WMI strings
@ 2023-04-03 1:31 Mark Pearson
2023-04-03 1:31 ` [PATCH v2 2/2] platform/x86: think-lmi: Clean up display of current_value on Thinkstation Mark Pearson
2023-04-03 9:43 ` [PATCH v2 1/2] platform/x86: think-lmi: Fix memory leaks when parsing ThinkStation WMI strings Hans de Goede
0 siblings, 2 replies; 5+ messages in thread
From: Mark Pearson @ 2023-04-03 1:31 UTC (permalink / raw)
To: mpearson-lenovo
Cc: hdegoede, markgross, W_Armin, mirsad.todorovac, linux,
Mario.Limonciello, platform-driver-x86, linux-kernel
My previous commit introduced a memory leak where the item allocated
from tlmi_setting was not freed.
This commit also renames it to avoid confusion with the similarly name
variable in the same function.
Fixes: 8a02d70679fc ("platform/x86: think-lmi: Add possible_values for ThinkStation")
Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Link: https://lore.kernel.org/lkml/df26ff45-8933-f2b3-25f4-6ee51ccda7d8@gmx.de/T/
Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
---
This patch series builds on top of the proposed patch from Armin Wolf
"platform/x86: think-lmi: Fix memory leak when showing current settings"
Changes in v2: None, but rebased series because of issues with second
patch.
drivers/platform/x86/think-lmi.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
index 6034df6d577d..87f832142d8d 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -1459,10 +1459,10 @@ static int tlmi_analyze(void)
* name string.
* Try and pull that out if it's available.
*/
- char *item, *optstart, *optend;
+ char *optitem, *optstart, *optend;
- if (!tlmi_setting(setting->index, &item, LENOVO_BIOS_SETTING_GUID)) {
- optstart = strstr(item, "[Optional:");
+ if (!tlmi_setting(setting->index, &optitem, LENOVO_BIOS_SETTING_GUID)) {
+ optstart = strstr(optitem, "[Optional:");
if (optstart) {
optstart += strlen("[Optional:");
optend = strstr(optstart, "]");
@@ -1471,6 +1471,7 @@ static int tlmi_analyze(void)
kstrndup(optstart, optend - optstart,
GFP_KERNEL);
}
+ kfree(optitem);
}
}
/*
--
2.39.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] platform/x86: think-lmi: Clean up display of current_value on Thinkstation
2023-04-03 1:31 [PATCH v2 1/2] platform/x86: think-lmi: Fix memory leaks when parsing ThinkStation WMI strings Mark Pearson
@ 2023-04-03 1:31 ` Mark Pearson
2023-04-03 2:05 ` Mario Limonciello
2023-04-03 8:21 ` Mirsad Goran Todorovac
2023-04-03 9:43 ` [PATCH v2 1/2] platform/x86: think-lmi: Fix memory leaks when parsing ThinkStation WMI strings Hans de Goede
1 sibling, 2 replies; 5+ messages in thread
From: Mark Pearson @ 2023-04-03 1:31 UTC (permalink / raw)
To: mpearson-lenovo
Cc: hdegoede, markgross, W_Armin, mirsad.todorovac, linux,
Mario.Limonciello, platform-driver-x86, linux-kernel
On ThinkStations on retrieving the attribute value the BIOS appends the
possible values to the string.
Clean up the display in the current_value_show function so the options
part is not displayed.
Fixes: a40cd7ef22fb ("platform/x86: think-lmi: Add WMI interface support on Lenovo platforms")
Reported by Mario Limoncello <Mario.Limonciello@amd.com>
Link: https://github.com/fwupd/fwupd/issues/5077#issuecomment-1488730526
Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
---
Changes in v2: For some reason v2 doesn't apply cleanly so rebased and
started again. Hopefully this one works
drivers/platform/x86/think-lmi.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
index 87f832142d8d..78dc82bda4dd 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -920,7 +920,7 @@ static ssize_t display_name_show(struct kobject *kobj, struct kobj_attribute *at
static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
{
struct tlmi_attr_setting *setting = to_tlmi_attr_setting(kobj);
- char *item, *value;
+ char *item, *value, *p;
int ret;
ret = tlmi_setting(setting->index, &item, LENOVO_BIOS_SETTING_GUID);
@@ -931,9 +931,12 @@ static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *a
value = strpbrk(item, ",");
if (!value || value == item || !strlen(value + 1))
ret = -EINVAL;
- else
+ else {
+ /* On Workstations remove the Options part after the value */
+ p = strchrnul(value, ';');
+ *p = '\0';
ret = sysfs_emit(buf, "%s\n", value + 1);
-
+ }
kfree(item);
return ret;
--
2.39.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] platform/x86: think-lmi: Clean up display of current_value on Thinkstation
2023-04-03 1:31 ` [PATCH v2 2/2] platform/x86: think-lmi: Clean up display of current_value on Thinkstation Mark Pearson
@ 2023-04-03 2:05 ` Mario Limonciello
2023-04-03 8:21 ` Mirsad Goran Todorovac
1 sibling, 0 replies; 5+ messages in thread
From: Mario Limonciello @ 2023-04-03 2:05 UTC (permalink / raw)
To: Mark Pearson
Cc: hdegoede, markgross, W_Armin, mirsad.todorovac, linux,
platform-driver-x86, linux-kernel
On 4/2/23 20:31, Mark Pearson wrote:
> On ThinkStations on retrieving the attribute value the BIOS appends the
> possible values to the string.
> Clean up the display in the current_value_show function so the options
> part is not displayed.
>
> Fixes: a40cd7ef22fb ("platform/x86: think-lmi: Add WMI interface support on Lenovo platforms")
> Reported by Mario Limoncello <Mario.Limonciello@amd.com>
> Link: https://github.com/fwupd/fwupd/issues/5077#issuecomment-1488730526
> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> ---
> Changes in v2: For some reason v2 doesn't apply cleanly so rebased and
> started again. Hopefully this one works
Tested-by: Mario Limonciello <mario.limonciello@amd.com>
tested on 6.3rc5 + prerequisite Armin's patch.
>
> drivers/platform/x86/think-lmi.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
> index 87f832142d8d..78dc82bda4dd 100644
> --- a/drivers/platform/x86/think-lmi.c
> +++ b/drivers/platform/x86/think-lmi.c
> @@ -920,7 +920,7 @@ static ssize_t display_name_show(struct kobject *kobj, struct kobj_attribute *at
> static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> {
> struct tlmi_attr_setting *setting = to_tlmi_attr_setting(kobj);
> - char *item, *value;
> + char *item, *value, *p;
> int ret;
>
> ret = tlmi_setting(setting->index, &item, LENOVO_BIOS_SETTING_GUID);
> @@ -931,9 +931,12 @@ static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *a
> value = strpbrk(item, ",");
> if (!value || value == item || !strlen(value + 1))
> ret = -EINVAL;
> - else
> + else {
> + /* On Workstations remove the Options part after the value */
> + p = strchrnul(value, ';');
> + *p = '\0';
> ret = sysfs_emit(buf, "%s\n", value + 1);
> -
> + }
> kfree(item);
>
> return ret;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] platform/x86: think-lmi: Clean up display of current_value on Thinkstation
2023-04-03 1:31 ` [PATCH v2 2/2] platform/x86: think-lmi: Clean up display of current_value on Thinkstation Mark Pearson
2023-04-03 2:05 ` Mario Limonciello
@ 2023-04-03 8:21 ` Mirsad Goran Todorovac
1 sibling, 0 replies; 5+ messages in thread
From: Mirsad Goran Todorovac @ 2023-04-03 8:21 UTC (permalink / raw)
To: Mark Pearson
Cc: hdegoede, markgross, W_Armin, linux, Mario.Limonciello,
platform-driver-x86, linux-kernel
On 3.4.2023. 3:31, Mark Pearson wrote:
> On ThinkStations on retrieving the attribute value the BIOS appends the
> possible values to the string.
> Clean up the display in the current_value_show function so the options
> part is not displayed.
>
> Fixes: a40cd7ef22fb ("platform/x86: think-lmi: Add WMI interface support on Lenovo platforms")
> Reported by Mario Limoncello <Mario.Limonciello@amd.com>
> Link: https://github.com/fwupd/fwupd/issues/5077#issuecomment-1488730526
> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> ---
> Changes in v2: For some reason v2 doesn't apply cleanly so rebased and
> started again. Hopefully this one works
>
> drivers/platform/x86/think-lmi.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
> index 87f832142d8d..78dc82bda4dd 100644
> --- a/drivers/platform/x86/think-lmi.c
> +++ b/drivers/platform/x86/think-lmi.c
> @@ -920,7 +920,7 @@ static ssize_t display_name_show(struct kobject *kobj, struct kobj_attribute *at
> static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> {
> struct tlmi_attr_setting *setting = to_tlmi_attr_setting(kobj);
> - char *item, *value;
> + char *item, *value, *p;
> int ret;
>
> ret = tlmi_setting(setting->index, &item, LENOVO_BIOS_SETTING_GUID);
> @@ -931,9 +931,12 @@ static ssize_t current_value_show(struct kobject *kobj, struct kobj_attribute *a
> value = strpbrk(item, ",");
> if (!value || value == item || !strlen(value + 1))
> ret = -EINVAL;
> - else
> + else {
> + /* On Workstations remove the Options part after the value */
> + p = strchrnul(value, ';');
> + *p = '\0';
> ret = sysfs_emit(buf, "%s\n", value + 1);
> -
> + }
> kfree(item);
>
> return ret;
I can confirm the build against the Torvalds 6.3-rc5 tree and Armin's patch
applied w these.
[root@pc-mtodorov kernel]# uname -rms
Linux 6.3.0-rc5-mt-20230401-00005-g10de4cefccf7 x86_64
[root@pc-mtodorov kernel]# cat /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]# echo scan > !$
echo scan > /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]# cat /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]# echo scan > /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]# cat /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]# echo scan > /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]# cat /sys/kernel/debug/kmemleak
[root@pc-mtodorov kernel]#
The leak is apparently gone in the original setup that reproduced the leak.
At your convenience, you can please add
Tested-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Have a nice day.
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] platform/x86: think-lmi: Fix memory leaks when parsing ThinkStation WMI strings
2023-04-03 1:31 [PATCH v2 1/2] platform/x86: think-lmi: Fix memory leaks when parsing ThinkStation WMI strings Mark Pearson
2023-04-03 1:31 ` [PATCH v2 2/2] platform/x86: think-lmi: Clean up display of current_value on Thinkstation Mark Pearson
@ 2023-04-03 9:43 ` Hans de Goede
1 sibling, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2023-04-03 9:43 UTC (permalink / raw)
To: Mark Pearson
Cc: markgross, W_Armin, mirsad.todorovac, linux, Mario.Limonciello,
platform-driver-x86, linux-kernel
Hi,
On 4/3/23 03:31, Mark Pearson wrote:
> My previous commit introduced a memory leak where the item allocated
> from tlmi_setting was not freed.
> This commit also renames it to avoid confusion with the similarly name
> variable in the same function.
>
> Fixes: 8a02d70679fc ("platform/x86: think-lmi: Add possible_values for ThinkStation")
> Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
> Link: https://lore.kernel.org/lkml/df26ff45-8933-f2b3-25f4-6ee51ccda7d8@gmx.de/T/
> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> ---
> This patch series builds on top of the proposed patch from Armin Wolf
> "platform/x86: think-lmi: Fix memory leak when showing current settings"
> Changes in v2: None, but rebased series because of issues with second
> patch.
Thank you for your patch series, I've applied this series
to my fixes branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes
Note it will show up in my fixes branch once I've pushed my
local branch there, which might take a while.
I will include this series in my next fixes pull-req to Linus
for the current kernel development cycle.
Regards,
Hans
>
> drivers/platform/x86/think-lmi.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
> index 6034df6d577d..87f832142d8d 100644
> --- a/drivers/platform/x86/think-lmi.c
> +++ b/drivers/platform/x86/think-lmi.c
> @@ -1459,10 +1459,10 @@ static int tlmi_analyze(void)
> * name string.
> * Try and pull that out if it's available.
> */
> - char *item, *optstart, *optend;
> + char *optitem, *optstart, *optend;
>
> - if (!tlmi_setting(setting->index, &item, LENOVO_BIOS_SETTING_GUID)) {
> - optstart = strstr(item, "[Optional:");
> + if (!tlmi_setting(setting->index, &optitem, LENOVO_BIOS_SETTING_GUID)) {
> + optstart = strstr(optitem, "[Optional:");
> if (optstart) {
> optstart += strlen("[Optional:");
> optend = strstr(optstart, "]");
> @@ -1471,6 +1471,7 @@ static int tlmi_analyze(void)
> kstrndup(optstart, optend - optstart,
> GFP_KERNEL);
> }
> + kfree(optitem);
> }
> }
> /*
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-04-03 9:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-03 1:31 [PATCH v2 1/2] platform/x86: think-lmi: Fix memory leaks when parsing ThinkStation WMI strings Mark Pearson
2023-04-03 1:31 ` [PATCH v2 2/2] platform/x86: think-lmi: Clean up display of current_value on Thinkstation Mark Pearson
2023-04-03 2:05 ` Mario Limonciello
2023-04-03 8:21 ` Mirsad Goran Todorovac
2023-04-03 9:43 ` [PATCH v2 1/2] platform/x86: think-lmi: Fix memory leaks when parsing ThinkStation WMI strings Hans de Goede
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).