* [2.6 patch] drivers/block/cciss.c: fix check-after-use
@ 2007-08-27 21:30 Adrian Bunk
0 siblings, 0 replies; 3+ messages in thread
From: Adrian Bunk @ 2007-08-27 21:30 UTC (permalink / raw)
To: Andrew Morton, Mike Miller, Jens Axboe; +Cc: iss_storagedev, linux-kernel
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@stusta.de>
Acked-by: Mike Miller <mike.miller@hp.com>
---
This patch has been sent on:
- 31 Jul 2007
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.
*/
^ permalink raw reply [flat|nested] 3+ messages in thread* [2.6 patch] drivers/block/cciss.c: fix check-after-use
@ 2007-07-30 22:28 Adrian Bunk
2007-08-01 15:40 ` Miller, Mike (OS Dev)
0 siblings, 1 reply; 3+ messages in thread
From: Adrian Bunk @ 2007-07-30 22:28 UTC (permalink / raw)
To: Mike Miller, Jens Axboe; +Cc: iss_storagedev, linux-kernel
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@stusta.de>
---
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.
*/
^ permalink raw reply [flat|nested] 3+ messages in thread* RE: [2.6 patch] drivers/block/cciss.c: fix check-after-use
2007-07-30 22:28 Adrian Bunk
@ 2007-08-01 15:40 ` Miller, Mike (OS Dev)
0 siblings, 0 replies; 3+ messages in thread
From: Miller, Mike (OS Dev) @ 2007-08-01 15:40 UTC (permalink / raw)
To: Adrian Bunk, Jens Axboe; +Cc: ISS StorageDev, linux-kernel
> -----Original Message-----
> From: Adrian Bunk [mailto:bunk@stusta.de]
> Sent: Monday, July 30, 2007 5:28 PM
> To: Miller, Mike (OS Dev); Jens Axboe
> Cc: ISS StorageDev; linux-kernel@vger.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@stusta.de>
Acked-by: Mike Miller <mike.miller@hp.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.
> */
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-08-27 21:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-27 21:30 [2.6 patch] drivers/block/cciss.c: fix check-after-use Adrian Bunk
-- strict thread matches above, loose matches on Subject: below --
2007-07-30 22:28 Adrian Bunk
2007-08-01 15:40 ` Miller, Mike (OS Dev)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox