Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce the severity of HWASAN not being supported by sanitizer.py #3848

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ParisMeuleman
Copy link
Contributor

@jonathanmetzman noticed a spike in these errors, we are tracking the action of fixing the option handling in https://issues.chromium.org/u/2/issues/329346758, meanwhile, we can reduce the severity of the log.

I am not sure of the actual fix, the HWAsan documentation is a bit lacking. It seems we might be able to set those as an environment variable in a wrap.sh. Can/should this be done by clusterfuzz is to be determined.

Engine fuzzers are not uzing this (most of android fuzzing IIUC?)

sanitizer.py doesn't support writing hwasan options, this spams the logs heavily, crbug.com/329346758.
I will follow up through the bugs in properly setting these options.
@jonathanmetzman
Copy link
Collaborator

/gcbrun

@jonathanmetzman
Copy link
Collaborator

I don't have a great understanding of this some I'm a little worried about turning this off for the Android team's ClusterFuzz instance.

@ParisMeuleman
Copy link
Contributor Author

I don't have a great understanding of this some I'm a little worried about turning this off for the Android team's ClusterFuzz instance.

makes sense, I guess involving @marktefftech is our best course of action?

FYI regarding the follow up, I'm testing adding the HWASAN_OPTIONS through a wrap.sh in the apk. I had some success but still need to test some more.

@marktefftech
Copy link
Collaborator

This logging change LGTM

As far as a long-term solution, it would be nice to follow the existing convention of setting the ADDITIONAL_<sanitizer>_OPTIONS.

This will keep things the same for hwasan as they are for other sanitizers. We can set the options via configuration, for example, using the ClusterFuzz job definitions.

Take the engine_asan template as an example, which sets ADDITIONAL_ASAN_OPTIONS = quarantine_size_mb=64.......

@ParisMeuleman
Copy link
Contributor Author

This logging change LGTM

As far as a long-term solution, it would be nice to follow the existing convention of setting the ADDITIONAL_<sanitizer>_OPTIONS.

This will keep things the same for hwasan as they are for other sanitizers. We can set the options via configuration, for example, using the ClusterFuzz job definitions.

Take the engine_asan template as an example, which sets ADDITIONAL_ASAN_OPTIONS = quarantine_size_mb=64.......

So using the additional_hwasan_options sounds good. Our hwasan job actually had some (but mislabelled).
The issue here is that we are not using engine fuzzers, which as I understand have their own way of setting options. Instead we have a blackbox fuzzer where we:

  • generate files (fuzzer's job)
  • serve files over HTTP[s], with port forwarding to the host
  • open chrome (a hwasan build) on one of those file
  • observe the result
  • repeat

Opening Chrome with those hwasan options set is the challenge:

  • We have the option to add a wrap.sh script in the APK, but this means the options need to be decided at build time.
  • Have a wrap.sh set using adb shell setprop wrap.$PACKAGE_NAME. This seems more flexible and seems to work with my local tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants