Hi,
On 3/31/23 20:54, Mark Pearson wrote:
> Hi all,
>
> On Wed, Mar 29, 2023, at 5:50 PM, Mirsad Goran Todorovac wrote:
>> On 29. 03. 2023. 21:21, Thomas Weißschuh wrote:
>>>
>>> Mar 29, 2023 14:00:22 Mark Pearson <mpearson-lenovo@xxxxxxxxx>:
>>>
>>>> Thanks Mirsad
>>>>
>>>> On Wed, Mar 29, 2023, at 2:49 PM, Mirsad Goran Todorovac wrote:
>>>> <snip>
>>>>>
>>>>> Here is the patch proposal according to what Mark advised (using
>>>>> different name for optitem):
>>>>>
>>>>> diff --git a/drivers/platform/x86/think-lmi.c
>>>>> b/drivers/platform/x86/think-lmi.c
>>>>> index c816646eb661..ab17254781c4 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);
>>>>> @@ -1380,7 +1382,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) &&
>>>>> @@ -1417,8 +1418,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)
>>>>
>>>> Really minor, but tweak to be this and save a line of code?
>>>
>>> This hunk is actually from another commit and should not be needed here.
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/platform/x86/think-lmi.c?id=da62908efe80f132f691efc2ace4ca67626de86b
>>
>> Thank you, Thomas,
>>
>> Indeed, my mistake.
>>
>> I have accepted Armin's suggestion to test if that patch closed the leak, and I
>> have just quoted it, never claiming authorship.
>>
>> I ought to apologise if I made confusion here.
>>
>> I was a bit euphoric about the leak being fixed, so forgive me for this blatant
>> mistake. Of course, putting it here would cause a patch collision, so it was a
>> stupid thing to do, and I would never do it in a formal patch submission ...
>>
>> Thanks, anyway for correction.
>>
>> Best regards,
>> Mirsad
>>
>
> I have the patches ready to fix this issue - I just wanted to check that I wouldn't be stepping on anybodies toes or if there is a protocol for doing this.
> - I will add Reported-by tag for Mirsad and Suggested-by for Armin.
> - I've identified Fixes tags for the two commits that caused the issue.
> Let me know if there's anything else I should do - otherwise I'll get them sent out ASAP.
This sounds to me like you have covered all the bases.
Note Armin did send out a related fix earlier today,
which I guess is duplicate with one of your patches:
https://patchwork.kernel.org/project/platform-driver-x86/patch/20230331180912.38392-1-W_Armin@xxxxxx/
So maybe add Armin's patch on top of pdx86/fixes and
use that as a base for your series (dropping your
likely duplicate patch) ?
Regards,
Hans
