[Qemu-commits] [qemu/qemu] 5e3c02: nvme: fix oob access issue(CVE-2018-16847)

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 5e3c0220d7e4f0361c4d36c697a8842f2b583402
      
https://github.com/qemu/qemu/commit/5e3c0220d7e4f0361c4d36c697a8842f2b583402
  Author: Li Qiang <address@hidden>
  Date:   2018-11-19 (Mon, 19 Nov 2018)

  Changed paths:
    M hw/block/nvme.c

  Log Message:
  -----------
  nvme: fix oob access issue(CVE-2018-16847)

Currently, the nvme_cmb_ops mr doesn't check the addr and size.
This can lead an oob access issue. This is triggerable in the guest.
Add check to avoid this issue.

Fixes CVE-2018-16847.

Reported-by: Li Qiang <address@hidden>
Reviewed-by: Paolo Bonzini <address@hidden>
Signed-off-by: Li Qiang <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 441f6692ecc14859b77af2ac6d8f55e6f1354d3b
      
https://github.com/qemu/qemu/commit/441f6692ecc14859b77af2ac6d8f55e6f1354d3b
  Author: Mark Cave-Ayland <address@hidden>
  Date:   2018-11-19 (Mon, 19 Nov 2018)

  Changed paths:
    M hw/block/fdc.c

  Log Message:
  -----------
  fdc: fix segfault in fdctrl_stop_transfer() when DMA is disabled

Commit c8a35f1cf0f "fdc: use IsaDma interface instead of global DMA_*
functions" accidentally introduced a segfault in fdctrl_stop_transfer() for
non-DMA transfers.

If fdctrl->dma_chann has not been configured then the fdctrl->dma interface
reference isn't initialised during isabus_fdc_realize(). Unfortunately
fdctrl_stop_transfer() unconditionally references the DMA interface when
finishing the transfer causing a NULL pointer dereference.

Fix the issue by adding a check in fdctrl_stop_transfer() so that the DMA
interface reference and release method is only invoked if fdctrl->dma_chann
has been set.

(This issue was discovered by Martin testing a recent change in the NetBSD
installer under qemu-system-sparc)

Cc: address@hidden
Reported-by: Martin Husemann <address@hidden>
Signed-off-by: Mark Cave-Ayland <address@hidden>
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
Reviewed-by: Hervé Poussineau <address@hidden>
Reviewed-by: John Snow <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 443ba6befa2f47243def9b190c930a8a4f59f888
      
https://github.com/qemu/qemu/commit/443ba6befa2f47243def9b190c930a8a4f59f888
  Author: Kevin Wolf <address@hidden>
  Date:   2018-11-19 (Mon, 19 Nov 2018)

  Changed paths:
    M block/vvfat.c

  Log Message:
  -----------
  vvfat: Fix memory leak

Don't leak 'cluster' in the mapping == NULL case. Found by Coverity
(CID 1055918).

Fixes: 8d9401c2791ee2d2805b741b1ee3006041edcd3e
Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
Reviewed-by: Liam Merwick <address@hidden>
Tested-by: Philippe Mathieu-Daudé <address@hidden>


  Commit: d3e1a7eb4ceb9489d575c45c9518137dfbd1389d
      
https://github.com/qemu/qemu/commit/d3e1a7eb4ceb9489d575c45c9518137dfbd1389d
  Author: Eric Blake <address@hidden>
  Date:   2018-11-19 (Mon, 19 Nov 2018)

  Changed paths:
    M docs/interop/qcow2.txt

  Log Message:
  -----------
  qcow2: Document some maximum size constraints

Although off_t permits up to 63 bits (8EB) of file offsets, in
practice, we're going to hit other limits first.  Document some
of those limits in the qcow2 spec (some are inherent, others are
implementation choices of qemu), and how choice of cluster size
can influence some of the limits.

While we cannot map any uncompressed virtual cluster to any
address higher than 64 PB (56 bits) (due to the current L1/L2
field encoding stopping at bit 55), qemu's cap of 8M for the
refcount table can still access larger host addresses for some
combinations of large clusters and small refcount_order.  For
comparison, ext4 with 4k blocks caps files at 16PB.

