[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH] block/nvme: fix Coverity reports
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-block] [PATCH] block/nvme: fix Coverity reports |
Date: |
Fri, 9 Feb 2018 17:17:08 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
On 09/02/2018 17:16, Kevin Wolf wrote:
> Am 09.02.2018 um 16:24 hat Paolo Bonzini geschrieben:
>> 1) string not null terminated in sysfs_find_group_file
>>
>> 2) NULL pointer dereference and dead local variable in nvme_init.
>>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>> block/nvme.c | 14 ++++++--------
>> util/vfio-helpers.c | 2 +-
>> 2 files changed, 7 insertions(+), 9 deletions(-)
>>
>> diff --git a/block/nvme.c b/block/nvme.c
>> index e9d0e218fc..ce217ffc81 100644
>> --- a/block/nvme.c
>> +++ b/block/nvme.c
>> @@ -553,7 +553,6 @@ static int nvme_init(BlockDriverState *bs, const char
>> *device, int namespace,
>> uint64_t cap;
>> uint64_t timeout_ms;
>> uint64_t deadline, now;
>> - Error *local_err = NULL;
>>
>> qemu_co_mutex_init(&s->dma_map_lock);
>> qemu_co_queue_init(&s->dma_flush_queue);
>> @@ -645,11 +644,6 @@ static int nvme_init(BlockDriverState *bs, const char
>> *device, int namespace,
>> false, nvme_handle_event, nvme_poll_cb);
>>
>> nvme_identify(bs, namespace, errp);
>> - if (local_err) {
>> - error_propagate(errp, local_err);
>> - ret = -EIO;
>> - goto fail_handler;
>> - }
>>
>> /* Set up command queues. */
>> if (!nvme_add_io_queue(bs, errp)) {
>
> I don't think this is right, errp must not be assigned twice, and you
> don't want to return 0 when an error is set. Even if we were return the
> right return value and errp content, it would be rather surprising to
> have an error set without jumping to an error label.
>
> So we should either pass &local_err to nvme_identify() or make it return
> a success flag so we can run a proper error path.
You're right, that was dumb. :(
Paolo