Re: [Qemu-block] [PATCH v4 7/7] block/nbd: NBDReply is used being uninitialized

[Eric Blake]

On 7/19/19 9:34 AM, Eric Blake wrote:
> On 7/19/19 4:40 AM, Andrey Shinkevich wrote:
>> In case nbd_co_receive_one_chunk() fails in
>> nbd_reply_chunk_iter_receive(), 'NBDReply reply' parameter is used in
>> the check nbd_reply_is_simple() without being initialized. The iotest
>> 083 does not pass under the Valgrind: $./check -nbd -valgrind 083.
>> The alternative solution is to swap the operands in the condition:
>> 'if (s->quit || nbd_reply_is_simple(reply))'
>>
>> Signed-off-by: Andrey Shinkevich <address@hidden>
>> ---
>>  block/nbd.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Huh. Very similar to
> https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg03712.html, but
> affects a different function. I can queue this one through my NBD tree
> to get both in my rc2 pull request.
> 
> Reviewed-by: Eric Blake <address@hidden>

Actually, since this is the second patch on the same topic, I'm
wondering if it's better to use the following one-liner to fix BOTH
issues and without relying on a gcc extension:

diff --git i/block/nbd.c w/block/nbd.c
index 8d565cc624ec..f751a8e633e5 100644
--- i/block/nbd.c
+++ w/block/nbd.c
@@ -640,6 +640,7 @@ static coroutine_fn int nbd_co_receive_one_chunk(
                                           request_ret, qiov, payload,
errp);

     if (ret < 0) {
+        memset(reply, 0, sizeof *reply);
         s->quit = true;
     } else {
         /* For assert at loop start in nbd_connection_entry */

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature