[Qemu-commits] [qemu/qemu] 71eaec: block: Avoid memleak on qcow2 image info failure

[Peter Maydell]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 71eaec2e8c7c8d266137b5c5f42da0bd6d6b5eb7
      
https://github.com/qemu/qemu/commit/71eaec2e8c7c8d266137b5c5f42da0bd6d6b5eb7
  Author: Eric Blake <address@hidden>
  Date:   2020-03-24 (Tue, 24 Mar 2020)

  Changed paths:
    M block/qcow2.c

  Log Message:
  -----------
  block: Avoid memleak on qcow2 image info failure

If we fail to get bitmap info, we must not leak the encryption info.

Fixes: b8968c875f403
Fixes: Coverity CID 1421894
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: Andrey Shinkevich <address@hidden>
Tested-by: Andrey Shinkevich <address@hidden>
Signed-off-by: Max Reitz <address@hidden>


  Commit: a15f08dceebce63ee15c91c7d74265d61d882ae5
      
https://github.com/qemu/qemu/commit/a15f08dceebce63ee15c91c7d74265d61d882ae5
  Author: Philippe Mathieu-Daudé <address@hidden>
  Date:   2020-03-24 (Tue, 24 Mar 2020)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: Assert BlockDriver::format_name is not NULL

bdrv_do_find_format() calls strcmp() using BlockDriver::format_name
as argument, which must not be NULL. Assert this field is not null
when we register a block driver in bdrv_register().

Reported-by: Mansour Ahmadi <address@hidden>
Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Alberto Garcia <address@hidden>
Signed-off-by: Max Reitz <address@hidden>


  Commit: 6e57963a77df1e275a73dab4c6a7ec9a9d3468d4
      
https://github.com/qemu/qemu/commit/6e57963a77df1e275a73dab4c6a7ec9a9d3468d4
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2020-03-24 (Tue, 24 Mar 2020)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: bdrv_set_backing_bs: fix use-after-free

There is a use-after-free possible: bdrv_unref_child() leaves
bs->backing freed but not NULL. bdrv_attach_child may produce nested
polling loop due to drain, than access of freed pointer is possible.

I've produced the following crash on 30 iotest with modified code. It
does not reproduce on master, but still seems possible:

    #0  __strcmp_avx2 () at /lib64/libc.so.6
    #1  bdrv_backing_overridden (bs=0x55c9d3cc2060) at block.c:6350
    #2  bdrv_refresh_filename (bs=0x55c9d3cc2060) at block.c:6404
    #3  bdrv_backing_attach (c=0x55c9d48e5520) at block.c:1063
    #4  bdrv_replace_child_noperm
        (child=child@entry=0x55c9d48e5520,
        new_bs=new_bs@entry=0x55c9d3cc2060) at block.c:2290
    #5  bdrv_replace_child
        (child=child@entry=0x55c9d48e5520,
        new_bs=new_bs@entry=0x55c9d3cc2060) at block.c:2320
    #6  bdrv_root_attach_child
        (child_bs=child_bs@entry=0x55c9d3cc2060,
        child_name=child_name@entry=0x55c9d241d478 "backing",
        child_role=child_role@entry=0x55c9d26ecee0 <child_backing>,
        ctx=<optimized out>, perm=<optimized out>, shared_perm=21,
        opaque=0x55c9d3c5a3d0, errp=0x7ffd117108e0) at block.c:2424
    #7  bdrv_attach_child
        (parent_bs=parent_bs@entry=0x55c9d3c5a3d0,
        child_bs=child_bs@entry=0x55c9d3cc2060,
        child_name=child_name@entry=0x55c9d241d478 "backing",
        child_role=child_role@entry=0x55c9d26ecee0 <child_backing>,
        errp=errp@entry=0x7ffd117108e0) at block.c:5876
    #8  in bdrv_set_backing_hd
        (bs=bs@entry=0x55c9d3c5a3d0,
        backing_hd=backing_hd@entry=0x55c9d3cc2060,
        errp=errp@entry=0x7ffd117108e0)
        at block.c:2576
    #9  stream_prepare (job=0x55c9d49d84a0) at block/stream.c:150
    #10 job_prepare (job=0x55c9d49d84a0) at job.c:761
    #11 job_txn_apply (txn=<optimized out>, fn=<optimized out>) at
        job.c:145
    #12 job_do_finalize (job=0x55c9d49d84a0) at job.c:778
    #13 job_completed_txn_success (job=0x55c9d49d84a0) at job.c:832
    #14 job_completed (job=0x55c9d49d84a0) at job.c:845
    #15 job_completed (job=0x55c9d49d84a0) at job.c:836
    #16 job_exit (opaque=0x55c9d49d84a0) at job.c:864
    #17 aio_bh_call (bh=0x55c9d471a160) at util/async.c:117
    #18 aio_bh_poll (ctx=ctx@entry=0x55c9d3c46720) at util/async.c:117
    #19 aio_poll (ctx=ctx@entry=0x55c9d3c46720,
        blocking=blocking@entry=true)
        at util/aio-posix.c:728
    #20 bdrv_parent_drained_begin_single (poll=true, c=0x55c9d3d558f0)
        at block/io.c:121
    #21 bdrv_parent_drained_begin_single (c=c@entry=0x55c9d3d558f0,
        poll=poll@entry=true)
        at block/io.c:114
    #22 bdrv_replace_child_noperm
        (child=child@entry=0x55c9d3d558f0,
        new_bs=new_bs@entry=0x55c9d3d27300) at block.c:2258
    #23 bdrv_replace_child
        (child=child@entry=0x55c9d3d558f0,
        new_bs=new_bs@entry=0x55c9d3d27300) at block.c:2320
    #24 bdrv_root_attach_child
        (child_bs=child_bs@entry=0x55c9d3d27300,
        child_name=child_name@entry=0x55c9d241d478 "backing",
        child_role=child_role@entry=0x55c9d26ecee0 <child_backing>,
        ctx=<optimized out>, perm=<optimized out>, shared_perm=21,
        opaque=0x55c9d3cc2060, errp=0x7ffd11710c60) at block.c:2424
    #25 bdrv_attach_child
        (parent_bs=parent_bs@entry=0x55c9d3cc2060,
        child_bs=child_bs@entry=0x55c9d3d27300,
        child_name=child_name@entry=0x55c9d241d478 "backing",
        child_role=child_role@entry=0x55c9d26ecee0 <child_backing>,
        errp=errp@entry=0x7ffd11710c60) at block.c:5876
    #26 bdrv_set_backing_hd
        (bs=bs@entry=0x55c9d3cc2060,
        backing_hd=backing_hd@entry=0x55c9d3d27300,
        errp=errp@entry=0x7ffd11710c60)
        at block.c:2576
    #27 stream_prepare (job=0x55c9d495ead0) at block/stream.c:150
    ...

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
Reviewed-by: John Snow <address@hidden>
Signed-off-by: Max Reitz <address@hidden>


  Commit: 808cf3cb6af8171b4e24d24f2a2d461434dc6572
      
