Chrome platform driver development
 help / color / mirror / Atom feed
* [PATCH 2/5] platform/chrome: cros_ec_lpc: fix reference leak on failed device registration
       [not found] <cover.1777889235.git.vebohr@gmail.com>
@ 2026-05-04 10:08 ` Vastargazing
  2026-05-05  2:40   ` Tzung-Bi Shih
  2026-05-05 10:36 ` [PATCH 1/5] perf/arm_pmu_acpi: fix reference leak in arm_pmu_acpi_probe error path Valery Borovsky
  1 sibling, 1 reply; 3+ messages in thread
From: Vastargazing @ 2026-05-04 10:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Vastargazing, stable, Benson Leung, Tzung-Bi Shih, Guenter Roeck,
	Gwendal Grignou, Thierry Escande, Enric Balletbo i Serra,
	chrome-platform

When platform_device_register() fails in cros_ec_lpc_init(), the embedded
struct device has already been initialized by device_initialize() inside
platform_device_register(). The error path unregisters the driver but
returns without dropping the device reference:

  cros_ec_lpc_init()
    -> platform_device_register(&cros_ec_lpc_device)
       -> device_initialize(&cros_ec_lpc_device.dev)   /* kref = 1 */
       -> platform_device_add(&cros_ec_lpc_device)     /* fails */
    <- platform_driver_unregister() called, but kref still 1

Per platform_device_register() kernel-doc:

  NOTE: _Never_ directly free @pdev after calling this function, even if
  it returned an error! Always use platform_device_put() to give up the
  reference initialised in this function instead.

Fix this by calling platform_device_put() before unregistering the driver.

Fixes: 5f454bdf6353 ("platform/chrome: cros_ec_lpc: Register the driver if ACPI entry is missing.")
Cc: stable@vger.kernel.org
Assisted-by: GitHub Copilot (Claude Sonnet 4.5)
Signed-off-by: Vastargazing <vebohr@gmail.com>
---
 drivers/platform/chrome/cros_ec_lpc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 78cfff80cdea..cb3ff76d29e9 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -892,6 +892,7 @@ static int __init cros_ec_lpc_init(void)
 		ret = platform_device_register(&cros_ec_lpc_device);
 		if (ret) {
 			pr_err(DRV_NAME ": can't register device: %d\n", ret);
+			platform_device_put(&cros_ec_lpc_device);
 			platform_driver_unregister(&cros_ec_lpc_driver);
 		}
 	}
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 2/5] platform/chrome: cros_ec_lpc: fix reference leak on failed device registration
  2026-05-04 10:08 ` [PATCH 2/5] platform/chrome: cros_ec_lpc: fix reference leak on failed device registration Vastargazing
@ 2026-05-05  2:40   ` Tzung-Bi Shih
  0 siblings, 0 replies; 3+ messages in thread
From: Tzung-Bi Shih @ 2026-05-05  2:40 UTC (permalink / raw)
  To: Vastargazing
  Cc: linux-kernel, stable, Benson Leung, Guenter Roeck,
	Gwendal Grignou, Thierry Escande, Enric Balletbo i Serra,
	chrome-platform

On Mon, May 04, 2026 at 01:08:44PM +0300, Vastargazing wrote:
> When platform_device_register() fails in cros_ec_lpc_init(), the embedded
> struct device has already been initialized by device_initialize() inside
> platform_device_register(). The error path unregisters the driver but
> returns without dropping the device reference:
> 
>   cros_ec_lpc_init()
>     -> platform_device_register(&cros_ec_lpc_device)
>        -> device_initialize(&cros_ec_lpc_device.dev)   /* kref = 1 */
>        -> platform_device_add(&cros_ec_lpc_device)     /* fails */
>     <- platform_driver_unregister() called, but kref still 1
> 
> Per platform_device_register() kernel-doc:
> 
>   NOTE: _Never_ directly free @pdev after calling this function, even if
>   it returned an error! Always use platform_device_put() to give up the
>   reference initialised in this function instead.
> 
> Fix this by calling platform_device_put() before unregistering the driver.
> 
> Fixes: 5f454bdf6353 ("platform/chrome: cros_ec_lpc: Register the driver if ACPI entry is missing.")
> Cc: stable@vger.kernel.org
> Assisted-by: GitHub Copilot (Claude Sonnet 4.5)
> Signed-off-by: Vastargazing <vebohr@gmail.com>
> ---
>  drivers/platform/chrome/cros_ec_lpc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 78cfff80cdea..cb3ff76d29e9 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -892,6 +892,7 @@ static int __init cros_ec_lpc_init(void)
>  		ret = platform_device_register(&cros_ec_lpc_device);
>  		if (ret) {
>  			pr_err(DRV_NAME ": can't register device: %d\n", ret);
> +			platform_device_put(&cros_ec_lpc_device);
>  			platform_driver_unregister(&cros_ec_lpc_driver);
>  		}
>  	}

The patch is identical to [1].  See also [2].

[1] https://lore.kernel.org/chrome-platform/20260415175707.3640225-1-lgs201920130244@gmail.com/
[2] https://lore.kernel.org/chrome-platform/CANUHTR9_c=msO7mAax=Aj4U--GB=2ozserOvrTU8WueqdSMN8Q@mail.gmail.com/

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/5] perf/arm_pmu_acpi: fix reference leak in arm_pmu_acpi_probe error path
       [not found] <cover.1777889235.git.vebohr@gmail.com>
  2026-05-04 10:08 ` [PATCH 2/5] platform/chrome: cros_ec_lpc: fix reference leak on failed device registration Vastargazing
@ 2026-05-05 10:36 ` Valery Borovsky
  1 sibling, 0 replies; 3+ messages in thread
From: Valery Borovsky @ 2026-05-05 10:36 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, Arnd Bergmann, Greg Kroah-Hartman, Benson Leung,
	Tzung-Bi Shih, Guenter Roeck, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Andy Shevchenko, Linus Walleij, Randy Dunlap,
	linux-arm-kernel, linux-perf-users, linux-kernel, chrome-platform,
	linux-mtd, Valery Borovsky

Yeah, you're right, my bad. The `arm_pmu_acpi.c` patch is definitely broken.

Since `spe_dev` and `trbe_dev` are statically allocated, they don't have a
`.dev.release` callback. If we hit `platform_device_put()` here, the refcount
drops to zero and triggers `device_release()`, which is going to scream about
the missing release function. At best, we get a messy WARN; at worst, it'll
panic the kernel if someone's running with `panic_on_warn`.

The kernel-doc note about `platform_device_put()` is really meant for dynamic
allocations where the release path actually frees memory. For static setups
like this, the original code is actually the right way to go.

Please drop patches 1/5 through 4/5 from the v1 series—they all suffer from
the same logic error. Patch 5/5 (mfd: sm501) is the only clean one, so I've
re-sent that as a standalone v2.

Sorry for the noise.

Valery Borovsky

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-05 10:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <cover.1777889235.git.vebohr@gmail.com>
2026-05-04 10:08 ` [PATCH 2/5] platform/chrome: cros_ec_lpc: fix reference leak on failed device registration Vastargazing
2026-05-05  2:40   ` Tzung-Bi Shih
2026-05-05 10:36 ` [PATCH 1/5] perf/arm_pmu_acpi: fix reference leak in arm_pmu_acpi_probe error path Valery Borovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox