PNP: Release protocol device on registration failure

[PATCH v1] PNP: Release protocol device on registration failure

Posted by Yuho Choi 1 month, 3 weeks ago

pnp_register_protocol() adds a protocol to the PNP list before calling
device_register(). If device registration fails, pnp_remove_protocol() only
removes the list entry and leaves the device-core reference acquired by
device_initialize() held.

Give static protocol devices a release callback and drop the reference on
registration failure. Also stop PNP ACPI device enumeration when protocol
registration fails, instead of using an unregistered protocol device as a
parent.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
---
 drivers/pnp/core.c         | 9 ++++++++-
 drivers/pnp/pnpacpi/core.c | 7 ++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c
index 81603327079c..fc3bff43fdd8 100644
--- a/drivers/pnp/core.c
+++ b/drivers/pnp/core.c
@@ -23,6 +23,10 @@ static LIST_HEAD(pnp_protocols);
 LIST_HEAD(pnp_global);
 DEFINE_MUTEX(pnp_lock);
 
+static void pnp_protocol_release(struct device *dev)
+{
+}
+
 /*
  * ACPI or PNPBIOS should tell us about all platform devices, so we can
  * skip some blind probes.  ISAPNP typically enumerates only plug-in ISA
@@ -66,14 +70,17 @@ int pnp_register_protocol(struct pnp_protocol *protocol)
 
 	protocol->number = nodenum;
 	dev_set_name(&protocol->dev, "pnp%d", nodenum);
+	protocol->dev.release = pnp_protocol_release;
 
 	list_add_tail(&protocol->protocol_list, &pnp_protocols);
 
 	mutex_unlock(&pnp_lock);
 
 	ret = device_register(&protocol->dev);
-	if (ret)
+	if (ret) {
 		pnp_remove_protocol(protocol);
+		put_device(&protocol->dev);
+	}
 
 	return ret;
 }
diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index fbf03ff007eb..da0ebc378696 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -298,12 +298,17 @@ static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle,
 int pnpacpi_disabled __initdata;
 static int __init pnpacpi_init(void)
 {
+	int ret;
+
 	if (acpi_disabled || pnpacpi_disabled) {
 		printk(KERN_INFO "pnp: PnP ACPI: disabled\n");
 		return 0;
 	}
 	printk(KERN_INFO "pnp: PnP ACPI init\n");
-	pnp_register_protocol(&pnpacpi_protocol);
+	ret = pnp_register_protocol(&pnpacpi_protocol);
+	if (ret)
+		return ret;
+
 	acpi_get_devices(NULL, pnpacpi_add_device_handler, NULL, NULL);
 	printk(KERN_INFO "pnp: PnP ACPI: found %d devices\n", num);
 	pnp_platform_devices = 1;
-- 
2.43.0

Re: [PATCH v1] PNP: Release protocol device on registration failure

Posted by Rafael J. Wysocki (Intel) 1 month, 3 weeks ago

On Fri, Aug 7, 2026 at 5:55 AM Yuho Choi <dbgh9129@gmail.com> wrote:
>
> pnp_register_protocol() adds a protocol to the PNP list before calling
> device_register(). If device registration fails, pnp_remove_protocol() only
> removes the list entry and leaves the device-core reference acquired by
> device_initialize() held.
>
> Give static protocol devices a release callback and drop the reference on
> registration failure. Also stop PNP ACPI device enumeration when protocol
> registration fails, instead of using an unregistered protocol device as a
> parent.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
> ---
>  drivers/pnp/core.c         | 9 ++++++++-
>  drivers/pnp/pnpacpi/core.c | 7 ++++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c
> index 81603327079c..fc3bff43fdd8 100644
> --- a/drivers/pnp/core.c
> +++ b/drivers/pnp/core.c
> @@ -23,6 +23,10 @@ static LIST_HEAD(pnp_protocols);
>  LIST_HEAD(pnp_global);
>  DEFINE_MUTEX(pnp_lock);
>
> +static void pnp_protocol_release(struct device *dev)
> +{
> +}
> +
>  /*
>   * ACPI or PNPBIOS should tell us about all platform devices, so we can
>   * skip some blind probes.  ISAPNP typically enumerates only plug-in ISA
> @@ -66,14 +70,17 @@ int pnp_register_protocol(struct pnp_protocol *protocol)
>
>         protocol->number = nodenum;
>         dev_set_name(&protocol->dev, "pnp%d", nodenum);
> +       protocol->dev.release = pnp_protocol_release;

Sashiko complains about adding an empty release callback here and IMV
it has a point:

https://sashiko.dev/#/patchset/20260807035453.948129-1-dbgh9129%40gmail.com

>
>         list_add_tail(&protocol->protocol_list, &pnp_protocols);
>
>         mutex_unlock(&pnp_lock);
>
>         ret = device_register(&protocol->dev);
> -       if (ret)
> +       if (ret) {
>                 pnp_remove_protocol(protocol);
> +               put_device(&protocol->dev);
> +       }
>
>         return ret;
>  }
> diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
> index fbf03ff007eb..da0ebc378696 100644
> --- a/drivers/pnp/pnpacpi/core.c
> +++ b/drivers/pnp/pnpacpi/core.c
> @@ -298,12 +298,17 @@ static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle,
>  int pnpacpi_disabled __initdata;
>  static int __init pnpacpi_init(void)
>  {
> +       int ret;
> +
>         if (acpi_disabled || pnpacpi_disabled) {
>                 printk(KERN_INFO "pnp: PnP ACPI: disabled\n");
>                 return 0;
>         }
>         printk(KERN_INFO "pnp: PnP ACPI init\n");
> -       pnp_register_protocol(&pnpacpi_protocol);
> +       ret = pnp_register_protocol(&pnpacpi_protocol);
> +       if (ret)
> +               return ret;
> +
>         acpi_get_devices(NULL, pnpacpi_add_device_handler, NULL, NULL);
>         printk(KERN_INFO "pnp: PnP ACPI: found %d devices\n", num);
>         pnp_platform_devices = 1;
> --
> 2.43.0
>