[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH] nbd: Initialize reply on failure

From: Eric Blake
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH] nbd: Initialize reply on failure
Date: Fri, 19 Jul 2019 12:15:47 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0

On 7/19/19 10:03 AM, Eric Blake wrote:
> We've had two separate reports of a caller running into use of
> uninitialized data if s->quit is set (one detected by gcc -O3, another
> by valgrind), due to checking 'nbd_reply_is_simple(reply) || s->quit'
> in the wrong order. Rather than chasing down which callers need to
> pre-initialize reply, it's easier to guarantee that reply will always
> be set by nbd_co_receive_one_chunk() even on failure.
> Reported-by: Thomas Huth <address@hidden>
> Reported-by: Andrey Shinkevich <address@hidden>
> Signed-off-by: Eric Blake <address@hidden>
> ---

Blech. Needs a v2.  Expanding context:

> +++ b/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 */
        if (reply) {
            *reply = s->reply;

either callers can pass in reply==NULL (in which case the memset()
dereferences NULL, oops), or always pass in non-NULL reply (in which
case the null check is dead code).

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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]