Re: [PATCH] arm_pmu: acpi: fix reference leak on failed device registration

Greg Kroah-Hartman
2026-04-20T08:05:16Z

On Mon, Apr 20, 2026 at 09:28:47AM +0200, Johan Hovold wrote:
> On Thu, Apr 16, 2026 at 10:30:23AM +0100, Mark Rutland wrote:
> > On Thu, Apr 16, 2026 at 09:23:33AM +0200, Johan Hovold wrote:
>
> > > It's not just the platform code as this directly reflects the behaviour
> > > of device_register() as Mark pointed out.
> > >
> > > It is indeed an unfortunate quirk of the driver model, but one can argue
> > > that having a registration function that frees its argument on errors
> > > would be even worse. And even more so when many (or most) users get this
> > > right.
> >
> > Ah, sorry; I had missed that the _put() step would actually free the
> > object (and as you explain below, how that won't work for many callers).
> >
> > > So if we want to change this, I think we would need to deprecate
> > > device_register() in favour of explicit device_initialize() and
> > > device_add().
> >
> > Is is possible to have {platfom_,}device_uninitialize() functions that
> > does everything except the ->release() call? If we had that, then we'd
> > be able to have a flow along the lines of:
> >
> > int some_init_function(void)
> > {
> > int err;
> >
> > platform_device_init(&static_pdev);
> >
> > err = platform_device_add(&static_pdev))
> > if (err)
> > goto out_uninit;
> >
> > return 0;
> >
> > out_uninit:
> > platform_device_uninit(&static_pdev);
> > return err;
> > }
> >
> > ... which I think would align with what people generally expect to have
> > to do.
>
> The issue here is that platform_device_add() allocates a device name and
> such resources are not released until the last reference is dropped.
>
> It's been this way since 2008, but some of the static platform devices
> predates that and they both lack a release callback (explicitly required
> since 2003) and are not cleaned up on registration failure.
>
> Since registration would essentially only fail during development (e.g.
> due to name collision or fault injection), this is hardly something to
> worry about, but we could consider moving towards dynamic objects to
> address both issues.

Agreed, this whole thing, including the error handling, is all just
theoretical as no real user ever hits this, which is why it has been
_way_ down my priority list.

> We have a few functions for allocating *and* registering platform
> devices that could be used in many of these cases (and they already
> clean up after themselves on errors):
>
> platform_device_register_simple()
> platform_device_register_data()
> platform_device_register_resndata()
> platform_device_register_full()
>
> and where those do not fit (and cannot be extended) we have the
> underlying:
>
> platform_device_alloc()
> platform_device_add_resources()
> platform_device_add_data()
> plaform_device_add()
>
> But there are some 800 static platform devices left, mostly in legacy
> platform code and board files that I assume few people care about.

Yes, I agree that we do have all of the needed apis here already, we
should just work at converting existing drivers to the new apis OR just
not caring at all as again, no one will ever hit these code paths :)

thanks,

greg k-h