Re: [PATCH V2] tty: vcc: Add check for kstrdup() in vcc_probe()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 4 Sep 2023 11:52:20 +0800
Yi Yang <yiyang13@xxxxxxxxxx> wrote:

> Add check for the return value of kstrdup() and return the error, if it
> fails in order to avoid NULL pointer dereference.
> 
> Fixes: 5d171050e28f ("sparc64: vcc: Enable VCC port probe and removal")
> Signed-off-by: Yi Yang <yiyang13@xxxxxxxxxx>
> ---
> V2: Add goto target for error paths.
> ---
>  drivers/tty/vcc.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c
> index a39ed981bfd3..5b625f20233b 100644
> --- a/drivers/tty/vcc.c
> +++ b/drivers/tty/vcc.c
> @@ -579,18 +579,22 @@ static int vcc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
>  		return -ENOMEM;
>  
>  	name = kstrdup(dev_name(&vdev->dev), GFP_KERNEL);
> +	if (!name) {
> +		rv = -ENOMEM;
> +		goto free_port;

Hi,
at this point, the port is not yet allocated, so you should not jump to
free_port. You should simply return with -ENOMEM.


> +	}
>  
>  	rv = vio_driver_init(&port->vio, vdev, VDEV_CONSOLE_CON, vcc_versions,
>  			     ARRAY_SIZE(vcc_versions), NULL, name);
>  	if (rv)
> -		goto free_port;
> +		goto free_name;
>  
>  	port->vio.debug = vcc_dbg_vio;
>  	vcc_ldc_cfg.debug = vcc_dbg_ldc;
>  
>  	rv = vio_ldc_alloc(&port->vio, &vcc_ldc_cfg, port);
>  	if (rv)
> -		goto free_port;
> +		goto free_name;

You should still jump to free_port, not free_name, after seeing my
comments below


>  
>  	spin_lock_init(&port->lock);
>  
> @@ -624,6 +628,11 @@ static int vcc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
>  		goto unreg_tty;
>  	}
>  	port->domain = kstrdup(domain, GFP_KERNEL);
> +	if (!port->domain) {
> +		rv = -ENOMEM;
> +		goto unreg_tty;
> +	}
> +
>  
>  	mdesc_release(hp);
>  
> @@ -653,8 +662,9 @@ static int vcc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
>  	vcc_table_remove(port->index);
>  free_ldc:
>  	vio_ldc_free(&port->vio);
> -free_port:
> +free_name:
>  	kfree(name);
> +free_port:
>  	kfree(port);

free_name should come after free_port...

Hugo.


>  
>  	return rv;
> -- 
> 2.17.1
> 



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux