Re: [PATCH -next] Input: serio - fix possible memory leak while device_add() fails

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Zhu,

On Tue, Aug 01, 2023 at 07:51:05PM +0800, Zhu Wang wrote:
> If device_add() returns error, the name allocated by dev_set_name() need
> be freed. As comment of device_add() says, it should use put_device() to
> release the reference in the error path. So fix this by calling
> put_device, then the name can be freed in kobject_cleanp().

Unfortunately this is not as simple. To support serio ports chained
behind other serio ports (to accommodate Synaptics touchpads with
Trackpoints connected to pass-through ports) serio device registration
is done on a thread. So even if you free the device the driver that
ultimately issued serio_add_port() call will not be aware of the failure
and will continue using the device, which will much more likely crash
the kernel as opposed to leaving the device structure around.

> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Zhu Wang <wangzhu9@huawei.com>
> ---
>  drivers/input/serio/serio.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
> index 767fc9efb4a8..d3bb6ec91326 100644
> --- a/drivers/input/serio/serio.c
> +++ b/drivers/input/serio/serio.c
> @@ -537,10 +537,12 @@ static void serio_add_port(struct serio *serio)
>  		serio->start(serio);
>  
>  	error = device_add(&serio->dev);
> -	if (error)
> +	if (error) {
> +		put_device(&serio->dev);
>  		dev_err(&serio->dev,
>  			"device_add() failed for %s (%s), error: %d\n",
>  			serio->phys, serio->name, error);
> +	}
>  }
>  
>  /*
> -- 
> 2.17.1
> 

Thanks.

-- 
Dmitry