linux-kernel - Re: [PATCH] NVMe: Fix error handling of class

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1426015102.18746.10.camel@jpfreyen-mobl5.amr.corp.intel.com>
Date:	Tue, 10 Mar 2015 12:18:22 -0700
From:	J Freyensee <james_p_freyensee@...ux.intel.com>
To:	Alexey Khoroshilov <khoroshilov@...ras.ru>
Cc:	Matthew Wilcox <willy@...ux.intel.com>,
	Keith Busch <keith.busch@...el.com>,
	ldv-project@...uxtesting.org, linux-nvme@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] NVMe: Fix error handling of class_create("nvme")

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@...ras.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)


--
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/