linux-kernel - Re: [PATCH v2] ACPI: LPSS: use acpi_dev_uid_match() for matching

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]

Date:   Fri, 27 Oct 2023 19:40:38 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Raag Jadav <raag.jadav@...el.com>
Cc:     Mika Westerberg <mika.westerberg@...ux.intel.com>,
        rafael@...nel.org, len.brown@...el.com,
        andriy.shevchenko@...ux.intel.com, linux-acpi@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        mallikarjunappa.sangannavar@...el.com, bala.senthil@...el.com
Subject: Re: [PATCH v2] ACPI: LPSS: use acpi_dev_uid_match() for matching _UID

On Fri, Oct 27, 2023 at 7:19 PM Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> On Fri, Oct 27, 2023 at 6:51 PM Raag Jadav <raag.jadav@...el.com> wrote:
> >
> > On Fri, Oct 27, 2023 at 05:28:56PM +0300, Mika Westerberg wrote:
> > > On Fri, Oct 27, 2023 at 05:17:12PM +0300, Raag Jadav wrote:
> > > > Or perhaps something like,
> > > >
> > > > bool acpi_dev_uid_match(struct acpi_device *adev, const void *uid2, enum uid_type type)
> > > > {
> > > >         u64 uid1_d, uid2_d;
> > > >
> > > >         if (type == UID_TYPE_STR) {
> > > >                 char *uid2_s = (char *)uid2;
> > > >                 if (!(uid2_s && !kstrtou64(uid2_s, 0, &uid2_d)))
> > > >                         return false;
> > > >         } else if (type == UID_TYPE_INT) {
> > > >                 u64 *uid2_p;
> > > >                 uid2_p = (u64 *)uid2;
> > > >                 uid2_d = *uid2_p;
> > > >         } else {
> > > >                 return false;
> > > >         }
> > > >
> > > >         if (!acpi_dev_uid_to_integer(adev, &uid1_d) && uid1_d == uid2_d)
> > > >                 return true;
> > > >         else
> > > >                 return false;
> > > > }
> > > >
> > > > Although this looks unnecessarily hideous.
> > >
> > > Indeed, but using the _Generic() you should be able to have
> > > a single acpi_dev_uid_match() to work for either type so:
> > >
> > > acpi_dev_uid_match(adev, "1")
> > >
> > > and
> > >
> > > acpi_dev_uid_match(adev, 1)
> > >
> > > would both work with type checkings etc.
> > >
> > > Not sure if this is worth the trouble though.
> >
> > Well, in that case we can probably try both and hope for the best ;)
> >
> > bool acpi_dev_uid_match(struct acpi_device *adev, const char *uid2)
> > {
> >         const char *uid1 = acpi_device_uid(adev);
> >         u64 uid1_d;
> >
> >         return uid1 && uid2 && (!strcmp(uid1, uid2) ||
> >                (!kstrtou64(uid1, 0, &uid1_d) && uid1_d == (u64)uid2));
> > }
> >
> > But I'm guessing the compiler wouldn't be very happy about this.
>
> IMO it would be better to use the observation that if the uid2 string
> can be successfully cast to an int and the device's unique_id string
> can't be cast to an int (or the other way around), there is no match.
>
> If they both can be cast to an int, they match as long as the casting
> results are equal.
>
> If none of them can be cast to an int,  they need to be compared as strings.

Or if the strings don't match literally, try to convert them both to
ints and compare.