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?
--
i.
> Fixes: 6ee50aaa9a20 ("platform/x86: wmi: Instantiate all devices before adding them")
> Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
> ---
> drivers/platform/x86/wmi.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index e3984801883a..ab24ea9ffc9a 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -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;
> }
>
> /*
> --
> 2.39.2
>
