[PATCH] staging: dgnc: Fix a possible crash when the class_create fails

Dan Carpenter dan.carpenter at oracle.com
Wed Oct 15 09:17:01 UTC 2014


On Mon, Oct 13, 2014 at 10:26:54PM +0530, devendranaga4 at gmail.com wrote:
> From: Devendra Naga <devendranaga4 at gmail.com>
> 

Just fix your email client, so the from header is correct.  These From
headers should be for when you forward an email from someone else.

> The class_create function can return a pointer with an error code
> stuffed inside it if it fails. Handled it to make sure
> device_register doesn't get an invalid pointer.
> 
> Also removed assignment of rc to 0 in the beginning of function
> because it gets changed after the call to register_chrdev function.
> 
> Signed-off-by: Devendra Naga <devendranaga4 at gmail.com>
> ---
>  drivers/staging/dgnc/dgnc_driver.c |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
> index 2154665..72bfb29 100644
> --- a/drivers/staging/dgnc/dgnc_driver.c
> +++ b/drivers/staging/dgnc/dgnc_driver.c
> @@ -248,7 +248,7 @@ module_exit(dgnc_cleanup_module);
>   */
>  static int dgnc_start(void)
>  {
> -	int rc = 0;
> +	int rc;

Well done.  :)  Removing this was the right thing.

>  	unsigned long flags;
>  
>  	/* make sure that the globals are init'd before we do anything else */
> @@ -271,6 +271,12 @@ static int dgnc_start(void)
>  	dgnc_Major = rc;
>  
>  	dgnc_class = class_create(THIS_MODULE, "dgnc_mgmt");
> +	if (IS_ERR(dgnc_class)) {
> +		rc = PTR_ERR(dgnc_class);
> +		APR(("Can't creat dgnc class (%d)\n", rc));

No need for an error message here.  The lower levels handle this.

> +		goto bad;
> +	}
> +
>  	device_create(dgnc_class, NULL,
>  		MKDEV(dgnc_Major, 0),
>  		NULL, "dgnc_mgmt");
> @@ -297,6 +303,10 @@ static int dgnc_start(void)
>  	add_timer(&dgnc_poll_timer);
>  
>  	return rc;

This should be "return 0;"

Add  a goto after dgnc_tty_preinit() while you are at it.  Call the
patch "fix error handling in dgnc_start()".

> +
> +bad:

This label is not as useful as could be.  We already know it's "bad"
when class_create() failed so it doesn't add any new information.
Call is something like: "err_unregister:".


> +	unregister_chrdev(dgnc_Major, "dgnc");
> +	return rc;

regards,
dan carpenter



More information about the devel mailing list