Another interesting limit: for compressed clusters, the L2 layout
requires an ever-smaller maximum host offset as cluster size gets
larger, down to a 512 TB maximum with 2M clusters.  In particular,
note that with a cluster size of 8k or smaller, the L2 entry for
a compressed cluster could technically point beyond the 64PB mark,
but when you consider that with 8k clusters and refcount_order = 0,
you cannot access beyond 512T without exceeding qemu's limit of an
8M cap on the refcount table, it is unlikely that any image in the
wild has attempted to do so.  To be safe, let's document that bits
beyond 55 in a compressed cluster must be 0.

Signed-off-by: Eric Blake <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 77d6a21558577fbdd35e65e0e1d03ae07214329f
      
https://github.com/qemu/qemu/commit/77d6a21558577fbdd35e65e0e1d03ae07214329f
  Author: Eric Blake <address@hidden>
  Date:   2018-11-19 (Mon, 19 Nov 2018)

  Changed paths:
    M block/qcow2-refcount.c
    M block/qcow2.h

  Log Message:
  -----------
  qcow2: Don't allow overflow during cluster allocation

Our code was already checking that we did not attempt to
allocate more clusters than what would fit in an INT64 (the
physical maximimum if we can access a full off_t's worth of
data).  But this does not catch smaller limits enforced by
various spots in the qcow2 image description: L1 and normal
clusters of L2 are documented as having bits 63-56 reserved
for other purposes, capping our maximum offset at 64PB (bit
55 is the maximum bit set).  And for compressed images with
2M clusters, the cap drops the maximum offset to bit 48, or
a maximum offset of 512TB.  If we overflow that offset, we
would write compressed data into one place, but try to
decompress from another, which won't work.

It's actually possible to prove that overflow can cause image
corruption without this patch; I'll add the iotests separately
in the next commit.

Signed-off-by: Eric Blake <address@hidden>
Reviewed-by: Alberto Garcia <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 3b94c343f98b660e1c1cf79a8f704151962ecd48
      
https://github.com/qemu/qemu/commit/3b94c343f98b660e1c1cf79a8f704151962ecd48
  Author: Eric Blake <address@hidden>
  Date:   2018-11-19 (Mon, 19 Nov 2018)

  Changed paths:
    A tests/qemu-iotests/220
    A tests/qemu-iotests/220.out
    M tests/qemu-iotests/group

  Log Message:
  -----------
  iotests: Add new test 220 for max compressed cluster offset

If you have a capable file system (tmpfs is good, ext4 not so much;
run ./check with TEST_DIR pointing to a good location so as not
to skip the test), it's actually possible to create a qcow2 file
that expands to a sparse 512T image with just over 38M of content.
The test is not the world's fastest (qemu crawling through 256M
bits of refcount table to find the next cluster to allocate takes
several seconds, as does qemu-img check reporting millions of
leaked clusters); but it DOES catch the problem that the previous
patch just fixed where writing a compressed cluster to a full
image ended up overwriting the wrong cluster.

Suggested-by: Max Reitz <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
Reviewed-by: Alberto Garcia <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 9ad08c44566bf4466c6263c71d43e9f7a354d4ba
      
https://github.com/qemu/qemu/commit/9ad08c44566bf4466c6263c71d43e9f7a354d4ba
  Author: Max Reitz <address@hidden>
  Date:   2018-11-19 (Mon, 19 Nov 2018)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: Always abort reopen after prepare succeeded

bdrv_reopen_multiple() does not invoke bdrv_reopen_abort() for the
element of the reopen queue for which bdrv_reopen_prepare() failed,
because it assumes that the prepare function will have rolled back all
changes already.

However, bdrv_reopen_prepare() does not do this in every case: It may
notice an error after BlockDriver.bdrv_reopen_prepare() succeeded, and
it will not invoke BlockDriver.bdrv_reopen_abort() then; and neither
will bdrv_reopen_multiple(), as explained above.

This is wrong because we must always call .bdrv_reopen_commit() or
.bdrv_reopen_abort() after .bdrv_reopen_prepare() has succeeded.
Otherwise, the block driver has no chance to undo what it has done in
its implementation of .bdrv_reopen_prepare().

To fix this, bdrv_reopen_prepare() has to call .bdrv_reopen_abort() if
it wants to return an error after .bdrv_reopen_prepare() has succeeded.

Signed-off-by: Max Reitz <address@hidden>
Reviewed-by: Alberto Garcia <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 577a133988c76e4ebf01d050d0d758d207a1baf7
      
https://github.com/qemu/qemu/commit/577a133988c76e4ebf01d050d0d758d207a1baf7
  Author: Max Reitz <address@hidden>
  Date:   2018-11-19 (Mon, 19 Nov 2018)

  Changed paths:
    M block/file-posix.c

  Log Message:
  -----------
  file-posix: Fix shared locks on reopen commit

s->locked_shared_perm is the set of bits locked in the file, which is
the inverse of the permissions actually shared.  So we need to pass them
as they are to raw_apply_lock_bytes() instead of inverting them again.

Reported-by: Alberto Garcia <address@hidden>
Signed-off-by: Max Reitz <address@hidden>
Reviewed-by: Alberto Garcia <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 6d0a4a0fb5c8f10c8eb68b52cfda0082b00ae963
      
https://github.com/qemu/qemu/commit/6d0a4a0fb5c8f10c8eb68b52cfda0082b00ae963
  Author: Max Reitz <address@hidden>
  Date:   2018-11-19 (Mon, 19 Nov 2018)

  Changed paths:
    M tests/qemu-iotests/182
    M tests/qemu-iotests/182.out

  Log Message:
  -----------
  iotests: Test file-posix locking and reopen

Signed-off-by: Max Reitz <address@hidden>
Reviewed-by: Alberto Garcia <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: e6ebbd46b6e539f3613136111977721d212c2812
      
https://github.com/qemu/qemu/commit/e6ebbd46b6e539f3613136111977721d212c2812
  Author: Peter Maydell <address@hidden>
  Date:   2018-11-19 (Mon, 19 Nov 2018)

  Changed paths:
    M block.c
    M block/file-posix.c
    M block/qcow2-refcount.c
    M block/qcow2.h
    M block/vvfat.c
    M docs/interop/qcow2.txt
    M hw/block/fdc.c
    M hw/block/nvme.c
    M tests/qemu-iotests/182
    M tests/qemu-iotests/182.out
    A tests/qemu-iotests/220
    A tests/qemu-iotests/220.out
    M tests/qemu-iotests/group

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging

Block layer patches:

- file-posix: Fix shared permission locks after reopen
- block: Fix error path for failed .bdrv_reopen_prepare
- qcow2: Catch invalid allocations when the image becomes too large
- vvfat/fdc/nvme: Fix segfaults and leaks

# gpg: Signature made Mon 19 Nov 2018 14:28:18 GMT
# gpg:                using RSA key 7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <address@hidden>"
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74  56FE 7F09 B272 C88F 2FD6

* remotes/kevin/tags/for-upstream:
  iotests: Test file-posix locking and reopen
  file-posix: Fix shared locks on reopen commit
  block: Always abort reopen after prepare succeeded
  iotests: Add new test 220 for max compressed cluster offset
  qcow2: Don't allow overflow during cluster allocation
  qcow2: Document some maximum size constraints
  vvfat: Fix memory leak
  fdc: fix segfault in fdctrl_stop_transfer() when DMA is disabled
  nvme: fix oob access issue(CVE-2018-16847)

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


Compare: https://github.com/qemu/qemu/compare/9436e082de18...e6ebbd46b6e5
      **NOTE:** This service has been marked for deprecation: 
https://developer.github.com/changes/2018-04-25-github-services-deprecation/

      Functionality will be removed from GitHub.com on January 31st, 2019.