[Bug 238333] bhyve random crash in rfb.c on FreeBSD current (after r346011)

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238333

--- Comment #2 from Conrad Meyer <cem at freebsd.org> ---
Ok, here's what rfb.c tries to do:

1. Allocate a single zstream per rfb_softc.  (fine)
2. Handle a single connection at a time per rfb_softc.  (fine)
3. Periodically re-send the screen from a co-thread.  (fine? could just be
   done from main rfb_handle thread)
4. Serialize screen send using a mutex + flag.  (fine)

Here is some dubious stuff:

1. Client commands, including "set encoding" (i.e., RAW, ZLIB, RESIZE) are not
serialized against periodic worker thread.
   a. Also including things like "change dimensions of the screen" and "enable
      ZLIB".

2. The zstream is never reinitialized, but reused.  This may be desirable
   (i.e., better compression is achieved if the client can reliably inflate
   references to historical stream data).

3. Client is allowed to arbitrarily update softc "height" and "width" fields,
   although amusingly these are never used?

4. zbuf isn't really sized for totally incompressible full size maximal
   screens?  2000*1200*4 is 9.1 MB and 16 bytes slop is definitely not enough
   for zlib --fast overhead.

---

The line number in deflate() and stack in flush_pending() suggest that
assumptions in rfb.c were violated.  rfb initiates every single operation with
Z_SYNC_FLUSH, which indicates that all input should be processed and nothing
should be left pending in zstream's output buffer.  However, it does nothing to
*verify* this assumption (avail_out >0).  I'm not immediately seeing how this
leads to this crash, though.

E.g.:

  $ dd if=/dev/urandom bs=$((2000*1200*4)) count=1 of=./randomscreen.dat
  9600000 bytes transferred in 0.026229 secs (366010901 bytes/sec)
  $ gzip --fast --no-name ./randomscreen.dat
  $ ls -l ./randomscreen.dat.gz
  ... 9602938 Jun  5 09:04 ./randomscreen.dat.gz

So that's 2938 bytes of zlib overhead for a maximally sized, incompressible
screen.  Subtract a handful of bytes for gzip header/trailer (maybe 32) and
another 16 for the manual slop in rfb.c and you're still short 2.8 kB re:
unchecked assumptions made in rfb.c zbuf sizing.  (And to what end?  zlib can
operate efficiently with much smaller buffers.)

Can you also print `*stream` and `*s`?  What are avail_out, etc?  What size of
framebuffer are you attempting to use?  Do you have a core you can share or
repro steps I could follow?

Thanks!

-- 
You are receiving this mail because:
You are the assignee for the bug.