linux-kernel - RE: [2.6 patch] drivers/block/cciss.c: fix check-after-use

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]

Message-ID: <226E1C65E4F6164E8EA5FD3CC913AE8C01A3F8A2@G3W0639.americas.hpqcorp.net>
Date:	Wed, 1 Aug 2007 15:40:30 -0000
From:	"Miller, Mike (OS Dev)" <Mike.Miller@...com>
To:	"Adrian Bunk" <bunk@...sta.de>,
	"Jens Axboe" <jens.axboe@...cle.com>
Cc:	"ISS StorageDev" <iss_storagedev@...com>,
	<linux-kernel@...r.kernel.org>
Subject: RE: [2.6 patch] drivers/block/cciss.c: fix check-after-use

 

> -----Original Message-----
> From: Adrian Bunk [mailto:bunk@...sta.de] 
> Sent: Monday, July 30, 2007 5:28 PM
> To: Miller, Mike (OS Dev); Jens Axboe
> Cc: ISS StorageDev; linux-kernel@...r.kernel.org
> Subject: [2.6 patch] drivers/block/cciss.c: fix check-after-use
> 
> The Coverity checker spotted that we have already oops'ed if "disk"
> was NULL.
> 
> Since "disk" being NULL seems impossible at this point this 
> patch removes the NULL check.
> 
> Signed-off-by: Adrian Bunk <bunk@...sta.de>

Acked-by: Mike Miller <mike.miller@...com>

> 
> ---
> 
>  drivers/block/cciss.c |   56 
> ++++++++++++++++++++----------------------
>  1 file changed, 27 insertions(+), 29 deletions(-)
> 
> --- linux-2.6.23-rc1-mm1/drivers/block/cciss.c.old	
> 2007-07-30 02:27:15.000000000 +0200
> +++ linux-2.6.23-rc1-mm1/drivers/block/cciss.c	
> 2007-07-30 02:28:28.000000000 +0200
> @@ -1569,66 +1569,64 @@ static int deregister_disk(struct gendis
>  	ctlr_info_t *h = get_host(disk);
>  
>  	if (!capable(CAP_SYS_RAWIO))
>  		return -EPERM;
>  
>  	/* make sure logical volume is NOT is use */
>  	if (clear_all || (h->gendisk[0] == disk)) {
>  		if (drv->usage_count > 1)
>  			return -EBUSY;
>  	} else if (drv->usage_count > 0)
>  		return -EBUSY;
>  
>  	/* invalidate the devices and deregister the disk.  If 
> it is disk
>  	 * zero do not deregister it but just zero out it's 
> values.  This
>  	 * allows us to delete disk zero but keep the 
> controller registered.
>  	 */
>  	if (h->gendisk[0] != disk) {
> -		if (disk) {
> -			struct request_queue *q = disk->queue;
> -			if (disk->flags & GENHD_FL_UP)
> -				del_gendisk(disk);
> -			if (q) {
> -				blk_cleanup_queue(q);
> -				/* Set drv->queue to NULL so 
> that we do not try
> -				 * to call blk_start_queue on 
> this queue in the
> -				 * interrupt handler
> -				 */
> -				drv->queue = NULL;
> -			}
> -			/* If clear_all is set then we are 
> deleting the logical
> -			 * drive, not just refreshing its info. 
>  For drives
> -			 * other than disk 0 we will call 
> put_disk.  We do not
> -			 * do this for disk 0 as we need it to 
> be able to
> -			 * configure the controller.
> +		struct request_queue *q = disk->queue;
> +		if (disk->flags & GENHD_FL_UP)
> +			del_gendisk(disk);
> +		if (q) {
> +			blk_cleanup_queue(q);
> +			/* Set drv->queue to NULL so that we do not try
> +			 * to call blk_start_queue on this queue in the
> +			 * interrupt handler
> +			 */
> +			drv->queue = NULL;
> +		}
> +		/* If clear_all is set then we are deleting the logical
> +		 * drive, not just refreshing its info.  For drives
> +		 * other than disk 0 we will call put_disk.  We do not
> +		 * do this for disk 0 as we need it to be able to
> +		 * configure the controller.
> +		*/
> +		if (clear_all){
> +			/* This isn't pretty, but we need to find the
> +			 * disk in our array and NULL our the pointer.
> +			 * This is so that we will call alloc_disk if
> +			 * this index is used again later.
>  			*/
> -			if (clear_all){
> -				/* This isn't pretty, but we 
> need to find the
> -				 * disk in our array and NULL 
> our the pointer.
> -				 * This is so that we will call 
> alloc_disk if
> -				 * this index is used again later.
> -				*/
> -				for (i=0; i < CISS_MAX_LUN; i++){
> -					if(h->gendisk[i] == disk){
> -						h->gendisk[i] = NULL;
> -						break;
> -					}
> +			for (i=0; i < CISS_MAX_LUN; i++){
> +				if(h->gendisk[i] == disk){
> +					h->gendisk[i] = NULL;
> +					break;
>  				}
> -				put_disk(disk);
>  			}
> +			put_disk(disk);
>  		}
>  	} else {
>  		set_capacity(disk, 0);
>  	}
>  
>  	--h->num_luns;
>  	/* zero out the disk size info */
>  	drv->nr_blocks = 0;
>  	drv->block_size = 0;
>  	drv->heads = 0;
>  	drv->sectors = 0;
>  	drv->cylinders = 0;
>  	drv->raid_level = -1;	/* This can be used as a flag 
> variable to
>  				 * indicate that this element 
> of the drive
>  				 * array is free.
>  				 */
>  
> 
> 
-
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/