linux-kernel - Re: [PATCH v5 WIP 5/5] paride: use new parport device model

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150519113255.GO22558@mwanda>
Date:	Tue, 19 May 2015 14:32:55 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Sudip Mukherjee <sudipm.mukherjee@...il.com>
Cc:	Greg KH <gregkh@...uxfoundation.org>,
	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
	Jean Delvare <jdelvare@...e.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 WIP 5/5] paride: use new parport device model

On Wed, May 06, 2015 at 03:46:17PM +0530, Sudip Mukherjee wrote:
> +void *pi_register_driver(char *name)
> +{
> +	struct parport_driver *parp_drv;
> +	int ret;
> +
> +	parp_drv = kzalloc(sizeof(*parp_drv), GFP_KERNEL);
> +	if (!parp_drv)
> +		return NULL;
> +
> +	parp_drv->name = name;
> +	parp_drv->probe = pi_probe;
> +
> +	ret = parport_register_driver(parp_drv);
> +
> +	if (ret)
> +		return NULL;

Remove the blank line between the call and the error handling.  We
should probably free parp_drv on error.

> +	return (void *)parp_drv;
> +}
> +EXPORT_SYMBOL(pi_register_driver);
> +
> +void pi_unregister_driver(void *_drv)
> +{
> +	struct parport_driver *drv = _drv;
> +
> +	parport_unregister_driver(drv);
> +	kfree(drv);
> +}
> +EXPORT_SYMBOL(pi_unregister_driver);
> diff --git a/drivers/block/paride/paride.h b/drivers/block/paride/paride.h
> index 2bddbf4..ddb9e58 100644
> --- a/drivers/block/paride/paride.h
> +++ b/drivers/block/paride/paride.h
> @@ -165,6 +165,8 @@ typedef struct pi_protocol PIP;
>  
>  extern int paride_register( PIP * );
>  extern void paride_unregister ( PIP * );
> +void *pi_register_driver(char *);
> +void pi_unregister_driver(void *);
>  
>  #endif /* __DRIVERS_PARIDE_H__ */
>  /* end of paride.h */
> diff --git a/drivers/block/paride/pcd.c b/drivers/block/paride/pcd.c
> index 3b7c9f1..9336236 100644
> --- a/drivers/block/paride/pcd.c
> +++ b/drivers/block/paride/pcd.c
> @@ -221,6 +221,7 @@ static int pcd_busy;		/* request being processed ? */
>  static int pcd_sector;		/* address of next requested sector */
>  static int pcd_count;		/* number of blocks still to do */
>  static char *pcd_buf;		/* buffer for request in progress */
> +static void *par_drv;		/* reference of parport driver */
>  
>  /* kernel glue structures */
>  
> @@ -690,6 +691,12 @@ static int pcd_detect(void)
>  	printk("%s: %s version %s, major %d, nice %d\n",
>  	       name, name, PCD_VERSION, major, nice);
>  
> +	par_drv = pi_register_driver(name);
> +	if (!par_drv) {
> +		pr_err("failed to register %s driver\n", name);
> +		return -1;
> +	}
> +
>  	k = 0;
>  	if (pcd_drive_count == 0) { /* nothing spec'd - so autoprobe for 1 */
>  		cd = pcd;
> @@ -723,6 +730,7 @@ static int pcd_detect(void)
>  	printk("%s: No CD-ROM drive found\n", name);
>  	for (unit = 0, cd = pcd; unit < PCD_UNITS; unit++, cd++)
>  		put_disk(cd->disk);
> +	pi_unregister_driver(par_drv);

You didn't introduce this, but I think the error handling here is broken
for the autoprobe case.  The autoprobe case doesn't have a for loop.

>  	return -1;
>  }
>  

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/