1. 15 1月, 2018 5 次提交
  2. 13 1月, 2018 1 次提交
    • P
      Merge remote-tracking branch 'remotes/kraxel/tags/vnc-20180112-pull-request' into staging · 7398166d
      Peter Maydell 提交于
      vnc: limit memory usage (CVE-2017-15124)
      
      # gpg: Signature made Fri 12 Jan 2018 12:57:22 GMT
      # gpg:                using RSA key 0x4CB6D8EED3E87138
      # gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>"
      # gpg:                 aka "Gerd Hoffmann <gerd@kraxel.org>"
      # gpg:                 aka "Gerd Hoffmann (private) <kraxel@gmail.com>"
      # Primary key fingerprint: A032 8CFF B93A 17A7 9901  FE7D 4CB6 D8EE D3E8 7138
      
      * remotes/kraxel/tags/vnc-20180112-pull-request:
        ui: mix misleading comments & return types of VNC I/O helper methods
        ui: add trace events related to VNC client throttling
        ui: place a hard cap on VNC server output buffer size
        ui: fix VNC client throttling when forced update is requested
        ui: fix VNC client throttling when audio capture is active
        ui: refactor code for determining if an update should be sent to the client
        ui: correctly reset framebuffer update state after processing dirty regions
        ui: introduce enum to track VNC client framebuffer update request state
        ui: track how much decoded data we consumed when doing SASL encoding
        ui: avoid pointless VNC updates if framebuffer isn't dirty
        ui: remove redundant indentation in vnc_client_update
        ui: remove unreachable code in vnc_update_client
        ui: remove 'sync' parameter from vnc_update_client
        vnc: fix debug spelling
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      7398166d
  3. 12 1月, 2018 17 次提交
    • P
      target/xtensa: Remove duplicate typedef of DisasContext · a3380cf6
      Peter Maydell 提交于
      Some older versions of gcc complain if a typedef is defined twice:
      
      target/xtensa/translate.c:81: error: redefinition of typedef 'DisasContext'
      target/xtensa/cpu.h:339: note: previous declaration of 'DisasContext' was here
      
      Remove the now-redundant typedef from the definition of the struct in
      translate.c.
      Reported-by: NPaolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      Message-id: 1515762528-22818-1-git-send-email-peter.maydell@linaro.org
      a3380cf6
    • D
      ui: mix misleading comments & return types of VNC I/O helper methods · 30b80fd5
      Daniel P. Berrange 提交于
      While the QIOChannel APIs for reading/writing data return ssize_t, with negative
      value indicating an error, the VNC code passes this return value through the
      vnc_client_io_error() method. This detects the error condition, disconnects the
      client and returns 0 to indicate error. Thus all the VNC helper methods should
      return size_t (unsigned), and misleading comments which refer to the possibility
      of negative return values need fixing.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Reviewed-by: NDarren Kenny <darren.kenny@oracle.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171218191228.31018-14-berrange@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      30b80fd5
    • D
      ui: add trace events related to VNC client throttling · 6aa22a29
      Daniel P. Berrange 提交于
      The VNC client throttling is quite subtle so will benefit from having trace
      points available for live debugging.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Reviewed-by: NDarren Kenny <darren.kenny@oracle.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171218191228.31018-13-berrange@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      6aa22a29
    • D
      ui: place a hard cap on VNC server output buffer size · f887cf16
      Daniel P. Berrange 提交于
      The previous patches fix problems with throttling of forced framebuffer updates
      and audio data capture that would cause the QEMU output buffer size to grow
      without bound. Those fixes are graceful in that once the client catches up with
      reading data from the server, everything continues operating normally.
      
      There is some data which the server sends to the client that is impractical to
      throttle. Specifically there are various pseudo framebuffer update encodings to
      inform the client of things like desktop resizes, pointer changes, audio
      playback start/stop, LED state and so on. These generally only involve sending
      a very small amount of data to the client, but a malicious guest might be able
      to do things that trigger these changes at a very high rate. Throttling them is
      not practical as missed or delayed events would cause broken behaviour for the
      client.
      
      This patch thus takes a more forceful approach of setting an absolute upper
      bound on the amount of data we permit to be present in the output buffer at
      any time. The previous patch set a threshold for throttling the output buffer
      by allowing an amount of data equivalent to one complete framebuffer update and
      one seconds worth of audio data. On top of this it allowed for one further
      forced framebuffer update to be queued.
      
      To be conservative, we thus take that throttling threshold and multiply it by
      5 to form an absolute upper bound. If this bound is hit during vnc_write() we
      forceably disconnect the client, refusing to queue further data. This limit is
      high enough that it should never be hit unless a malicious client is trying to
      exploit the sever, or the network is completely saturated preventing any sending
      of data on the socket.
      
      This completes the fix for CVE-2017-15124 started in the previous patches.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Reviewed-by: NDarren Kenny <darren.kenny@oracle.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171218191228.31018-12-berrange@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      f887cf16
    • D
      ui: fix VNC client throttling when forced update is requested · ada8d2e4
      Daniel P. Berrange 提交于
      The VNC server must throttle data sent to the client to prevent the 'output'
      buffer size growing without bound, if the client stops reading data off the
      socket (either maliciously or due to stalled/slow network connection).
      
      The current throttling is very crude because it simply checks whether the
      output buffer offset is zero. This check is disabled if the client has requested
      a forced update, because we want to send these as soon as possible.
      
      As a result, the VNC client can cause QEMU to allocate arbitrary amounts of RAM.
      They can first start something in the guest that triggers lots of framebuffer
      updates eg play a youtube video. Then repeatedly send full framebuffer update
      requests, but never read data back from the server. This can easily make QEMU's
      VNC server send buffer consume 100MB of RAM per second, until the OOM killer
      starts reaping processes (hopefully the rogue QEMU process, but it might pick
      others...).
      
      To address this we make the throttling more intelligent, so we can throttle
      full updates. When we get a forced update request, we keep track of exactly how
      much data we put on the output buffer. We will not process a subsequent forced
      update request until this data has been fully sent on the wire. We always allow
      one forced update request to be in flight, regardless of what data is queued
      for incremental updates or audio data. The slight complication is that we do
      not initially know how much data an update will send, as this is done in the
      background by the VNC job thread. So we must track the fact that the job thread
      has an update pending, and not process any further updates until this job is
      has been completed & put data on the output buffer.
      
      This unbounded memory growth affects all VNC server configurations supported by
      QEMU, with no workaround possible. The mitigating factor is that it can only be
      triggered by a client that has authenticated with the VNC server, and who is
      able to trigger a large quantity of framebuffer updates or audio samples from
      the guest OS. Mostly they'll just succeed in getting the OOM killer to kill
      their own QEMU process, but its possible other processes can get taken out as
      collateral damage.
      
      This is a more general variant of the similar unbounded memory usage flaw in
      the websockets server, that was previously assigned CVE-2017-15268, and fixed
      in 2.11 by:
      
        commit a7b20a8e
        Author: Daniel P. Berrange <berrange@redhat.com>
        Date:   Mon Oct 9 14:43:42 2017 +0100
      
          io: monitor encoutput buffer size from websocket GSource
      
      This new general memory usage flaw has been assigned CVE-2017-15124, and is
      partially fixed by this patch.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Reviewed-by: NDarren Kenny <darren.kenny@oracle.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171218191228.31018-11-berrange@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      ada8d2e4
    • D
      ui: fix VNC client throttling when audio capture is active · e2b72cb6
      Daniel P. Berrange 提交于
      The VNC server must throttle data sent to the client to prevent the 'output'
      buffer size growing without bound, if the client stops reading data off the
      socket (either maliciously or due to stalled/slow network connection).
      
      The current throttling is very crude because it simply checks whether the
      output buffer offset is zero. This check must be disabled if audio capture is
      enabled, because when streaming audio the output buffer offset will rarely be
      zero due to queued audio data, and so this would starve framebuffer updates.
      
      As a result, the VNC client can cause QEMU to allocate arbitrary amounts of RAM.
      They can first start something in the guest that triggers lots of framebuffer
      updates eg play a youtube video. Then enable audio capture, and simply never
      read data back from the server. This can easily make QEMU's VNC server send
      buffer consume 100MB of RAM per second, until the OOM killer starts reaping
      processes (hopefully the rogue QEMU process, but it might pick others...).
      
      To address this we make the throttling more intelligent, so we can throttle
      when audio capture is active too. To determine how to throttle incremental
      updates or audio data, we calculate a size threshold. Normally the threshold is
      the approximate number of bytes associated with a single complete framebuffer
      update. ie width * height * bytes per pixel. We'll send incremental updates
      until we hit this threshold, at which point we'll stop sending updates until
      data has been written to the wire, causing the output buffer offset to fall
      back below the threshold.
      
      If audio capture is enabled, we increase the size of the threshold to also
      allow for upto 1 seconds worth of audio data samples. ie nchannels * bytes
      per sample * frequency. This allows the output buffer to have a mixture of
      incremental framebuffer updates and audio data queued, but once the threshold
      is exceeded, audio data will be dropped and incremental updates will be
      throttled.
      
      This unbounded memory growth affects all VNC server configurations supported by
      QEMU, with no workaround possible. The mitigating factor is that it can only be
      triggered by a client that has authenticated with the VNC server, and who is
      able to trigger a large quantity of framebuffer updates or audio samples from
      the guest OS. Mostly they'll just succeed in getting the OOM killer to kill
      their own QEMU process, but its possible other processes can get taken out as
      collateral damage.
      
      This is a more general variant of the similar unbounded memory usage flaw in
      the websockets server, that was previously assigned CVE-2017-15268, and fixed
      in 2.11 by:
      
        commit a7b20a8e
        Author: Daniel P. Berrange <berrange@redhat.com>
        Date:   Mon Oct 9 14:43:42 2017 +0100
      
          io: monitor encoutput buffer size from websocket GSource
      
      This new general memory usage flaw has been assigned CVE-2017-15124, and is
      partially fixed by this patch.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Reviewed-by: NDarren Kenny <darren.kenny@oracle.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171218191228.31018-10-berrange@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      e2b72cb6
    • D
      ui: refactor code for determining if an update should be sent to the client · 0bad8342
      Daniel P. Berrange 提交于
      The logic for determining if it is possible to send an update to the client
      will become more complicated shortly, so pull it out into a separate method
      for easier extension later.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Reviewed-by: NDarren Kenny <darren.kenny@oracle.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171218191228.31018-9-berrange@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      0bad8342
    • D
      ui: correctly reset framebuffer update state after processing dirty regions · 728a7ac9
      Daniel P. Berrange 提交于
      According to the RFB protocol, a client sends one or more framebuffer update
      requests to the server. The server can reply with a single framebuffer update
      response, that covers all previously received requests. Once the client has
      read this update from the server, it may send further framebuffer update
      requests to monitor future changes. The client is free to delay sending the
      framebuffer update request if it needs to throttle the amount of data it is
      reading from the server.
      
      The QEMU VNC server, however, has never correctly handled the framebuffer
      update requests. Once QEMU has received an update request, it will continue to
      send client updates forever, even if the client hasn't asked for further
      updates. This prevents the client from throttling back data it gets from the
      server. This change fixes the flawed logic such that after a set of updates are
      sent out, QEMU waits for a further update request before sending more data.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Reviewed-by: NDarren Kenny <darren.kenny@oracle.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171218191228.31018-8-berrange@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      728a7ac9
    • D
      ui: introduce enum to track VNC client framebuffer update request state · fef1bbad
      Daniel P. Berrange 提交于
      Currently the VNC servers tracks whether a client has requested an incremental
      or forced update with two boolean flags. There are only really 3 distinct
      states to track, so create an enum to more accurately reflect permitted states.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Reviewed-by: NDarren Kenny <darren.kenny@oracle.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171218191228.31018-7-berrange@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      fef1bbad
    • D
      ui: track how much decoded data we consumed when doing SASL encoding · 8f61f1c5
      Daniel P. Berrange 提交于
      When we encode data for writing with SASL, we encode the entire pending output
      buffer. The subsequent write, however, may not be able to send the full encoded
      data in one go though, particularly with a slow network. So we delay setting the
      output buffer offset back to zero until all the SASL encoded data is sent.
      
      Between encoding the data and completing sending of the SASL encoded data,
      however, more data might have been placed on the pending output buffer. So it
      is not valid to set offset back to zero. Instead we must keep track of how much
      data we consumed during encoding and subtract only that amount.
      
      With the current bug we would be throwing away some pending data without having
      sent it at all. By sheer luck this did not previously cause any serious problem
      because appending data to the send buffer is always an atomic action, so we
      only ever throw away complete RFB protocol messages. In the case of frame buffer
      updates we'd catch up fairly quickly, so no obvious problem was visible.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Reviewed-by: NDarren Kenny <darren.kenny@oracle.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171218191228.31018-6-berrange@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      8f61f1c5
    • D
      ui: avoid pointless VNC updates if framebuffer isn't dirty · 3541b084
      Daniel P. Berrange 提交于
      The vnc_update_client() method checks the 'has_dirty' flag to see if there are
      dirty regions that are pending to send to the client. Regardless of this flag,
      if a forced update is requested, updates must be sent. For unknown reasons
      though, the code also tries to sent updates if audio capture is enabled. This
      makes no sense as audio capture state does not impact framebuffer contents, so
      this check is removed.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Reviewed-by: NDarren Kenny <darren.kenny@oracle.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171218191228.31018-5-berrange@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      3541b084
    • D
      ui: remove redundant indentation in vnc_client_update · b939eb89
      Daniel P. Berrange 提交于
      Now that previous dead / unreachable code has been removed, we can simplify
      the indentation in the vnc_client_update method.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Reviewed-by: NDarren Kenny <darren.kenny@oracle.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171218191228.31018-4-berrange@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      b939eb89
    • D
      ui: remove unreachable code in vnc_update_client · c53df961
      Daniel P. Berrange 提交于
      A previous commit:
      
        commit 5a8be0f7
        Author: Gerd Hoffmann <kraxel@redhat.com>
        Date:   Wed Jul 13 12:21:20 2016 +0200
      
          vnc: make sure we finish disconnect
      
      Added a check for vs->disconnecting at the very start of the
      vnc_update_client method. This means that the very next "if"
      statement check for !vs->disconnecting always evaluates true,
      and is thus redundant. This in turn means the vs->disconnecting
      check at the very end of the method never evaluates true, and
      is thus unreachable code.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Reviewed-by: NDarren Kenny <darren.kenny@oracle.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171218191228.31018-3-berrange@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      c53df961
    • D
      ui: remove 'sync' parameter from vnc_update_client · 6af998db
      Daniel P. Berrange 提交于
      There is only one caller of vnc_update_client and that always passes false
      for the 'sync' parameter.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Reviewed-by: NDarren Kenny <darren.kenny@oracle.com>
      Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171218191228.31018-2-berrange@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      6af998db
    • M
      vnc: fix debug spelling · 090fdc83
      Marc-André Lureau 提交于
      Signed-off-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-id: 20171220140618.12701-1-marcandre.lureau@redhat.com
      Signed-off-by: NGerd Hoffmann <kraxel@redhat.com>
      090fdc83
    • P
      Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging · 36b5e43a
      Peter Maydell 提交于
      pc, pci, virtio: features, fixes, cleanups
      
      A bunch of fixes, cleanus and new features all over the place.
      Signed-off-by: NMichael S. Tsirkin <mst@redhat.com>
      
      # gpg: Signature made Thu 11 Jan 2018 20:04:57 GMT
      # gpg:                using RSA key 0x281F0DB8D28D5469
      # gpg: Good signature from "Michael S. Tsirkin <mst@kernel.org>"
      # gpg:                 aka "Michael S. Tsirkin <mst@redhat.com>"
      # Primary key fingerprint: 0270 606B 6F3C DF3D 0B17  0970 C350 3912 AFBE 8E67
      #      Subkey fingerprint: 5D09 FD08 71C8 F85B 94CA  8A0D 281F 0DB8 D28D 5469
      
      * remotes/mst/tags/for_upstream: (23 commits)
        smbus: do not immediately complete commands
        dump-guest-memory.py: fix "You can't do that without a process to debug"
        virtio-pci: Don't force Subsystem Vendor ID = Vendor ID
        intel_iommu: fix error param in string
        intel_iommu: remove X86_IOMMU_PCI_DEVFN_MAX
        vhost-user: document memory accesses
        vhost-user: fix indentation in protocol specification
        hw/pci-host/xilinx: QOM'ify the AXI-PCIe host bridge
        hw/pci-host/piix: QOM'ify the IGD Passthrough host bridge
        tests/pxe-test: Add some extra tests
        tests/pxe-test: Test net booting over IPv6 in some cases
        tests/pxe-test: Use table of testcases rather than open-coding
        tests/pxe-test: Remove unnecessary special case test functions
        virtio_error: don't invoke status callbacks
        pci: Eliminate pci_find_primary_bus()
        pci: Eliminate redundant PCIDevice::bus pointer
        pci: Add pci_dev_bus_num() helper
        pci: Move bridge data structures from pci_bus.h to pci_bridge.h
        pci: Rename root bus initialization functions for clarity
        tests: add test to check VirtQueue object
        ...
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      36b5e43a
    • M
      Merge remote-tracking branch 'origin/master' into HEAD · acc95bc8
      Michael S. Tsirkin 提交于
      Resolve conflicts around apb.
      Signed-off-by: NMichael S. Tsirkin <mst@redhat.com>
      acc95bc8
  4. 11 1月, 2018 17 次提交