https://github.com/qemu/qemu/commit/808cf3cb6af8171b4e24d24f2a2d461434dc6572
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2020-03-24 (Tue, 24 Mar 2020)

  Changed paths:
    M block/qcow2.c

  Log Message:
  -----------
  block/qcow2: zero data_file child after free

data_file being NULL doesn't seem to be a correct state, but it's
better than dead pointer and simpler to debug.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: John Snow <address@hidden>
Signed-off-by: Max Reitz <address@hidden>


  Commit: 801ddbda7183e1e043015fd357ea5eb97d925fd2
      
https://github.com/qemu/qemu/commit/801ddbda7183e1e043015fd357ea5eb97d925fd2
  Author: Max Reitz <address@hidden>
  Date:   2020-03-24 (Tue, 24 Mar 2020)

  Changed paths:
    M tests/qemu-iotests/085
    M tests/qemu-iotests/087
    M tests/qemu-iotests/279

  Log Message:
  -----------
  iotests: Fix cleanup path in some tests

Some iotests leave behind some external data file when run for qcow2
with -o data_file.  Fix that.

Signed-off-by: Max Reitz <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Max Reitz <address@hidden>


  Commit: c264e5d2f9f5d73977eac8e5d084f727b3d07ea9
      
https://github.com/qemu/qemu/commit/c264e5d2f9f5d73977eac8e5d084f727b3d07ea9
  Author: Max Reitz <address@hidden>
  Date:   2020-03-24 (Tue, 24 Mar 2020)

  Changed paths:
    M tests/qemu-iotests/026
    M tests/qemu-iotests/026.out
    M tests/qemu-iotests/026.out.nocache
    A tests/qemu-iotests/289
    A tests/qemu-iotests/289.out
    M tests/qemu-iotests/group

  Log Message:
  -----------
  iotests/026: Move v3-exclusive test to new file

data_file does not work with v2, and we probably want 026 to keep
working for v2 images.  Thus, open a new file for v3-exclusive error
path test cases.

Fixes: 81311255f217859413c94f2cd9cebf2684bbda94
       (“iotests/026: Test EIO on allocation in a data-file”)
Signed-off-by: Max Reitz <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: John Snow <address@hidden>
Tested-by: John Snow <address@hidden>
Signed-off-by: Max Reitz <address@hidden>


  Commit: 62a43e53faed67a5aa4bfededca24c9079de9720
      
https://github.com/qemu/qemu/commit/62a43e53faed67a5aa4bfededca24c9079de9720
  Author: Peter Maydell <address@hidden>
  Date:   2020-03-24 (Tue, 24 Mar 2020)

  Changed paths:
    M block.c
    M block/qcow2.c
    M tests/qemu-iotests/026
    M tests/qemu-iotests/026.out
    M tests/qemu-iotests/026.out.nocache
    M tests/qemu-iotests/085
    M tests/qemu-iotests/087
    M tests/qemu-iotests/279
    A tests/qemu-iotests/289
    A tests/qemu-iotests/289.out
    M tests/qemu-iotests/group

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-03-24' 
into staging

Block patches for 5.0-rc0:
- Use-after-free fix
- Fix for a memleak in an error path
- Preventative measures against other potential use-after-frees, and
  against NULL deferences at runtime
- iotest fixes

# gpg: Signature made Tue 24 Mar 2020 12:19:09 GMT
# gpg:                using RSA key 91BEB60A30DB3E8857D11829F407DB0061D5CF40
# gpg:                issuer "address@hidden"
# gpg: Good signature from "Max Reitz <address@hidden>" [full]
# Primary key fingerprint: 91BE B60A 30DB 3E88 57D1  1829 F407 DB00 61D5 CF40

* remotes/maxreitz/tags/pull-block-2020-03-24:
  iotests/026: Move v3-exclusive test to new file
  iotests: Fix cleanup path in some tests
  block/qcow2: zero data_file child after free
  block: bdrv_set_backing_bs: fix use-after-free
  block: Assert BlockDriver::format_name is not NULL
  block: Avoid memleak on qcow2 image info failure

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/09a98dd988c7...62a43e53faed