Re: [PATCH] serial: 8250_accent: fix reference leak on failed device registration

[Jiri Slaby <jirislaby@kernel.org>]

Hi,

On 15. 04. 26, 20:34, Guangshuo Li wrote:
> When platform_device_register() fails in accent_init(), the embedded
> struct device in accent_device has already been initialized by
> device_initialize(), but the failure path returns the error without
> dropping the device reference for the current platform device:
> 
>    accent_init()
>      -> platform_device_register(&accent_device)
>         -> device_initialize(&accent_device.dev)
>         -> setup_pdev_dma_masks(&accent_device)
>         -> platform_device_add(&accent_device)
> 
> This leads to a reference leak when platform_device_register() fails.

What reference exactly?

> Fix this by calling platform_device_put() before returning the error.
> 
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.

How did you verify you did the right change?

> Fixes: ec9f47cd6a14c ("[PATCH] Serial: Split 8250 port table")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>   drivers/tty/serial/8250/8250_accent.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_accent.c b/drivers/tty/serial/8250/8250_accent.c
> index 1691f1a57f89..e9cf40268c0e 100644
> --- a/drivers/tty/serial/8250/8250_accent.c
> +++ b/drivers/tty/serial/8250/8250_accent.c
> @@ -25,7 +25,13 @@ static struct platform_device accent_device = {
>   
>   static int __init accent_init(void)
>   {
> -	return platform_device_register(&accent_device);
> +	int ret;
> +
> +	ret = platform_device_register(&accent_device);
> +	if (ret)
> +		platform_device_put(&accent_device);

In particular, what does put_device() do on a static device, even 
initialized, ie. with no device::release? Try it...

IMO, all the patches are bogus.

thanks,
-- 
js
suse labs