Skip to content

Limit websockets memory usage & other bug fixes

Daniel P. Berrange qemu-devel 2 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest7 patches 4 repliesIn review
  2. v17 patches 11 repliesSuperseded

Changes in v2 from the cover letter

- Correctly index the struct iovec array when encoding (Eric)
- Change ping_remain to pong_remain (Eric)
- Misc typos (Eric)

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/7io: monitor encoutput buffer size from websocket GSourceio/channel-websock.c+2 -20 replies
2/7io: simplify websocket ping reply handlinginclude/io/channel-websock.h io/channel-websock.c+16 -140 replies
3/7io: get rid of qio_channel_websock_encode helper methodio/channel-websock.c+6 -140 replies
4/7io: pass a struct iovec into qio_channel_websock_encodeio/channel-websock.c+42 -273 replies
5/7io: get rid of bounce buffering in websock write pathinclude/io/channel-websock.h io/channel-websock.c+28 -371 reply
6/7io: cope with websock 'Connection' header having multiple valuesio/channel-websock.c+13 -10 replies
7/7io: add trace points for websocket HTTP protocol headersio/channel-websock.c io/trace-events+6 -00 replies

Reviews trailers per patch

All 7 patches have a Reviewed-by or Acked-by.

PatchEric Blake
1/7R-b
2/7R-b
3/7R-b
4/7R-b
5/7R-b
6/7R-b
7/7R-b

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v2 thread

Cover letter [Qemu-devel] [PATCH v2 0/7] Limit websockets memory usage & other bug fixes

The core motivation for this patch series is to fix a security issue publically reported, where websockets code can consume arbitrary amounts of RAM with slow clients:

https://bugs.launchpad.net/qemu/+bug/1718964

I've asked for a CVE but its not assigned yet. Since the bug is public we might as well get on with code review while waiting for the CVE number.

The first patch is the minimum required to fix the actual CVE in git master, taking advantage of how we know the VNC server will call us. The 5th patch lets us tighten up buffer limiting of writes further, so we're not making assumptions about VNC server code.

The websockets code is broken right back to the day it was merged in QEMU 1.2.1

The fix in patch 1 can apply to stable branches from 2.6 -> 2.10 inclusive, provided another fix from master is cherry-picked first

commit eefa3d8ef649f9055611361e2201cca49f8c3433
Author: Brandon Carpenter <[email protected]>
Date: Tue Sep 12 08:21:48 2017 -0700

io: Small updates in preparation for websocket changes

since that refactors code duplication in the GSource impl. Once we merge for master, I'll send a patch to qemu-stable.

Versions prior to 2.6 would require a fix to be done in the ui/vnc.c file vnc_update_client method instead. It would need to check vs->ws_output buffer size. I'm not intending to write any such patch - this is just info in case anyone is stuck on such ancient versions and needs to figure out a fix.

Changed in v2:

- Correctly index the struct iovec array when encoding (Eric)
- Change ping_remain to pong_remain (Eric)
- Misc typos (Eric)

Daniel P. Berrange (7):
io: monitor encoutput buffer size from websocket GSource
io: simplify websocket ping reply handling
io: get rid of qio_channel_websock_encode helper method
io: pass a struct iovec into qio_channel_websock_encode
io: get rid of bounce buffering in websock write path
io: cope with websock 'Connection' header having multiple values
io: add trace points for websocket HTTP protocol headers

 include/io/channel-websock.h |   3 +-
 io/channel-websock.c         | 163 ++++++++++++++++++++++++-------------------
 io/trace-events              |   2 +
 3 files changed, 93 insertions(+), 75 deletions(-)
Signature · 1 line
2.13.5