Re: [PATCH v2] platform/x86: asus-wireless: Fail probe when there is no ACPI match

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3128 bytes --]

On Thu, 9 Jul 2026, Ilpo Järvinen wrote:

> On Wed, 8 Jul 2026, Wysocki, Rafael J wrote:
> 
> > On 7/8/2026 3:07 AM, Linmao Li wrote:
> > > Every platform driver can be forced to match a device that doesn't match
> > > its list of device IDs because of device_match_driver_override(), so
> > > platform drivers that rely on the existence of a device's ACPI companion
> > > object need to verify its presence.
> > > 
> > > asus_wireless_probe() returns success when acpi_match_acpi_device()
> > > finds no match, leaving behind an input device that never reports
> > > anything because the notify handler is not installed.  Worse, when the
> > > driver is force-bound to a device without an ACPI companion, probe
> > > still succeeds and stores a NULL companion pointer, which
> > > asus_wireless_remove() later passes to acpi_dev_remove_notify_handler(),
> > > leading to a NULL pointer dereference on unbind.
> > > 
> > > Return -ENODEV when the device does not match the ID table.  This also
> > > covers the missing-companion case, because acpi_match_acpi_device()
> > > rejects a NULL device.
> > > 
> > > Fixes: f7e648027d7e ("platform/x86: asus-wireless: Convert ACPI driver to a
> > > platform one")
> > > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> > 
> > Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > (no issues found)
> > 
> > > ---
> > > v2: Fail probe when the ACPI ID match fails instead of adding a
> > >      separate ACPI companion check at the top of probe (Rafael).
> > > 
> > >   drivers/platform/x86/asus-wireless.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/platform/x86/asus-wireless.c
> > > b/drivers/platform/x86/asus-wireless.c
> > > index 2b494bf3cba8..d33853a1baf3 100644
> > > --- a/drivers/platform/x86/asus-wireless.c
> > > +++ b/drivers/platform/x86/asus-wireless.c
> > > @@ -155,7 +155,7 @@ static int asus_wireless_probe(struct platform_device
> > > *pdev)
> > >     	id = acpi_match_acpi_device(device_ids, adev);
> > >   	if (!id)
> > > -		return 0;
> > > +		return -ENODEV;
> 
> I'm somewhat unsure what the intent here is because the driver is using 
> device_ids as its .acpi_match_table anyway so what's the benefit of doing 
> matching again for the same set of ids?
> 
> Of course having this prevents those overrides but is it really a task for 
> a platform driver to prevent user shooting himself into a foot with that 
> gun (I mean beyond checking NULL like v1 did)?
> 
> I'd tend to think this matching can be removed and the v1 check done 
> instead. But Rafael who undoubtedly knows ACPI much better than I do seems 
> to be in favor of this version?

Nevermind about this, I see now it want to collect ->driver_data...

> I also don't understand why this check is done so late into the probe, 
> IMO, it should simply be done before setting up the input device 
> (logically just before assign adev).

...But I still think it should be done earlier.

-- 
 i.