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.
- 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.
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 PRINT_OPTIONS, TGSI_PRINT_SANITY, USE_EGL_ SURFACELESS, VTEST_USE_GLES, LIBGL_ALWAYS_ SOFTWARE,
- Logging looked fine
- Uses many of its own environment variables:
GALLIUM_LOG_FILE, GALLIUM_
GALLIUM_DUMP_CPU, VIRGL_DISABLE_MT, VTEST_SAVE, VTEST_USE_GLX,
VTEST_
- 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