qemu-commits
[Top][All Lists]
Advanced

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

[Qemu-commits] [qemu/qemu] 695834: file-posix: clean up max_segments buf


From: GitHub
Subject: [Qemu-commits] [qemu/qemu] 695834: file-posix: clean up max_segments buffer terminati...
Date: Fri, 17 Mar 2017 07:15:11 -0700

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 69583490856713f693291b32fc74b6d0f5992b72
      
https://github.com/qemu/qemu/commit/69583490856713f693291b32fc74b6d0f5992b72
  Author: Stefan Hajnoczi <address@hidden>
  Date:   2017-03-17 (Fri, 17 Mar 2017)

  Changed paths:
    M block/file-posix.c

  Log Message:
  -----------
  file-posix: clean up max_segments buffer termination

The following pattern is unsafe:

  char buf[32];
  ret = read(fd, buf, sizeof(buf));
  ...
  buf[ret] = 0;

If read(2) returns 32 then a byte beyond the end of the buffer is
zeroed.

In practice this buffer overflow does not occur because the sysfs
max_segments file only contains an unsigned short + '\n'.  The string is
always shorter than 32 bytes.

Regardless, avoid this pattern because static analysis tools might
complain and it could lead to real buffer overflows if copy-pasted
elsewhere in the codebase.

Signed-off-by: Stefan Hajnoczi <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 37a9051cc7d30672216af5f6620af1da122f66b3
      
https://github.com/qemu/qemu/commit/37a9051cc7d30672216af5f6620af1da122f66b3
  Author: Changlong Xie <address@hidden>
  Date:   2017-03-17 (Fri, 17 Mar 2017)

  Changed paths:
    M block/replication.c

  Log Message:
  -----------
  replication: clarify permissions

Even if hidden_disk, secondary_disk are backing files, they all need
write permissions in replication scenario. Otherwise we will encouter
below exceptions on secondary side during adding nbd server:

