[Qemu-devel] [PULL 0/1] NBD patches for -rc2

[Qemu-devel] [PULL 0/1] NBD patches for -rc2

[Eric Blake]

The following changes since commit e2b47666fe1544959c89bd3ed159e9e37cc9fc73:

  Merge remote-tracking branch 'remotes/berrange/tags/misc-next-pull-request' into staging (2019-07-19 14:29:13 +0100)

are available in the Git repository at:

  https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2019-07-19

for you to fetch changes up to 5cf42b1c1f75499b467701926d3c9691d27712e1:

  nbd: Initialize reply on failure (2019-07-19 13:19:18 -0500)

----------------------------------------------------------------
nbd patches for 2019-07-19

- silence harmless compiler/valgrind warning

----------------------------------------------------------------
Eric Blake (1):
      nbd: Initialize reply on failure

 block/nbd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

-- 
2.20.1

[Qemu-devel] [PULL 1/1] nbd: Initialize reply on failure

[Eric Blake]

We've had two separate reports of different callers 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, and whether there are any other
uninitialized uses, it's easier to guarantee that reply will always be
set by nbd_co_receive_one_chunk() even on failure.

The uninitialized use happens to be harmless (the only time the
variable is uninitialized is if s->quit is set, so the conditional
results in the same action regardless of what was read from reply),
and was introduced in commit 65e01d47.

In fixing the problem, it can also be seen that all (one) callers pass
in a non-NULL reply, so there is a dead conditional to also be cleaned
up.

Reported-by: Thomas Huth <thuth@redhat.com>
Reported-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190719172001.19770-1-eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 block/nbd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 81edabbf35ed..57c1a205811a 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -640,12 +640,11 @@ 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;
-        }
+        *reply = s->reply;
         s->reply.handle = 0;
     }

-- 
2.20.1

Re: [Qemu-devel] [PULL 0/1] NBD patches for -rc2

[Peter Maydell]

On Fri, 19 Jul 2019 at 21:21, Eric Blake <eblake@redhat.com> wrote:
>
> The following changes since commit e2b47666fe1544959c89bd3ed159e9e37cc9fc73:
>
>   Merge remote-tracking branch 'remotes/berrange/tags/misc-next-pull-request' into staging (2019-07-19 14:29:13 +0100)
>
> are available in the Git repository at:
>
>   https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2019-07-19
>
> for you to fetch changes up to 5cf42b1c1f75499b467701926d3c9691d27712e1:
>
>   nbd: Initialize reply on failure (2019-07-19 13:19:18 -0500)
>
> ----------------------------------------------------------------
> nbd patches for 2019-07-19
>
> - silence harmless compiler/valgrind warning
>
> ----------------------------------------------------------------
> Eric Blake (1):
>       nbd: Initialize reply on failure
>
>  block/nbd.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM