From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Johan Hovold <johan@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
Guangshuo Li <lgs201920130244@gmail.com>,
Will Deacon <will@kernel.org>,
Anshuman Khandual <anshuman.khandual@arm.com>,
linux-arm-kernel@lists.infradead.org,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH] arm_pmu: acpi: fix reference leak on failed device registration
Date: Mon, 20 Apr 2026 10:05:13 +0200 [thread overview]
Message-ID: <2026042058-charm-storable-4ad8@gregkh> (raw)
In-Reply-To: <aeXVr5enpjb3rfq7@hovoldconsulting.com>
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
prev parent reply other threads:[~2026-04-20 8:05 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-15 17:41 [PATCH] arm_pmu: acpi: fix reference leak on failed device registration Guangshuo Li
2026-04-15 18:19 ` Mark Rutland
2026-04-16 4:40 ` Greg Kroah-Hartman
2026-04-16 6:34 ` Guangshuo Li
2026-04-16 7:23 ` Johan Hovold
2026-04-16 8:59 ` Guangshuo Li
2026-04-16 9:50 ` Mark Rutland
2026-04-16 9:30 ` Mark Rutland
2026-04-20 7:28 ` Johan Hovold
2026-04-20 8:05 ` Greg Kroah-Hartman [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2026042058-charm-storable-4ad8@gregkh \
--to=gregkh@linuxfoundation.org \
--cc=anshuman.khandual@arm.com \
--cc=johan@kernel.org \
--cc=lgs201920130244@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=stable@vger.kernel.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox