linux-kernel - Re: [PATCH] nvme: do not call del_gendisk() on a disk that was never added

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: <e7a92bb5-6400-34cf-393c-489ae6c6f072@mellanox.com>
Date:   Sun, 7 Jun 2020 16:13:39 +0300
From:   Max Gurtovoy <maxg@...lanox.com>
To:     Niklas Cassel <niklas.cassel@....com>,
        Keith Busch <kbusch@...nel.org>, Jens Axboe <axboe@...com>,
        Christoph Hellwig <hch@....de>,
        Sagi Grimberg <sagi@...mberg.me>,
        James Smart <james.smart@...adcom.com>,
        Israel Rukshin <israelr@...lanox.com>
Cc:     linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] nvme: do not call del_gendisk() on a disk that was never
 added


On 6/7/2020 2:45 PM, Niklas Cassel wrote:
> device_add_disk() is negated by del_gendisk().
> alloc_disk_node() is negated by put_disk().
>
> In nvme_alloc_ns(), device_add_disk() is one of the last things being
> called in the success case, and only void functions are being called
> after this. Therefore this call should not be negated in the error path.
>
> The superfluous call to del_gendisk() leads to the following prints:
> [    7.839975] kobject: '(null)' (000000001ff73734): is not initialized, yet kobject_put() is being called.
> [    7.840865] WARNING: CPU: 2 PID: 361 at lib/kobject.c:736 kobject_put+0x70/0x120
>
> Fixes: 33cfdc2aa696 ("nvme: enforce extended LBA format for fabrics metadata")
> Signed-off-by: Niklas Cassel <niklas.cassel@....com>
> ---
> An alternative would be to do like nvme_ns_remove(), i.e. in the error
> path; check if ns->disk->flags & GENHD_FL_UP is set, and only then call
> del_gendisk(). However, that seems unnecessary, since as nvme_alloc_ns()
> is currently written, we know that device_add_disk() does not need to be
> negated.
>
>   drivers/nvme/host/core.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)

Looks good,

Reviewed-by: Max Gurtovoy <maxg@...lanox.com>