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

[Rafael J. Wysocki (Intel)]

On Fri, Sep 11, 2026 at 6:29 PM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> On Fri, Sep 11, 2026 at 03:00:51PM +0200, Rafael J. Wysocki wrote:
>
> > 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).
>
> ...
>
> >       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;
>
> I was about ranting on goto after devm_*(), but looking at the context,
> I understand why it's not a problem. While at it, a side note: perhaps it makes
> sense to use ACPI_FREE(obj) instead of kfree()?

It should be ACPI_FREE() strictly speaking.

> Or even better to have __free() version of it, so we can declare the object with autoclean.

That one is a bit tricky, but I think I have an idea how to do it.
I'll post something next week.