Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 05 Jun 2019 16:11:27 +0000
From:      bugzilla-noreply@freebsd.org
To:        virtualization@FreeBSD.org
Subject:   [Bug 238333] bhyve random crash in rfb.c on FreeBSD current (after r346011)
Message-ID:  <bug-238333-27103-bnGAZmhuTP@https.bugs.freebsd.org/bugzilla/>
In-Reply-To: <bug-238333-27103@https.bugs.freebsd.org/bugzilla/>
References:  <bug-238333-27103@https.bugs.freebsd.org/bugzilla/>

next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D238333

--- Comment #2 from Conrad Meyer <cem@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 "ena=
ble
      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" field=
s,
   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 enou=
gh
   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 w=
ith
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 nothin=
g to
*verify* this assumption (avail_out >0).  I'm not immediately seeing how th=
is
leads to this crash, though.

E.g.:

  $ dd if=3D/dev/urandom bs=3D$((2000*1200*4)) count=3D1 of=3D./randomscree=
n.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 c=
an
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!

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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-238333-27103-bnGAZmhuTP>