linux-kernel - Re: [PATCH v3 1/2] char: pcmcia: scr24x_cs: Fix failure handling of device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 27 May 2021 14:44:32 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     nizamhaider786@...il.com
Cc:     lkundrak@...sk, arnd@...db.de, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] char: pcmcia: scr24x_cs: Fix failure handling of
 device_create()

On Tue, May 25, 2021 at 03:22:01AM +0530, nizamhaider786@...il.com wrote:
> From: Nijam Haider <nizamhaider786@...il.com>
> 
> Ignored error in device_create() and pcmcia_enable_device()
> this patch implements proper error handling.
> 
> Signed-off-by: Nijam Haider <nizamhaider786@...il.com>
> ---
> V2 -> V3: Added description, Changelog and removed whitespace error
> V1 -> V2: Split the patch into two parts and addressed review comments
> ---
>  drivers/char/pcmcia/scr24x_cs.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/pcmcia/scr24x_cs.c b/drivers/char/pcmcia/scr24x_cs.c
> index 47feb39af34c..b48e79356611 100644
> --- a/drivers/char/pcmcia/scr24x_cs.c
> +++ b/drivers/char/pcmcia/scr24x_cs.c
> @@ -233,6 +233,7 @@ static int scr24x_probe(struct pcmcia_device *link)
>  {
>  	struct scr24x_dev *dev;
>  	int ret;
> +	struct device *dev_ret;
>  
>  	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>  	if (!dev)
> @@ -272,12 +273,20 @@ static int scr24x_probe(struct pcmcia_device *link)
>  
>  	ret = pcmcia_enable_device(link);
>  	if (ret < 0) {
> +		cdev_del(&dev->c_dev);
>  		pcmcia_disable_device(link);
>  		goto err;
>  	}
>  
> -	device_create(scr24x_class, NULL, MKDEV(MAJOR(scr24x_devt), dev->devno),
> +	dev_ret = device_create(scr24x_class, NULL, MKDEV(MAJOR(scr24x_devt), dev->devno),
>  		      NULL, "scr24x%d", dev->devno);
> +	if (IS_ERR(dev_ret)) {
> +		dev_err(&link->dev, "device_create failed for %d\n",
> +			dev->devno);
> +		cdev_del(&dev->c_dev);
> +		pcmcia_disable_device(link);
> +		goto err;
> +	}

The "better" way to do this is to have more err_: labels that do the
unwinding for you, so that you do not have to duplicate the same logic
in multiple places, like you are doing here.

Can you change this patch to do that instead?  Should be shorter overall
than this one and easier to maintain over time.

thanks,

greg k-h