[PATCH v2 4/5] staging: dgap: tty.c: changes error handing to tty driver allocations

Dan Carpenter dan.carpenter at oracle.com
Thu Sep 26 10:19:47 UTC 2013


This one is not right.

On Wed, Sep 25, 2013 at 07:08:53PM -0400, Lidza Louina wrote:
> This patch changes error handling to the
> tty_driver allocations in dgap_tty_register.
> 
> Before, it didn't handle the possibility of an
> alloc_tty_driver failure. This patch makes
> dgap_register_driver return -ENOMEM if that fails.
> 
> This patch also adds handling to the possibility that
> the brd->SerialDriver->ttys or brd->PrintDriver->ttys
> allocation will fail. It now calls put_tty_driver on
> that driver after it fails and returns -ENOMEM.
> 
> Signed-off-by: Lidza Louina <lidza.louina at gmail.com>
> ---
>  drivers/staging/dgap/dgap_tty.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/dgap/dgap_tty.c b/drivers/staging/dgap/dgap_tty.c
> index 59fda2e..3e12ddb 100644
> --- a/drivers/staging/dgap/dgap_tty.c
> +++ b/drivers/staging/dgap/dgap_tty.c
> @@ -220,6 +220,8 @@ int dgap_tty_register(struct board_t *brd)
>  	DPR_INIT(("tty_register start"));
>  
>  	brd->SerialDriver = alloc_tty_driver(MAXPORTS);
> +	if (!brd->SerialDriver)
> +		return -ENOMEM;
>  
>  	snprintf(brd->SerialName, MAXTTYNAMELEN, "tty_dgap_%d_", brd->boardnum);
>  	brd->SerialDriver->name = brd->SerialName;
> @@ -234,9 +236,10 @@ int dgap_tty_register(struct board_t *brd)
>  
>  	/* The kernel wants space to store pointers to tty_structs */
>  	brd->SerialDriver->ttys = dgap_driver_kzmalloc(MAXPORTS * sizeof(struct tty_struct *), GFP_KERNEL);
> -	if (!brd->SerialDriver->ttys)
> -		return(-ENOMEM);
> -
> +	if (!brd->SerialDriver->ttys){
> +		rc = -ENOMEM;
> +		goto err_put_tty_serial;	
> +	}
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,28)
>  	brd->SerialDriver->refcount = brd->TtyRefCnt;
>  #endif
> @@ -253,6 +256,8 @@ int dgap_tty_register(struct board_t *brd)
>  	 * we are when we get into the dgap_tty_open() routine.
>  	 */
>  	brd->PrintDriver = alloc_tty_driver(MAXPORTS);
> +	if (!brd->PrintDriver)
> +		return -ENOMEM;

	if (!brd->PrintDriver) {
		rc = -ENOMEM:
		goto err_free_serial_ttys;
	}

>  
>  	snprintf(brd->PrintName, MAXTTYNAMELEN, "pr_dgap_%d_", brd->boardnum);
>  	brd->PrintDriver->name = brd->PrintName;
> @@ -267,9 +272,10 @@ int dgap_tty_register(struct board_t *brd)
>  
>  	/* The kernel wants space to store pointers to tty_structs */
>  	brd->PrintDriver->ttys = dgap_driver_kzmalloc(MAXPORTS * sizeof(struct tty_struct *), GFP_KERNEL);
> -	if (!brd->PrintDriver->ttys)
> -		return(-ENOMEM);
> -
> +	if (!brd->PrintDriver->ttys){
> +		rc = -ENOMEM;
> +		goto err_put_tty_print;
> +	}
>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,28)
>  	brd->PrintDriver->refcount = brd->TtyRefCnt;
>  #endif
> @@ -285,7 +291,7 @@ int dgap_tty_register(struct board_t *brd)
>  		rc = tty_register_driver(brd->SerialDriver);
>  		if (rc < 0) {
>  			APR(("Can't register tty device (%d)\n", rc));
> -			return(rc);
> +			goto err_put_tty_serial;

			goto err_free_print_ttys;

>  		}
>  		brd->dgap_Major_Serial_Registered = TRUE;
>  		dgap_BoardsByMajor[brd->SerialDriver->major] = brd;
> @@ -297,7 +303,7 @@ int dgap_tty_register(struct board_t *brd)
>   		rc = tty_register_driver(brd->PrintDriver);
>  		if (rc < 0) {
>  			APR(("Can't register Transparent Print device (%d)\n", rc));
> -			return(rc);
> +			goto err_put_tty_print;

			goto err_unregister_serial;

>  		}
>  		brd->dgap_Major_TransparentPrint_Registered = TRUE;
>  		dgap_BoardsByMajor[brd->PrintDriver->major] = brd;
> @@ -307,6 +313,10 @@ int dgap_tty_register(struct board_t *brd)
>  	DPR_INIT(("DGAP REGISTER TTY: MAJORS: %d %d\n", brd->SerialDriver->major,
>  		brd->PrintDriver->major));

Your patch frees everything by mistake on the success path.  It should
be:

	return 0;

err_unregister_serial:
	tty_unregister_driver(brd->SerialDriver);
err_free_print_ttys:
	kfree(brd->PrintDriver->ttys);
err_put_tty_print:
	put_tty_driver(brd->PrintDriver);
err_free_serial_ttys:
	kfree(brd->SerialDriver->ttys);
err_put_tty_serial:
	put_tty_driver(brd->SerialDriver);

	return rc;
}

regards,
dan carpenter


More information about the devel mailing list