[PATCH] pcmcia: tcic: fix init_tcic() error handling

Guangshuo Li posted 1 patch 2 months ago
drivers/pcmcia/tcic.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
[PATCH] pcmcia: tcic: fix init_tcic() error handling
Posted by Guangshuo Li 2 months ago
When platform_device_register() fails in init_tcic(), the embedded
struct device in tcic_device has already been initialized by
device_initialize(), but the failure path does not drop the device
reference for the current platform device:

  init_tcic()
    -> platform_device_register(&tcic_device)
       -> device_initialize(&tcic_device.dev)
       -> setup_pdev_dma_masks(&tcic_device)
       -> platform_device_add(&tcic_device)

This leads to a reference leak when platform_device_register() fails.

The reference leak was identified by a static analysis tool I developed
and confirmed by manual review. While reviewing the code, I also found
that init_tcic() continues to use tcic_device.dev as the parent for
registered sockets even if platform device registration fails, and that
the pcmcia_register_socket() failure path only unregisters the first
socket instead of rolling back all previously registered sockets.

Fix all of these issues by checking the return value from
platform_device_register(), calling platform_device_put() on failure,
stopping the initialization immediately, and properly unwinding already
registered sockets and other resources on later failures.

Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/pcmcia/tcic.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/pcmcia/tcic.c b/drivers/pcmcia/tcic.c
index 060aed0edc65..43bda9930645 100644
--- a/drivers/pcmcia/tcic.c
+++ b/drivers/pcmcia/tcic.c
@@ -362,6 +362,7 @@ static int __init init_tcic(void)
 {
     int i, sock, ret = 0;
     u_int mask, scan;
+	bool irq_registered = false;
 
     if (platform_driver_register(&tcic_driver))
 	return -1;
@@ -464,8 +465,10 @@ static int __init init_tcic(void)
 	for (i = 15; i > 0; i--)
 	    if ((cs_mask & (1 << i)) &&
 		(request_irq(i, tcic_interrupt, 0, "tcic",
-			     tcic_interrupt) == 0))
+				tcic_interrupt) == 0)) {
+		irq_registered = true;
 		break;
+		}
 	cs_irq = i;
 	if (cs_irq == 0) poll_interval = HZ;
     }
@@ -486,20 +489,33 @@ static int __init init_tcic(void)
     /* jump start interrupt handler, if needed */
     tcic_interrupt(0, NULL);
 
-    platform_device_register(&tcic_device);
+	ret = platform_device_register(&tcic_device);
+	if (ret) {
+		platform_device_put(&tcic_device);
+		goto out_cleanup;
+	}
 
     for (i = 0; i < sockets; i++) {
 	    socket_table[i].socket.ops = &tcic_operations;
 	    socket_table[i].socket.resource_ops = &pccard_nonstatic_ops;
 	    socket_table[i].socket.dev.parent = &tcic_device.dev;
 	    ret = pcmcia_register_socket(&socket_table[i].socket);
-	    if (ret && i)
-		    pcmcia_unregister_socket(&socket_table[0].socket);
+		if (ret)
+			goto out_unregister_sockets;
     }
     
     return ret;
 
-    return 0;
+out_unregister_sockets:
+	while (i--)
+		pcmcia_unregister_socket(&socket_table[i].socket);
+	platform_device_unregister(&tcic_device);
+out_cleanup:
+	if (irq_registered)
+		free_irq(cs_irq, tcic_interrupt);
+	release_region(tcic_base, 16);
+	platform_driver_unregister(&tcic_driver);
+	return ret;
     
 } /* init_tcic */
 
-- 
2.43.0
Re: [PATCH] pcmcia: tcic: fix init_tcic() error handling
Posted by Guangshuo Li 1 month, 3 weeks ago
Hi,

Please disregard this patch.

On Thu, 16 Apr 2026 at 01:36, Guangshuo Li <lgs201920130244@gmail.com> wrote:
>
> When platform_device_register() fails in init_tcic(), the embedded
> struct device in tcic_device has already been initialized by
> device_initialize(), but the failure path does not drop the device
> reference for the current platform device:
>
>   init_tcic()
>     -> platform_device_register(&tcic_device)
>        -> device_initialize(&tcic_device.dev)
>        -> setup_pdev_dma_masks(&tcic_device)
>        -> platform_device_add(&tcic_device)
>
> This leads to a reference leak when platform_device_register() fails.
>
> The reference leak was identified by a static analysis tool I developed
> and confirmed by manual review. While reviewing the code, I also found
> that init_tcic() continues to use tcic_device.dev as the parent for
> registered sockets even if platform device registration fails, and that
> the pcmcia_register_socket() failure path only unregisters the first
> socket instead of rolling back all previously registered sockets.
>
> Fix all of these issues by checking the return value from
> platform_device_register(), calling platform_device_put() on failure,
> stopping the initialization immediately, and properly unwinding already
> registered sockets and other resources on later failures.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/pcmcia/tcic.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pcmcia/tcic.c b/drivers/pcmcia/tcic.c
> index 060aed0edc65..43bda9930645 100644
> --- a/drivers/pcmcia/tcic.c
> +++ b/drivers/pcmcia/tcic.c
> @@ -362,6 +362,7 @@ static int __init init_tcic(void)
>  {
>      int i, sock, ret = 0;
>      u_int mask, scan;
> +       bool irq_registered = false;
>
>      if (platform_driver_register(&tcic_driver))
>         return -1;
> @@ -464,8 +465,10 @@ static int __init init_tcic(void)
>         for (i = 15; i > 0; i--)
>             if ((cs_mask & (1 << i)) &&
>                 (request_irq(i, tcic_interrupt, 0, "tcic",
> -                            tcic_interrupt) == 0))
> +                               tcic_interrupt) == 0)) {
> +               irq_registered = true;
>                 break;
> +               }
>         cs_irq = i;
>         if (cs_irq == 0) poll_interval = HZ;
>      }
> @@ -486,20 +489,33 @@ static int __init init_tcic(void)
>      /* jump start interrupt handler, if needed */
>      tcic_interrupt(0, NULL);
>
> -    platform_device_register(&tcic_device);
> +       ret = platform_device_register(&tcic_device);
> +       if (ret) {
> +               platform_device_put(&tcic_device);
> +               goto out_cleanup;
> +       }
>
>      for (i = 0; i < sockets; i++) {
>             socket_table[i].socket.ops = &tcic_operations;
>             socket_table[i].socket.resource_ops = &pccard_nonstatic_ops;
>             socket_table[i].socket.dev.parent = &tcic_device.dev;
>             ret = pcmcia_register_socket(&socket_table[i].socket);
> -           if (ret && i)
> -                   pcmcia_unregister_socket(&socket_table[0].socket);
> +               if (ret)
> +                       goto out_unregister_sockets;
>      }
>
>      return ret;
>
> -    return 0;
> +out_unregister_sockets:
> +       while (i--)
> +               pcmcia_unregister_socket(&socket_table[i].socket);
> +       platform_device_unregister(&tcic_device);
> +out_cleanup:
> +       if (irq_registered)
> +               free_irq(cs_irq, tcic_interrupt);
> +       release_region(tcic_base, 16);
> +       platform_driver_unregister(&tcic_driver);
> +       return ret;
>
>  } /* init_tcic */
>
> --
> 2.43.0
>

After re-checking it, tcic_device is a static platform_device and it does
not provide a dev.release callback. Therefore calling
platform_device_put() on the platform_device_register() failure path is
not appropriate here and can trigger the missing release callback
warning.

This falls into the same static platform_device pattern pointed out in
the other reviews, so I will drop this version.

Sorry for the noise.

Best regards,
Guangshuo Li