New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 852111 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature

Blocked on:
issue 852182
issue 852942
issue 862699
issue 866546
issue 868644



Sign in to add a comment

Create fuzzer for virglrenderer

Project Member Reported by davidri...@chromium.org, Jun 12 2018

Issue description

Create fuzzer for virglrenderer suitable for use with ClusterFuzz
 
I've posted an initial version which just passes the data buffer directly to the submit cmd (https://chromium-review.googlesource.com/c/chromiumos/overlays/portage-stable/+/1096537).  It's finding a few things (in particular, https://bugs.chromium.org/p/chromium/issues/detail?id=852182 shows up repeatedly).

I'm still deciding between where it's worth to do more targeting fuzzing of the submit cmd instead of just passing in the random buffer as is currently being done, or to branch out and cover other functions so I'm likely going to try and figure out how to view coverage.
Thanks for the update David! The Chrome browser Bugs-- folks (Abhishek and Jonathan) would likely have the best input regarding coverage.
Blockedon: 852942 852182
Moved the ebuild to chromiumos-overlay (chain ending at https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/1100122/2) and currently waiting for a new virglrenderer repo to be created (crbug.com/852942).

Attached is a coverage report (interesting file is vrend_decode.c) from directly passing random data to the virgl_renderer_submit_cmd, with three functions which cause issues stubbed out (vrend_set_single_sampler_view & vrend_set_num_sampler_views  crbug.com/852182 , vrend_set_sample_mask) via the following commands:
ASAN_OPTIONS="halt_on_error=false:log_path=stderr" /usr/libexec/fuzzers/virgl_fuzzer -runs=1000000 CORPUS
llvm-profdata merge -sparse *.profraw -o default.profdata
llvm-cov show -instr-profile=default.profdata /build/amd64-generic/usr/lib64/libvirglrenderer.so

The decode gets down through the first level dispatch, but then usually fails the next size checks:  827|       |static int vrend_decode_set_polygon_stipple(struct vrend_decode_ctx *ctx, int length)
  828|     39|{
  829|     39|   struct pipe_poly_stipple ps;
  830|     39|   int i;
  831|     39|
  832|     39|   if (length != VIRGL_POLYGON_STIPPLE_SIZE)
  833|     39|      return EINVAL;
  834|      0|
  835|      0|   for (i = 0; i < 32; i++)
  836|      0|      ps.stipple[i] = get_buf_entry(ctx, VIRGL_POLYGON_STIPPLE_P0 + i);
  837|      0|
  838|      0|   vrend_set_polygon_stipple(ctx->grctx, &ps);
  839|      0|   return 0;
  840|      0|}

I think running longer would cover these cases eventually, but the entire fuzzer usually fails due to running out of memory due to not cleaning up after each run.  Not cleaning up lets the fuzzer run much much more quickly because GL doesn't need to be initialized each time, but runs into these issues since things are allocated but not freed.  The startup costs might not be an issue on ClusterFuzz due to parallelism and a saved corpus, and I am also going to explore tearing down things regularly.  Maintaining the state is potentially a bad idea anyways due to reduced reproducibility if failures happen to rely on previous runs.

Structuring data as jorgelo@ suggested is another option, but that will make the code harder to upstream due to adding additional dependencies on C++ and libchrome and I'm still hopeful that full random data might work with submit_cmd once the kinks are worked out.  I think eventually we'll need to go down that path for fuzzing other APIs however so maybe those shouldn't be taken into consideration as drawbacks.
virgl_fuzzer_coverage-20180615.txt
2.3 MB View Download
As an update before I take off on vacation.

All of my current changes are going through CQ right now.  These changes should be sufficient for anyone to replicate on their desktop, but is not sufficient to run on ClusterFuzz.  In particular, the current set of changes uses EGL which fails when eglInitialize() is called due to /dev/dri (DRM) not being available on GCE and through minijail.

I tried getting surfaceless (https://www.khronos.org/registry/EGL/extensions/MESA/EGL_MESA_platform_surfaceless.txt) going, but that appears to still require EGL initialization.

I tried GLX w/ Xvfb but that fails due to glXChooseFBConfig() failing on Xvfb since it's not supported on unaccelerated X servers.  Xdummy might be an option I haven't explored yet.

I'm currently trying rebuilding mesa with --enable-glx=gallium-xlib for client-side glx but running into mesa/ebuild configuration option conflicts.  Specifically:
mesa: --enable-glx=gallium-xlib conflicts with --enable-dri
mesa: --enable-dri required for egl
ebuild: egl required by virtual/opengles and x11-libs/libva

Comment 5 Deleted

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/aaef58641180bd9d879da05e99a87cd851ebbb50

commit aaef58641180bd9d879da05e99a87cd851ebbb50
Author: David Riley <davidriley@chromium.org>
Date: Thu Jun 21 19:17:59 2018

virglrenderer: Create fuzzer.

Initially fuzzes virgl_renderer_submit_cmd with random data directly
passed in.

BUG= chromium:852111 
TEST=USE="asan fuzzer" emerge-amd64-genric virglrenderer; LIBGL_ALWAYS_SOFTWARE=true ASAN_OPTIONS="log_path=stderr" LD_LIBRARY_PATH=/build/amd64-generic/usr/lib64 LIBGL_DRIVERS_PATH=/build/amd64-generic/usr/lib64/dri /build/amd64-generic/usr/libexec/fuzzers/virgl_fuzzer
TEST=USE="asan fuzzer" emerge-amd64-genric virglrenderer; chromite/bin/cros_fuzz_test_env --board amd64-generic; sudo chroot chroot/build/amd64-generic; /usr/libexec/fuzzers/virgl_fuzzer

Change-Id: I50dc6567457ec59bb96086765c8151305f164841
Reviewed-on: https://chromium-review.googlesource.com/1100067
Commit-Ready: David Riley <davidriley@chromium.org>
Tested-by: David Riley <davidriley@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>

[add] https://crrev.com/aaef58641180bd9d879da05e99a87cd851ebbb50/media-libs/virglrenderer/files/virglrenderer-0.6.0-fuzzer.patch
[modify] https://crrev.com/aaef58641180bd9d879da05e99a87cd851ebbb50/media-libs/virglrenderer/virglrenderer-9999.ebuild
[modify] https://crrev.com/aaef58641180bd9d879da05e99a87cd851ebbb50/media-libs/virglrenderer/virglrenderer-0.6.0_p20180615.ebuild
[add] https://crrev.com/aaef58641180bd9d879da05e99a87cd851ebbb50/media-libs/virglrenderer/files/fuzzer-OWNERS

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/e2577be4b3d728cfb6aa4a24cd38e757daa8781b

commit e2577be4b3d728cfb6aa4a24cd38e757daa8781b
Author: David Riley <davidriley@chromium.org>
Date: Sat Jun 23 00:22:15 2018

virglrenderer: Add coverage options when USE=profiling specified.

BUG= chromium:852111 
TEST=USE="asan fuzzer profiling" emerge-amd64-genric virglrenderer; LIBGL_ALWAYS_SOFTWARE=true ASAN_OPTIONS="log_path=stderr" LD_LIBRARY_PATH=/build/amd64-generic/usr/lib64 LIBGL_DRIVERS_PATH=/build/amd64-generic/usr/lib64/dri /build/amd64-generic/usr/libexec/fuzzers/virgl_fuzzer; llvm-profdata merge -sparse *.profraw -o default.profdata; llvm-cov show /build/amd64-generic/usr/libexec/fuzzers/virgl_fuzzer -instr-profile=default.profdata -name=virgl_fuzzer

Change-Id: I1a9d5823330e09537e58ce783b79c6499dea64e2
Reviewed-on: https://chromium-review.googlesource.com/1100122
Commit-Ready: Pohsien Wang <pwang@chromium.org>
Tested-by: Pohsien Wang <pwang@chromium.org>
Reviewed-by: Pohsien Wang <pwang@chromium.org>

[modify] https://crrev.com/e2577be4b3d728cfb6aa4a24cd38e757daa8781b/media-libs/virglrenderer/virglrenderer-9999.ebuild
[modify] https://crrev.com/e2577be4b3d728cfb6aa4a24cd38e757daa8781b/media-libs/virglrenderer/virglrenderer-0.6.0_p20180615.ebuild

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/1c9ecfc6bf9eddc9e830531bc4e543392675abe1

commit 1c9ecfc6bf9eddc9e830531bc4e543392675abe1
Author: Po-Hsien Wang <pwang@chromium.org>
Date: Wed Jun 27 06:44:17 2018

xwayland: allow minimal built.

allow minimal built. This CL is part port from xorg-server config.

BUG= chromium:852111 
TEST=None

Change-Id: I3705d8476f7388f9edf144345ce6adb53d5a4bad
Reviewed-on: https://chromium-review.googlesource.com/1115474
Commit-Ready: Pohsien Wang <pwang@chromium.org>
Tested-by: Pohsien Wang <pwang@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>

[modify] https://crrev.com/1c9ecfc6bf9eddc9e830531bc4e543392675abe1/x11-base/xwayland/xwayland-1.19.5-r4.ebuild

Blockedon: 862699
I think I have a line on getting things going with cluster fuzz with change to the fuzzer to cleanup after each run and a couple of mesa changes I'm planning to send upstream (hopefully tomorrow):
1. allow a surfaceless path to work without DRM
2. fix a mesa llvm memory leak which occurs with each dlopen()/dlclose() (and consequently each eglInitialize()/eglTerminate())

There's still a slight memory leak with dlopen()/dlclose() from LLVM that is minimal enough that it shouldn't cause OOM very often, but will need to be fixed/suppressed still.  Example lsan suppression: https://cs.chromium.org/chromium/src/third_party/skia/tools/LsanSuppressions.cpp?q=suppressions+file:%5Esrc/third_party/skia/+package:%5Echromium$&dr=CSs&l=19
Cc: -puneetster@chromium.org puneets...@chromium.orgm
Here's a result from ClusterFuzz which matches what was found manually ( crbug.com/864695 ):
https://paste.googleplex.com/4913420059017216

This build was locally made and includes:
- mesa changes to support DRMless surfaceless: chain ending at https://chromium-review.googlesource.com/c/chromiumos/third_party/mesa/+/1136584 ; sent upstream
- virglrender changes to do some reinit with each input and update to upstream to address memory leaks: chain ending at https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/1140907/1 ; fuzzer sent upstream
Cc: -puneets...@chromium.orgm puneetster@chromium.org metzman@chromium.org
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/d1d15cd1ef9d9dda928313dcb94f6bc1d25e0b50

commit d1d15cd1ef9d9dda928313dcb94f6bc1d25e0b50
Author: Po-Hsien Wang <pwang@chromium.org>
Date: Tue Jul 24 04:05:04 2018

chromium-os-fuzzers: Add virglrenderer

BUG= chromium:852111 
TEST=emerge-amd64-generic chromium-os-fuzzers
CQ-DEPEND=CL:1100122, CL:1140741

Change-Id: Ibedd3609f375a40fdc82e50aee553ed12668b3b4
Reviewed-on: https://chromium-review.googlesource.com/1105572
Commit-Ready: Pohsien Wang <pwang@chromium.org>
Tested-by: Pohsien Wang <pwang@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Pohsien Wang <pwang@chromium.org>

[modify] https://crrev.com/d1d15cd1ef9d9dda928313dcb94f6bc1d25e0b50/virtual/chromium-os-fuzzers/chromium-os-fuzzers-1.ebuild
[rename] https://crrev.com/d1d15cd1ef9d9dda928313dcb94f6bc1d25e0b50/virtual/chromium-os-fuzzers/chromium-os-fuzzers-1-r8.ebuild

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/7f14bdc94d31898de4ff8b6c2d6cada3c009250a

commit 7f14bdc94d31898de4ff8b6c2d6cada3c009250a
Author: Pohsien Wang <pwang@chromium.org>
Date: Thu Jul 26 13:29:12 2018

Revert "chromium-os-fuzzers: Add virglrenderer"

This reverts commit d1d15cd1ef9d9dda928313dcb94f6bc1d25e0b50.

Reason for revert: The builder start failing in the build stage. Temporary revert this change so that others are unblocked.
BUG= chromium:867022 

Original change's description:
> chromium-os-fuzzers: Add virglrenderer
>
> BUG= chromium:852111 
> TEST=emerge-amd64-generic chromium-os-fuzzers
> CQ-DEPEND=CL:1100122, CL:1140741
>
> Change-Id: Ibedd3609f375a40fdc82e50aee553ed12668b3b4
> Reviewed-on: https://chromium-review.googlesource.com/1105572
> Commit-Ready: Pohsien Wang <pwang@chromium.org>
> Tested-by: Pohsien Wang <pwang@chromium.org>
> Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
> Reviewed-by: Pohsien Wang <pwang@chromium.org>

Bug:  chromium:852111 
Change-Id: I5487efb7c6615043af8febf73485bff9b712e083
Reviewed-on: https://chromium-review.googlesource.com/1150980
Commit-Ready: Pohsien Wang <pwang@chromium.org>
Tested-by: Pohsien Wang <pwang@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Pohsien Wang <pwang@chromium.org>

[modify] https://crrev.com/7f14bdc94d31898de4ff8b6c2d6cada3c009250a/virtual/chromium-os-fuzzers/chromium-os-fuzzers-1.ebuild
[rename] https://crrev.com/7f14bdc94d31898de4ff8b6c2d6cada3c009250a/virtual/chromium-os-fuzzers/chromium-os-fuzzers-1-r7.ebuild

Project Member

Comment 16 by bugdroid1@chromium.org, Jul 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/02c8bd18a6cecea1cb0b8acff9dba529454f89d0

commit 02c8bd18a6cecea1cb0b8acff9dba529454f89d0
Author: David Riley <davidriley@chromium.org>
Date: Sat Jul 28 05:33:19 2018

virglrenderer: Update to upstream, remove local fuzzer patch.

From: 0fb73b11e4cdadced885e52848002b2e9c79e3f5
To: 9c420d224d86215d408dff8dea599ed9414a24d6

9c420d2 vrend, caps: Move GL only caps into newly created function
f4ac4c6 vrend, caps: Move the sanity checks up in the call hierarchy
60521af vrend, caps: Split GL/GLES version checking and move caps set check up
97ddb62 vrend: remove superfluous initializations
c2e457e vrend: correct the stride if the client sends it
cd14ff1 renderer: Protect glSampleMaski and GL_SAMPLE_MASK.
519a091 shader_buffers: fix macros and use in decode.
42e2a4c vrend: use the row-stride when directly reading back to an IOV
34809ef vrend: Set scissor_state_dirty correctly.
58e521c get rid of yet another bind-flag set
2e84388 discourage using legacy-definitions
dc1bc1e get rid of diplicate definition of VREND_RES_BIND-flags
5ff40d5 add VIRGL_BIND_*-flags from mesa
9eaf2c8 features: disallow ssbos if we don't have the feature (v2)
6a6f3c4 features: add ubo feature (v2)
7958225 features: add transform_feedback feature (v2)
4145714 features: add multisample texture feature.
dd2f62b features: add cube map array feature.
c8d3c59 features: add texture array feature
4593bef features: add conditional render inverted.
0dc96e9 features: add transform feedback overflow query
ea7f3c1 features: add geometry shader feature
e8eeea7 features: add dual src blend support
1497dd9 features: add viewport array feature
4ed679c features: use correct extensions for tbo size
2704d81 features: add independent blend function feature.
c402e82 features: add indirect draw feature.
35356ec features: add independent blend enable feature
36ac335 features: add base instance feature.
7c23f33 features: add draw_instance feature.
c8269ae renderer: get return value from draw vbo.
31049f6 features: move existing features to a table init (v2)
87d8671 features: add transform_feedback3 feature
6ff41a3 features: move some caps to use has_feature flags
edd2478 Fix create_shader buf boundary check
fe7a1ef gles: report maximum vertex-attrib stride to guest
e898b8f Avoid needless repetition
2c0d096 use short-hand state accessors
7a37a36 Fix NULL dereference in vrend_draw_bind_samplers_shader
87b346a fuzzer: Add a libFuzzer based fuzzer.
5057fb9 tests: Fix virgl_init_cbs_wrong_ver test
79479ac blitter, GL blit fallback: clean up framebuffer after use
eb9555c features: convert current feature list to an array (v2)
cdf8860 renderer: fix ambiguous else warning
97b9df0 add a cap for TGSI precise modifiers
47387e4 emit precise keyword
ef70cef tgsi/text: parse _PRECISE modifier
46d2cf8 tgsi: populate precise
654647c protect calls to glPrimitiveRestart on GLES 3.1
39add38 protect gl{Begin, End}ConditionalRendering calls
4349893 protect call to glDeleteSamplers
89f7995 protect call to glPrimitiveRestartIndex
ec454b9 renderer: fix ssbo != -1 comparison.
df7322e ssbo: reorder var assignment
083d97f renderer: add shader_storage_buffer_object support. (v4)
1800bd4 shader: add basic shader_storage_buffer_object parsing. (v4)
4013fbc gallium: import MAX_SHADER_BUFFERS from mesa
dfa1e8c u_math: bring over u_bit_scan_consecutive_range.
7f96206 shader: drop unused sviews_used
249fb00 shader: pass sinfo/dinfo into translate_tex
a04a63e virgl-caps: Report support for GL_ARB_copy_image to the guest
8ad0201 vrend_formats: Replace RGB(8|16) formats with RGBX(8|16)
2846dcf vrend: If available use glCopyImageSubData to execute memcopy like blits
be3b107 vrend: Remove bad sRGB warning on GLES
cae96e1 shader: drop unused function.
e387116 report maximum vertex-attrib stride to host
6a4ef6d renderer: swizzle sampler border color channel if we emulate alpha format

BUG= chromium:852111 , chromium:864689 , chromium:862699 , chromium:864695 , chromium:864792 
TEST=ASAN_OPTIONS="log_path=stderr" /usr/libexec/fuzzers/virgl_fuzzer

Change-Id: I6e9b40675053dc1f18af6dfd888a145caecf13b7
Reviewed-on: https://chromium-review.googlesource.com/1153607
Commit-Ready: Manoj Gupta <manojgupta@chromium.org>
Tested-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Pohsien Wang <pwang@chromium.org>

[delete] https://crrev.com/6c1c4a5360e6a3a124cc64ebdc22943fbeb8211a/media-libs/virglrenderer/files/virglrenderer-0.6.0-fuzzer.patch
[delete] https://crrev.com/6c1c4a5360e6a3a124cc64ebdc22943fbeb8211a/media-libs/virglrenderer/virglrenderer-0.6.0_p20180716-r2.ebuild
[modify] https://crrev.com/02c8bd18a6cecea1cb0b8acff9dba529454f89d0/media-libs/virglrenderer/Manifest
[rename] https://crrev.com/02c8bd18a6cecea1cb0b8acff9dba529454f89d0/media-libs/virglrenderer/virglrenderer-0.6.0_p20180727.ebuild

Blockedon: 866546 868644
Project Member

Comment 18 by bugdroid1@chromium.org, Aug 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/b2bda079a5606d767b2a591f740fd9d9322ac754

commit b2bda079a5606d767b2a591f740fd9d9322ac754
Author: David Riley <davidriley@chromium.org>
Date: Wed Aug 01 07:05:29 2018

media-libs/mesa: Patches to support ClusterFuzz of virgl.

These patches posted upstream allow mesa to be started without
DRM which is necessary for running the virgl fuzzer in ClusterFuzz
from within its minijail.

BUG= chromium:852111 
TEST=ASAN_OPTIONS="log_path=stderr" /usr/libexec/fuzzers/virgl_fuzzer

Change-Id: Icfeec019b03cde8280c9b23d5f4a698eae9711f7
Reviewed-on: https://chromium-review.googlesource.com/1153992
Commit-Ready: David Riley <davidriley@chromium.org>
Tested-by: David Riley <davidriley@chromium.org>
Reviewed-by: David Riley <davidriley@chromium.org>

[add] https://crrev.com/b2bda079a5606d767b2a591f740fd9d9322ac754/media-libs/mesa/files/18.1-egl-surfaceless-swrastloader.patch
[modify] https://crrev.com/b2bda079a5606d767b2a591f740fd9d9322ac754/media-libs/mesa/mesa-18.2_pre1.ebuild
[rename] https://crrev.com/b2bda079a5606d767b2a591f740fd9d9322ac754/media-libs/mesa/mesa-18.2_pre1-r11.ebuild
[add] https://crrev.com/b2bda079a5606d767b2a591f740fd9d9322ac754/media-libs/mesa/files/18.1-egl-surfaceless-drmless.patch
[modify] https://crrev.com/b2bda079a5606d767b2a591f740fd9d9322ac754/media-libs/mesa/mesa-9999.ebuild

Status: Fixed (was: Assigned)

Sign in to add a comment