{'execute': 'nbd-server-add', 'arguments': {'device': 'colo-disk', 'writable': 
true } }
{"error": {"class": "GenericError", "desc": "Conflicts with use by 
hidden-qcow2-driver as 'backing', which does not allow 'write' on 
sec-qcow2-driver-for-nbd"}}

CC: Zhang Hailiang <address@hidden>
CC: Zhang Chen <address@hidden>
CC: Wen Congyang <address@hidden>
Signed-off-by: Changlong Xie <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: fed414df9dc9abef040adfbd8c5956fb610edaa2
      
https://github.com/qemu/qemu/commit/fed414df9dc9abef040adfbd8c5956fb610edaa2
  Author: Fam Zheng <address@hidden>
  Date:   2017-03-17 (Fri, 17 Mar 2017)

  Changed paths:
    M block/file-posix.c

  Log Message:
  -----------
  file-posix: Don't leak fd in hdev_get_max_segments

This fixes a leaked fd introduced in commit 9103f1ce.

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


  Commit: c1cef67251d3e6ae275c0898ccf4cbcfe85d4e0b
      
https://github.com/qemu/qemu/commit/c1cef67251d3e6ae275c0898ccf4cbcfe85d4e0b
  Author: Fam Zheng <address@hidden>
  Date:   2017-03-17 (Fri, 17 Mar 2017)

  Changed paths:
    M block.c
    M block/mirror.c
    M include/block/block_int.h

  Log Message:
  -----------
  block: Always call bdrv_child_check_perm first

bdrv_child_set_perm alone is not very usable because the caller must
call bdrv_child_check_perm first. This is already encapsulated
conveniently in bdrv_child_try_set_perm, so remove the other prototypes
from the header and fix the one wrong caller, block/mirror.c.

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


  Commit: 184dd9c49b522fe729cfe21d119f6db241268a50
      
https://github.com/qemu/qemu/commit/184dd9c49b522fe729cfe21d119f6db241268a50
  Author: John Snow <address@hidden>
  Date:   2017-03-17 (Fri, 17 Mar 2017)

  Changed paths:
    M blockdev.c

  Log Message:
  -----------
  blockdev: fix bitmap clear undo

Only undo the action if we actually prepared the action.

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


  Commit: 8cd1a3e470212685bdd48ac03fdf9721dafc100b
      
https://github.com/qemu/qemu/commit/8cd1a3e470212685bdd48ac03fdf9721dafc100b
  Author: Fam Zheng <address@hidden>
  Date:   2017-03-17 (Fri, 17 Mar 2017)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: Propagate error in bdrv_open_backing_file

Signed-off-by: Fam Zheng <address@hidden>
Reviewed-by: Alberto Garcia <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: b7a745dc33a18377bb4a8dfe54d1df01ea60bf66
      
https://github.com/qemu/qemu/commit/b7a745dc33a18377bb4a8dfe54d1df01ea60bf66
  Author: Peter Lieven <address@hidden>
  Date:   2017-03-17 (Fri, 17 Mar 2017)

  Changed paths:
    M util/thread-pool.c

  Log Message:
  -----------
  thread-pool: add missing qemu_bh_cancel in completion function

commit 3c80ca15 fixed a deadlock scenarion with nested aio_poll invocations.

However, the rescheduling of the completion BH introcuded unnecessary spinning
in the main-loop. On very fast file backends this can even lead to the
"WARNING: I/O thread spun for 1000 iterations" message popping up.

Callgrind reports about 3-4% less instructions with this patch running
qemu-img bench on a ramdisk based VMDK file.

Fixes: 3c80ca158c96ff902a30883a8933e755988948b1
Cc: address@hidden
Signed-off-by: Peter Lieven <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: c2b6428d388b3f03261be7b0be770919e4e3e5ec
      
https://github.com/qemu/qemu/commit/c2b6428d388b3f03261be7b0be770919e4e3e5ec
  Author: Paolo Bonzini <address@hidden>
  Date:   2017-03-17 (Fri, 17 Mar 2017)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: quiesce AioContext when detaching from it

While it is true that bdrv_set_aio_context only works on a single
BlockDriverState subtree (see commit message for 53ec73e, "block: Use
bdrv_drain to replace uncessary bdrv_drain_all", 2015-07-07), it works
at the AioContext level rather than the BlockDriverState level.

Therefore, it is also necessary to trigger pending bottom halves too,
even if no requests are pending.

For NBD this ensures that the aio_co_schedule of a previous call to
nbd_attach_aio_context is completed before detaching from the old
AioContext; it fixes qemu-iotest 094.  Another similar bug happens
when the VM is stopped and the virtio-blk dataplane irqfd is torn down.
In this case it's possible that guest I/O gets stuck if notify_guest_bh
was scheduled but doesn't run.

Calling aio_poll from another AioContext is safe if non-blocking; races
such as the one mentioned in the commit message for c9d1a56 ("block:
only call aio_poll on the current thread's AioContext", 2016-10-28)
are a concern for blocking calls.

I considered other options, including:

- moving the bs->wakeup mechanism to AioContext, and letting the caller
check.  This might work for virtio which has a clear place to wakeup
(notify_place_bh) and check the condition (virtio_blk_data_plane_stop).
For aio_co_schedule I couldn't find a clear place to check the condition.

- adding a dummy oneshot bottom half and waiting for it to trigger.
This has the complication that bottom half list is LIFO for historical
reasons.  There were performance issues caused by bottom half ordering
in the past, so I decided against it for 2.9.

Fixes: 99723548561978da8ef44cf804fb7912698f5d88
Reported-by: Max Reitz <address@hidden>
Reported-by: Halil Pasic <address@hidden>
Tested-by: Halil Pasic <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
Message-id: address@hidden
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Max Reitz <address@hidden>


  Commit: 11f0f5e553cff93d5fc77d67b98c40adba12b729
      
https://github.com/qemu/qemu/commit/11f0f5e553cff93d5fc77d67b98c40adba12b729
  Author: Kevin Wolf <address@hidden>
  Date:   2017-03-17 (Fri, 17 Mar 2017)

  Changed paths:
    M block.c

  Log Message:
  -----------
  Merge remote-tracking branch 'mreitz/tags/pull-block-2017-03-17' into 
queue-block

Block patches for 2.9-rc1

# gpg: Signature made Fri Mar 17 12:59:20 2017 CET
# gpg:                using RSA key 0xF407DB0061D5CF40
# gpg: Good signature from "Max Reitz <address@hidden>"
# Primary key fingerprint: 91BE B60A 30DB 3E88 57D1  1829 F407 DB00 61D5 CF40

* mreitz/tags/pull-block-2017-03-17:
  block: quiesce AioContext when detaching from it

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


  Commit: 31d89228366ce79064ac4fdef98ac85b39bf53ba
      
https://github.com/qemu/qemu/commit/31d89228366ce79064ac4fdef98ac85b39bf53ba
  Author: Peter Maydell <address@hidden>
  Date:   2017-03-17 (Fri, 17 Mar 2017)

  Changed paths:
    M block.c
    M block/file-posix.c
    M block/mirror.c
    M block/replication.c
    M blockdev.c
    M include/block/block_int.h
    M util/thread-pool.c

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

Block layer fixes for 2.9.0-rc1

# gpg: Signature made Fri 17 Mar 2017 12:06:04 GMT
# gpg:                using RSA key 0x7F09B272C88F2FD6
# 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:
  block: quiesce AioContext when detaching from it
  thread-pool: add missing qemu_bh_cancel in completion function
  block: Propagate error in bdrv_open_backing_file
  blockdev: fix bitmap clear undo
  block: Always call bdrv_child_check_perm first
  file-posix: Don't leak fd in hdev_get_max_segments
  replication: clarify permissions
  file-posix: clean up max_segments buffer termination

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


Compare: https://github.com/qemu/qemu/compare/272d7dee5951...31d89228366c

reply via email to

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