-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix leak checking #6386
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
Fix leak checking #6386
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative implementation would be to e.g. add some new message type to trigger leak checking here:
syzkaller/executor/executor_runner.h
Lines 612 to 621 in 65a0eec
| if (auto* msg = raw.msg.AsExecRequest()) | |
| Handle(*msg); | |
| else if (auto* msg = raw.msg.AsSignalUpdate()) | |
| Handle(*msg); | |
| else if (auto* msg = raw.msg.AsCorpusTriaged()) | |
| Handle(*msg); | |
| else if (auto* msg = raw.msg.AsStateRequest()) | |
| Handle(*msg); | |
| else | |
| failmsg("unknown host message type", "type=%d", static_cast<int>(raw.msg.type)); |
And then, in https://github.com/google/syzkaller/blob/master/pkg/rpcserver/runner.go, every X requests/Y minutes make sure there are no pending requests and start the leak checking process. Once leak checking is finished, continue fuzzing.
I am hesitating re. what'd be better here - the C++ implementation is shorter, but makes the poorly tested logic even more complicated, while the (mostly) Go implementation would be bigger than the 50 line change we have here.
07250bf to
ed8ce2c
Compare
ed8ce2c to
b04eb46
Compare
|
Otherwise looks good to me |
59a982e to
8796265
Compare
635421d to
83981ae
Compare
83981ae to
3fce097
Compare
3fce097 to
1e6dfc8
Compare
23e8226 to
a69c788
Compare
|
I think it should all work fine, but let's double-check it before merging. Please run the modified syzkaller for at least several hours on
(You can e.g. set it up overnight) We should ensure that in both cases there are no obvious syzkaller bugs, that fuzzing progresses and reproducers are found. |
This commit enables the periodic execution of a leak checker within the executor. The leak checker will now run every 2 * num_procs executions, but only after the corpus has been triaged and all executor processes are in an idle state.
At some point kmemleak started adding a CRC checksum to the "backtrace:" line in memory leak reports. The existing regular expression did not account for this, causing parsing to fail for these reports. Update the regex to make the CRC component optional, allowing reports both with and without the checksum to be parsed correctly.
This change is necessary as it allows us to access the reports printed by syz-executor, such as KMEMLEAK reports. Fixes google#4728.
KMEMLEAK now prints a crc hash. Add a test to ensure we can properly parse it.
a69c788 to
db5027c
Compare
This PR enables the periodic execution of a leak checker within the executor, it also fixes the parsing of kmemleak output.
Fixes #4728.