[MIR] virglrenderer

Move to the security team as per the description; there should be a security review of this package.

virglrenderer is currently undergoing a security review by Li Qiang and there are now 11 open CVEs against the package.

Please assign back to me once the security review is done so I can do the rest of the MIR review, assuming it passes your review.

Status changed to 'Confirmed' because the bug affects multiple users.

Hi, there was no update/result of the security Team review.

Since Debian might close in on this soon (see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=813658) it would be great to have this to be able to follow.

Therefore, what was the result of this evaluation?

Li, are you still reviewing that?

Li is a security researcher. There haven't been new CVEs this year, so it looks like that review is done with 13 CVEs identified.

This is on the list for us to review, but behind a some high priority reviews. Is there a need to increase the priority for this review or are you just cleaning up the queue?

Hi,
I think the time is right (going from 18.10 towards 20.04 some day) to enable it now meaning 18.10 or latest 19.04 to have a few releases in between the LTSes to see how it works.
Also as assumed for some time Debian now has enabled it [1].

@Emily - I think thereby the priority increased a bit, would be great to have the security POV to this package.

[1]: https://salsa.debian.org/qemu-team/qemu/commit/fc18833073661f760eb153f055f9b711beb6f49d

This is near to completion in Debian, so on next merge would be nice to be able to pick this up as well.
I'll bump the trello card for the review as well.

Just FYI: Please note that the overall feature to make it usable for a user also needs "libepoxy-dev, libdrm-dev, libgbm-dev" but those are all already in main. So it is just this MIR here.

FWIW I'm happily using VirGL based qemu also in my backport for 18.04 LTS. Thanks to bug #1804766 landing the needed changes to disco's 1:2.12+dfsg-3ubuntu9 are very small.

https://launchpad.net/~timo-jyrinki/+archive/ubuntu/qemu-virgl for whoever happens to need it on LTS.

Hopefully virgl support will make it to 19.04 officially too.

I reviewed virglrenderer version 0.7.0-1 as checked into disco; this
shouldn't be considered a full security audit but rather a quick gauge of
maintainability.

virgl is an attempt to provide GPUs to guests that leverage the host GPUs
but may not be direct passthroughs of devices.

There's 14 CVEs in our database; most were the result of one fairly active
researcher who appeared to be doing a fairly thorough job. I had the
impression the fixes were handled well.

- Build-Depends: autoconf-archive, debhelper, libdrm-dev,
  libegl1-mesa-dev, libepoxy-dev, libgbm-dev, pkg-config, python

Is the python build dependency cause for concern?

- Does not daemonize
- Does not appear to listen to network
- No pre/post inst/rm scripts
- No init scripts
- No systemd services
- No dbus services
- No setuid files
- No binaries in PATH
- No sudo fragments
- No udev rules
- There's 6KLOC in tests/ but nothing appears to run during the build
- No cron jobs
- Build logs are extremely messy. Addressing the issues would be an
  immense undertaking. (To the point that switching to C++ to use
  std::string or Rust to use String would be easier than trying to
  address these issues in C.)

- No subprocesses spawned
- Memory management is pretty rough. C string operations without bounds
  checking, snprintfs without error return validations, etc. Fixed length
  stack-allocated buffers are used extensively and it's far from clear
  that all inputs will properly fit into the buffers.

  The generated GLGL inputs are fed into other tools and errors would
  likely cause cascading failures.

  Addressing this in C could be a disaster. I suspect trying to "fix" the
  existing code would introduce more errors than it would solve. A rewrite
  into a better language would be a lot more plausible to me.

- Opens /dev/dri/renderD* files
- Logging looked fine
- Uses many of its own environment variables:
  GALLIUM_LOG_FILE, GALLIUM_PRINT_OPTIONS, TGSI_PRINT_SANITY,
  GALLIUM_DUMP_CPU, VIRGL_DISABLE_MT, VTEST_SAVE, VTEST_USE_GLX,
  VTEST_USE_EGL_SURFACELESS, VTEST_USE_GLES, LIBGL_ALWAYS_SOFTWARE,
- No privileged operations
- No cryptography
- I'm unsure about networking -- the test infrastructure uses unix domain
  sockets, but I don't obviously spot how virglrenderer interacts outside
  its process.
- No privileged portions of code
- No temp files
- No webkit
- No JavaScript
- Some cppcheck errors have been fixed upstream since our package

I don't love the GLGL code generation. Running this code may bring the
guest operating system video drivers into the trusted codebase.

I discussed some of the issues I found with upstream authors and found
them very responsive and understanding. (To be clear I found nothing of
importance.)

Security team ACK for promoting virglrenderer to main.

Thanks

Thank you so much for the check Seth.
Per process I set in-progress on this and continue to prep it that way on the merge.

qemu now built in Disco and triggers the expected mismatch
Doko pinged me on IRC about it:
<doko> cpaelzer: qemu-system-arm/amd64 unsatisfiable Depends: libvirglrenderer0 (>= 0.7.0)

Override component to main
virglrenderer 0.7.0-2 in disco: universe/misc -> main
libvirglrenderer-dev 0.7.0-2 in disco amd64: universe/libdevel/extra/100% -> main
libvirglrenderer-dev 0.7.0-2 in disco arm64: universe/libdevel/extra/100% -> main
libvirglrenderer-dev 0.7.0-2 in disco armhf: universe/libdevel/extra/100% -> main
libvirglrenderer-dev 0.7.0-2 in disco i386: universe/libdevel/extra/100% -> main
libvirglrenderer-dev 0.7.0-2 in disco ppc64el: universe/libdevel/extra/100% -> main
libvirglrenderer-dev 0.7.0-2 in disco s390x: universe/libdevel/extra/100% -> main
libvirglrenderer0 0.7.0-2 in disco amd64: universe/libs/extra/100% -> main
libvirglrenderer0 0.7.0-2 in disco arm64: universe/libs/extra/100% -> main
libvirglrenderer0 0.7.0-2 in disco armhf: universe/libs/extra/100% -> main
libvirglrenderer0 0.7.0-2 in disco i386: universe/libs/extra/100% -> main
libvirglrenderer0 0.7.0-2 in disco ppc64el: universe/libs/extra/100% -> main
libvirglrenderer0 0.7.0-2 in disco s390x: universe/libs/extra/100% -> main
13 publications overridden.