[4/6] platform/x86: wmi: Fix probe failure when failing to register WMI devices
Commit Message
When a WMI device besides the first one somehow fails to register, retval
is returned while still containing a negative error code. This causes the
ACPI device failing to probe, leaving behind zombie WMI devices leading
to various errors later.
Fix this by handling the single error path separately and return 0 after
trying to register all WMI devices. Also continue to register WMI devices
even if some fail to allocate.
Fixes: 6ee50aaa9a20 ("platform/x86: wmi: Instantiate all devices before adding them")
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
drivers/platform/x86/wmi.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
--
2.39.2
Comments
On Sun, 8 Oct 2023, Armin Wolf wrote:
> When a WMI device besides the first one somehow fails to register, retval
> is returned while still containing a negative error code. This causes the
> ACPI device failing to probe, leaving behind zombie WMI devices leading
> to various errors later.
> Fix this by handling the single error path separately and return 0 after
> trying to register all WMI devices. Also continue to register WMI devices
> even if some fail to allocate.
I think the usual approach would be to unroll all registerations done so
far when an error occurs while registering n devices.
Do you Hans have something to add what would be the best course of action
here?
Am 12.10.23 um 18:32 schrieb Ilpo Järvinen:
> On Sun, 8 Oct 2023, Armin Wolf wrote:
>
>> When a WMI device besides the first one somehow fails to register, retval
>> is returned while still containing a negative error code. This causes the
>> ACPI device failing to probe, leaving behind zombie WMI devices leading
>> to various errors later.
>> Fix this by handling the single error path separately and return 0 after
>> trying to register all WMI devices. Also continue to register WMI devices
>> even if some fail to allocate.
> I think the usual approach would be to unroll all registerations done so
> far when an error occurs while registering n devices.
I agree, however the surrounding code unrolls only the WMI device registration,
so i kept it that way. After all, this patch focuses on fixing the "zombie" WMI devices
problem, so changing the code to unroll all registrations should be done in a separate
patch IMHO.
Armin Wolf
> Do you Hans have something to add what would be the best course of action
> here?
>
@@ -1338,8 +1338,8 @@ static int parse_wdg(struct device *wmi_bus_dev, struct platform_device *pdev)
struct wmi_block *wblock;
union acpi_object *obj;
acpi_status status;
- int retval = 0;
u32 i, total;
+ int retval;
status = acpi_evaluate_object(device->handle, "_WDG", NULL, &out);
if (ACPI_FAILURE(status))
@@ -1350,8 +1350,8 @@ static int parse_wdg(struct device *wmi_bus_dev, struct platform_device *pdev)
return -ENXIO;
if (obj->type != ACPI_TYPE_BUFFER) {
- retval = -ENXIO;
- goto out_free_pointer;
+ kfree(obj);
+ return -ENXIO;
}
gblock = (const struct guid_block *)obj->buffer.pointer;
@@ -1366,8 +1366,8 @@ static int parse_wdg(struct device *wmi_bus_dev, struct platform_device *pdev)
wblock = kzalloc(sizeof(*wblock), GFP_KERNEL);
if (!wblock) {
- retval = -ENOMEM;
- break;
+ dev_err(wmi_bus_dev, "Failed to allocate %pUL\n", &gblock[i].guid);
+ continue;
}
wblock->acpi_device = device;
@@ -1398,9 +1398,9 @@ static int parse_wdg(struct device *wmi_bus_dev, struct platform_device *pdev)
}
}
-out_free_pointer:
- kfree(out.pointer);
- return retval;
+ kfree(obj);
+
+ return 0;
}
/*