Skip to contentSkip to conversation

target: checking for NULL instead of IS_ERR

PATCH In review Dan Carpenter 2 participants 2 messages Jan 15, 2011

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 target: checking for NULL instead of IS_ERR

blkdev_get_by_path() returns an ERR_PTR() or error and it doesn't return a NULL. It looks like this bug would be easy to trigger by mistake.

drivers/target/target_core_iblock.c +1 -1
index c6e0d75..3456135 100644
@@ -154,7 +154,7 @@ static struct se_device *iblock_create_virtdevice(
 
 	bd = blkdev_get_by_path(ib_dev->ibd_udev_path,
 				FMODE_WRITE|FMODE_READ|FMODE_EXCL, ib_dev);
-	if (!(bd))
+	if (IS_ERR(bd))
 		goto failed;
 	/*
 	 * Setup the local scope queue_limits from struct request_queue->limits
#1 lore raw
Quoted 5 lines · Dan Carpenter

blkdev_get_by_path() returns an ERR_PTR() or error and it doesn't return a NULL. It looks like this bug would be easy to trigger by mistake.

Good catch here Dan.. This was added after a linux-next build failure over the holidays, but I completely missed the fact that it returns ERR_PTR.. Thanks for spotting this one!

The same problem exists in target_core_pscsi.c:pscsi_create_type_disk() as well.. Committed both fixes as 862ede4cbaf in lio-core-2.6.git/linus-38-rc1 against the .38 mainline target merge, and cherry picked into scsi-post-merge-2.6.git/for-jejb..

James, please make sure this fix is included in your next pull to Linus.

--nab

Quoted 13 lines
drivers/target/target_core_iblock.c +1 -1
index c6e0d75..3456135 100644
@@ -154,7 +154,7 @@ static struct se_device *iblock_create_virtdevice(
 
 	bd = blkdev_get_by_path(ib_dev->ibd_udev_path,
 				FMODE_WRITE|FMODE_READ|FMODE_EXCL, ib_dev);
-	if (!(bd))
+	if (IS_ERR(bd))
 		goto failed;
 	/*
 	 * Setup the local scope queue_limits from struct request_queue->limits