Re: [PATCH v1 01/10] ACPI: fan: Fix memory leak due to leftover devm_kcalloc() argument

[Armin Wolf]

Am 11.09.26 um 15:00 schrieb Rafael J. Wysocki:

> From: "Rafael J. Wysocki" <rafael.j.wysocki@xxxxxxxxx>
>
> An ACPI device object's dev field in passed as the first argument to
> devm_kcalloc() in acpi_fan_get_fps() which is incorrect and leads to
> a memory leak on driver probe errors and removal because the driver
> is not bound to that ACPI device.
>
> Address this by replacing that pointer with a pointer to the device the
> driver is actually bound to.
>
> While at it, drop a redundant error message after a memory allocation
> failure (that also gets printed relative to the ACPI device).

Reviewed-by: Armin Wolf <W_Armin@xxxxxx>

> Fixes: d91a1d129b63 ("ACPI: fan: Use platform device for devres-related actions")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> ---
>   drivers/acpi/fan_core.c | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c
> index 624d0736b581..a31d7beca5d0 100644
> --- a/drivers/acpi/fan_core.c
> +++ b/drivers/acpi/fan_core.c
> @@ -284,7 +284,7 @@ static int acpi_fan_speed_cmp(const void *a, const void *b)
>   	return fps1->speed - fps2->speed;
>   }
>   
> -static int acpi_fan_get_fps(struct acpi_device *device)
> +static int acpi_fan_get_fps(struct device *dev, struct acpi_device *device)
>   {
>   	struct acpi_fan *fan = acpi_driver_data(device);
>   	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> @@ -304,11 +304,8 @@ static int acpi_fan_get_fps(struct acpi_device *device)
>   	}
>   
>   	fan->fps_count = obj->package.count - 1; /* minus revision field */
> -	fan->fps = devm_kcalloc(&device->dev,
> -				fan->fps_count, sizeof(struct acpi_fan_fps),
> -				GFP_KERNEL);
> +	fan->fps = devm_kcalloc(dev, fan->fps_count, sizeof(*fan->fps), GFP_KERNEL);
>   	if (!fan->fps) {
> -		dev_err(&device->dev, "Not enough memory\n");
>   		status = -ENOMEM;
>   		goto err;
>   	}
> @@ -522,7 +519,7 @@ static int acpi_fan_probe(struct platform_device *pdev)
>   		if (result)
>   			return result;
>   
> -		result = acpi_fan_get_fps(device);
> +		result = acpi_fan_get_fps(&pdev->dev, device);
>   		if (result)
>   			return result;
>   	}