Skip to contentSkip to conversation

NVMe: Fix error handling of class_create("nvme")

PATCH In review Alexey Khoroshilov 5 participants 6 messages Mar 6, 2015 – Apr 8, 2015

Subsystems (2)
Block layer NVMe
lore
Share & tools
lore

Alexey Khoroshilov

author
#0 lore raw

PATCH 1/1 NVMe: Fix error handling of class_create("nvme")

class_create() returns ERR_PTR on failure,
so IS_ERR() should be used instead of check for NULL.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by Alexey Khoroshilov <khoroshilov at ispras.ru>
---
 drivers/block/nvme-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/block/nvme-core.c +3 -1
index ceb32dd52a6c..30ac50b3009d 100644
@@ -3165,8 +3165,10 @@ static int __init nvme_init(void)
 		nvme_char_major = result;
 
 	nvme_class = class_create(THIS_MODULE, "nvme");
-	if (!nvme_class)
+	if (IS_ERR(nvme_class)) {
+		result = PTR_ERR(nvme_class);
 		goto unregister_chrdev;
+	}
 
 	result = pci_register_driver(&nvme_driver);
 	if (result)
Signature · 1 line
1.9.1
#1 lore raw
Quoted 15 lines · Alexey Khoroshilov

class_create() returns ERR_PTR on failure,
so IS_ERR() should be used instead of check for NULL.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by Alexey Khoroshilov <khoroshilov at ispras.ru>
---
 drivers/block/nvme-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/block/nvme-core.c +0 -0
index ceb32dd52a6c..30ac50b3009d 100644

drivers/block/nvme-core.c static int __init nvme_init(void) · line 3168

Quoted 5 lines
drivers/block/nvme-core.c +1 -1
@@ -3165,8 +3165,10 @@ static int __init nvme_init(void)
 		nvme_char_major = result;
 
 	nvme_class = class_create(THIS_MODULE, "nvme");
-	if (!nvme_class)
+	if (IS_ERR(nvme_class)) {

Looking at IS_ERR(), it uses IS_ERR_VALUE() which uses unlikely(), which from what I understand is a compiler hint that means "this error is unlikely to happen".

Well, is this error unlikely to happen? Looks like the error could be caused by kzalloc() failing, which could more likely happen in a low-cost Chromebook OS laptop (nvme boot driver just got accepted into the OS). Or a number of other things going wrong within the __class_register() call.

Without understanding the code in fine-print detail, it seems like how 'nvme_class' condition is checked can be a bit arbitrary.

Quoted 6 lines
drivers/block/nvme-core.c +2 -0
+		result = PTR_ERR(nvme_class);
 		goto unregister_chrdev;
+	}
 
 	result = pci_register_driver(&nvme_driver);
 	if (result)
#2 lore raw
Quoted 5 lines · J Freyensee

Looking at IS_ERR(), it uses IS_ERR_VALUE() which uses unlikely(), which from what I understand is a compiler hint that means "this error is unlikely to happen".

Well, is this error unlikely to happen? Looks like the error could be

Yes. All errors are, by definition, unlikely to happen.

Acked-by on v1 1/1
#3 lore raw
In reply to Alexey Khoroshilov
Quoted 6 lines · Alexey Khoroshilov

class_create() returns ERR_PTR on failure,
so IS_ERR() should be used instead of check for NULL.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by Alexey Khoroshilov <khoroshilov at ispras.ru>

Thanks for the fix.

Acked-by Keith Busch <keith.busch at intel.com>
#4 lore raw

PATCH 1/1 NVMe: Fix error handling of class_create("nvme")

In reply to Alexey Khoroshilov
Quoted 6 lines · Alexey Khoroshilov

class_create() returns ERR_PTR on failure,
so IS_ERR() should be used instead of check for NULL.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by Alexey Khoroshilov <khoroshilov at ispras.ru>

Just resending ack for 4.1.

Acked-by Keith Busch <keith.busch at intel.com>
Quoted 22 lines

---
drivers/block/nvme-core.c | 4 +++-

1 file changed, 3 insertions(+), 1 deletion(-)
drivers/block/nvme-core.c +3 -1
index ceb32dd52a6c..30ac50b3009d 100644
@@ -3165,8 +3165,10 @@ static int __init nvme_init(void)
 		nvme_char_major = result;
 
 	nvme_class = class_create(THIS_MODULE, "nvme");
-	if (!nvme_class)
+	if (IS_ERR(nvme_class)) {
+		result = PTR_ERR(nvme_class);
 		goto unregister_chrdev;
+	}
 
 	result = pci_register_driver(&nvme_driver);
 	if (result)
Signature · 1 line
1.9.1
maintainer
#5 lore raw

PATCH 1/1 NVMe: Fix error handling of class_create("nvme")

Quoted 11 lines · Keith Busch
Quoted 6 lines · Alexey Khoroshilov

class_create() returns ERR_PTR on failure,
so IS_ERR() should be used instead of check for NULL.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by Alexey Khoroshilov <khoroshilov at ispras.ru>

Just resending ack for 4.1.

Acked-by Keith Busch <keith.busch at intel.com>

Thanks, I missed this one. Applied to for-4.1/drivers, thanks!

Signature · 1 line
Jens Axboe