linux-kernel - Re: [PATCH v6] Introduce support for Systems Management Driver over WMI for Dell Systems

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7f40bc72-83cc-c265-ac46-9543f74d228a@redhat.com>
Date:   Mon, 26 Oct 2020 16:41:44 +0100
From:   Hans de Goede <hdegoede@...hat.com>
To:     "Limonciello, Mario" <Mario.Limonciello@...l.com>,
        Divya Bharathi <divya27392@...il.com>,
        "dvhart@...radead.org" <dvhart@...radead.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        "platform-driver-x86@...r.kernel.org" 
        <platform-driver-x86@...r.kernel.org>,
        "Bharathi, Divya" <Divya.Bharathi@...l.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        mark gross <mgross@...ux.intel.com>,
        "Ksr, Prasanth" <Prasanth.Ksr@...l.com>
Subject: Re: [PATCH v6] Introduce support for Systems Management Driver over
 WMI for Dell Systems

Hi,

On 10/26/20 4:39 PM, Limonciello, Mario wrote:
>> This was present in previous versions too, but I just noticed this are you
>> sure that using
>> .string.pointer is correct here? That seems wrong since the pointer gets
>> allocated by
>> the Linux ACPI core, so it is not under influence of the AML code?
>>
>> I think you want / need to use ".integer.value" here ?
>>
>> And maybe first do a type check, e.g.:
>>
>> 	if (obj->package.elements[CURRENT_VAL].type != ACPI_TYPE_INTEGER) {
>> 		ret = -EINVAL;
>> 		goto out;
>> 	}
>>
>> Adding this type check will also show if I'm right that you should use
>> .integer.value ...
>>
>> 	ret = snprintf(buf, PAGE_SIZE, "%lld\n", obj-
> 
> We'll need to double check this, but I'm pretty sure the firmware outputs
> everything as a string.

Ok, in that case it should probably be printed as a "%s" though and not interpret
the buffer-address which the kernel allocated for storing the string as an
integer.

And it would still be good to do the type-check for this, but then checking
for a type of ACPI_TYPE_STRING.

Regards,

Hans