All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] NVMe: Fix error handling of class_create("nvme")
@ 2015-03-06 22:43 ` Alexey Khoroshilov
  0 siblings, 0 replies; 10+ messages in thread
From: Alexey Khoroshilov @ 2015-03-06 22:43 UTC (permalink / raw)
  To: Matthew Wilcox, Keith Busch
  Cc: Alexey Khoroshilov, linux-nvme, linux-kernel, ldv-project

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@ispras.ru>
---
 drivers/block/nvme-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index ceb32dd52a6c..30ac50b3009d 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -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)
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH] NVMe: Fix error handling of class_create("nvme")
@ 2015-03-06 22:43 ` Alexey Khoroshilov
  0 siblings, 0 replies; 10+ messages in thread
From: Alexey Khoroshilov @ 2015-03-06 22:43 UTC (permalink / raw)


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(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index ceb32dd52a6c..30ac50b3009d 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -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)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] NVMe: Fix error handling of class_create("nvme")
  2015-03-06 22:43 ` Alexey Khoroshilov
@ 2015-03-10 19:18   ` J Freyensee
  -1 siblings, 0 replies; 10+ messages in thread
From: J Freyensee @ 2015-03-10 19:18 UTC (permalink / raw)
  To: Alexey Khoroshilov
  Cc: Matthew Wilcox, Keith Busch, ldv-project, linux-nvme, linux-kernel

On Sat, 2015-03-07 at 01:43 +0300, Alexey Khoroshilov wrote:
> 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@ispras.ru>
> ---
>  drivers/block/nvme-core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
> index ceb32dd52a6c..30ac50b3009d 100644
> --- a/drivers/block/nvme-core.c
> +++ b/drivers/block/nvme-core.c
> @@ -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.

> +		result = PTR_ERR(nvme_class);
>  		goto unregister_chrdev;
> +	}
>  
>  	result = pci_register_driver(&nvme_driver);
>  	if (result)



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] NVMe: Fix error handling of class_create("nvme")
@ 2015-03-10 19:18   ` J Freyensee
  0 siblings, 0 replies; 10+ messages in thread
From: J Freyensee @ 2015-03-10 19:18 UTC (permalink / raw)


On Sat, 2015-03-07@01:43 +0300, Alexey Khoroshilov wrote:
> 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(-)
> 
> diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
> index ceb32dd52a6c..30ac50b3009d 100644
> --- a/drivers/block/nvme-core.c
> +++ b/drivers/block/nvme-core.c
> @@ -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.

> +		result = PTR_ERR(nvme_class);
>  		goto unregister_chrdev;
> +	}
>  
>  	result = pci_register_driver(&nvme_driver);
>  	if (result)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] NVMe: Fix error handling of class_create("nvme")
  2015-03-10 19:18   ` J Freyensee
@ 2015-03-10 19:46     ` Matthew Wilcox
  -1 siblings, 0 replies; 10+ messages in thread
From: Matthew Wilcox @ 2015-03-10 19:46 UTC (permalink / raw)
  To: J Freyensee
  Cc: Alexey Khoroshilov, Matthew Wilcox, Keith Busch, ldv-project,
	linux-nvme, linux-kernel

On Tue, Mar 10, 2015 at 12:18:22PM -0700, J Freyensee wrote:
> 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] NVMe: Fix error handling of class_create("nvme")
@ 2015-03-10 19:46     ` Matthew Wilcox
  0 siblings, 0 replies; 10+ messages in thread
From: Matthew Wilcox @ 2015-03-10 19:46 UTC (permalink / raw)


On Tue, Mar 10, 2015@12:18:22PM -0700, J Freyensee wrote:
> 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] NVMe: Fix error handling of class_create("nvme")
  2015-03-06 22:43 ` Alexey Khoroshilov
@ 2015-03-16 17:22   ` Keith Busch
  -1 siblings, 0 replies; 10+ messages in thread
From: Keith Busch @ 2015-03-16 17:22 UTC (permalink / raw)
  To: Alexey Khoroshilov
  Cc: Matthew Wilcox, Keith Busch, linux-nvme, linux-kernel, ldv-project

On Fri, 6 Mar 2015, Alexey Khoroshilov wrote:
> 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@ispras.ru>

Thanks for the fix.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] NVMe: Fix error handling of class_create("nvme")
@ 2015-03-16 17:22   ` Keith Busch
  0 siblings, 0 replies; 10+ messages in thread
From: Keith Busch @ 2015-03-16 17:22 UTC (permalink / raw)


On Fri, 6 Mar 2015, Alexey Khoroshilov wrote:
> 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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] NVMe: Fix error handling of class_create("nvme")
  2015-03-06 22:43 ` Alexey Khoroshilov
                   ` (2 preceding siblings ...)
  (?)
@ 2015-04-07 23:03 ` Keith Busch
  2015-04-08  1:09   ` Jens Axboe
  -1 siblings, 1 reply; 10+ messages in thread
From: Keith Busch @ 2015-04-07 23:03 UTC (permalink / raw)


On Fri, 6 Mar 2015, Alexey Khoroshilov wrote:
> 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>

> ---
> drivers/block/nvme-core.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
> index ceb32dd52a6c..30ac50b3009d 100644
> --- a/drivers/block/nvme-core.c
> +++ b/drivers/block/nvme-core.c
> @@ -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)
> -- 
> 1.9.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] NVMe: Fix error handling of class_create("nvme")
  2015-04-07 23:03 ` Keith Busch
@ 2015-04-08  1:09   ` Jens Axboe
  0 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2015-04-08  1:09 UTC (permalink / raw)


On 04/07/2015 05:03 PM, Keith Busch wrote:
> On Fri, 6 Mar 2015, Alexey Khoroshilov wrote:
>> 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!

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-04-08  1:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-06 22:43 [PATCH] NVMe: Fix error handling of class_create("nvme") Alexey Khoroshilov
2015-03-06 22:43 ` Alexey Khoroshilov
2015-03-10 19:18 ` J Freyensee
2015-03-10 19:18   ` J Freyensee
2015-03-10 19:46   ` Matthew Wilcox
2015-03-10 19:46     ` Matthew Wilcox
2015-03-16 17:22 ` Keith Busch
2015-03-16 17:22   ` Keith Busch
2015-04-07 23:03 ` Keith Busch
2015-04-08  1:09   ` Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.