linux-kernel - Re: [PATCH] ACPI / scan: Set the visited flag for all enumerated devices

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: <CAJZ5v0gbeF=zG_H9ZJasOpojx0mVJDa_4+0JB9sgAYHJs5ASrg@mail.gmail.com>
Date:   Tue, 11 Apr 2017 16:15:51 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Linux ACPI <linux-acpi@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ACPI / scan: Set the visited flag for all enumerated devices

On Tue, Apr 11, 2017 at 11:36 AM, Mika Westerberg
<mika.westerberg@...ux.intel.com> wrote:
> On Tue, Apr 11, 2017 at 12:23:42AM +0200, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>>
>> Commit 10c7e20b2ff3 (ACPI / scan: fix enumeration (visited) flags for
>> bus rescans) attempted to fix a problem with ACPI-based enumerateion
>> of I2C/SPI devices, but it forgot to ensure that the visited flag
>> will be set for all of the other enumerated devices, so fix that.
>>
>> Fixes: 10c7e20b2ff3 (ACPI / scan: fix enumeration (visited) flags for bus rescans)
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> Reviewed-by: Mika Westerberg <mika.westerberg@...ux.intel.com>

Thanks for the review, but I'm actually unsure about one thing.

Namely, we may want to call acpi_default_enumeration(device) even if
an ACPI driver has been bound to the device.

The reason why is because this runs before driver modules are loaded
(and quite likely before ACPI drivers are registered too) and
acpi_default_enumeration(device) is going to be invoked even if there
is a matching ACPI driver, but it is registered later.  That driver
can still bind to the device object going forward, of course, so there
can be an ACPI device object with both a companion "physical" device
and an ACPI driver bound at the same time anyway.

However, if acpi_bus_scan() is called for the same device as a result
of a bus/device check notification, for example, the
acpi_default_enumeration(device) will be skipped for device objects
with ACPI drivers bound (as of the current version of the patch) which
is quite inconsistent IMO.

Thanks,
Rafael