linux-kernel - Re: [PATCH] ACPI: bus: Match first 9 bytes of device IDs

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMj1kXGtANm3SMoREymDSyx+wpn3L=Ex5q5mpgQigOwmEp33Lg@mail.gmail.com>
Date:   Fri, 25 Feb 2022 18:13:38 +0100
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Alexander Graf <graf@...zon.com>
Cc:     ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Len Brown <lenb@...nel.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        "Jason A . Donenfeld" <Jason@...c4.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        "Woodhouse, David" <dwmw@...zon.co.uk>
Subject: Re: [PATCH] ACPI: bus: Match first 9 bytes of device IDs

On Fri, 25 Feb 2022 at 16:56, Alexander Graf <graf@...zon.com> wrote:
>
> We create a list of ACPI "PNP" IDs which contains _HID, _CID and CLS
> entries of the respective devices. However, we squeeze them into
> struct acpi_device_id which only has 9 bytes space to store the identifier
> based on the ACPI spec:
>
> """
> A _HID object evaluates to either a numeric 32-bit compressed EISA
> type ID or a string. If a string, the format must be an alphanumeric
> PNP or ACPI ID with no asterisk or other leading characters.
> A valid PNP ID must be of the form "AAA####" where A is an uppercase
> letter and # is a hex digit.
> A valid ACPI ID must be of the form "NNNN####" where N is an uppercase
> letter or a digit ('0'-'9') and # is a hex digit. This specification
> reserves the string "ACPI" for use only with devices defined herein.
> It further reserves all strings representing 4 HEX digits for
> exclusive use with PCI-assigned Vendor IDs.
> """
>
> While most people adhere to the ACPI specs, Microsoft decided that its
> VM Generation Counter device [1] should only be identifiable by _CID with
> an value of "VM_Gen_Counter" - longer than 9 characters.
>
> To still allow device drivers to match identifiers that exceed the 9 byte
> limit, without wasting memory for the unlikely case that you have long
> identifiers, let's match only the first 9 characters of the identifier.
>
> This patch is a prerequisite to add support for VMGenID in Linux [2].
>
> [1] https://download.microsoft.com/download/3/1/C/31CFC307-98CA-4CA5-914C-D9772691E214/VirtualMachineGenerationID.docx
> [2] https://lore.kernel.org/lkml/20220225124848.909093-1-Jason@zx2c4.com/
>
> Signed-off-by: Alexander Graf <graf@...zon.com>
>
> ---
>
> Alternatives to the approach above would be:
>
>   1) Always set id[8] = '\0' in acpi_add_id()
>   2) Allocate the id in struct acpi_device_id dynamically
>
> I'm happy to explore option 1 instead if people believe it's cleaner.
> Option 2 on the other hand seems overkill for the issue at hand. We don't
> have a lot of devices that exceed the 8 character threshold, so chance of
> collision is quite small. On the other hand, the extra overhead of
> maintaining the string allocation dynamically will quickly become a
> headache.
>

I don't like this hack. If we are going to accept the fact that CIDs
could be arbitrary length strings, we should handle them properly.

The device subsystem side of things already deals with this properly:
the modalias of the QEMU vmgenid device comes up as
'acpi:QEMUVGID:VM_GEN_COUNTER', which means it already captures the
entire string, and exposes it in the correct way (modulo the all caps)

So what we need is a way for a module to describe its compatibility
with such a _CID, which shouldn't be that complicated.

I'll try to cook something up.


> ---
>  drivers/acpi/bus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 07f604832fd6..aba93171739f 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -829,7 +829,7 @@ static bool __acpi_match_device(struct acpi_device *device,
>                 /* First, check the ACPI/PNP IDs provided by the caller. */
>                 if (acpi_ids) {
>                         for (id = acpi_ids; id->id[0] || id->cls; id++) {
> -                               if (id->id[0] && !strcmp((char *)id->id, hwid->id))
> +                               if (id->id[0] && !strncmp((char *)id->id, hwid->id, ACPI_ID_LEN))
>                                         goto out_acpi_match;
>                                 if (id->cls && __acpi_match_device_cls(id, hwid))
>                                         goto out_acpi_match;
> --
> 2.16.4
>
>
>
>
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879
>
>
>