This is an archive of the discontinued LLVM Phabricator instance.

[scudo] Check if MADV_DONTNEED zeroes memory
ClosedPublic

Authored by vitalybuka on Apr 22 2021, 2:03 AM.

Diff Detail

Unit TestsFailed

Event Timeline

vitalybuka created this revision.Apr 22 2021, 2:03 AM
vitalybuka requested review of this revision.Apr 22 2021, 2:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 22 2021, 2:03 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript

remove unneeded include

reorder includes

cryptoad accepted this revision.Apr 22 2021, 8:04 AM

Thank you for finding this!
LGTM with a couple of nits

compiler-rt/lib/scudo/standalone/linux.cpp
95

getPageSizeCached() maybe

113

Maybe for clarity some enum to have descriptive names associated to 0/1/2

This revision is now accepted and ready to land.Apr 22 2021, 8:04 AM
vitalybuka marked an inline comment as done.

getPageSizeCached

vitalybuka added inline comments.Apr 22 2021, 10:32 AM
compiler-rt/lib/scudo/standalone/linux.cpp
123

I believe in most cases of NoFill it's not needed.
However I could not tell if anything internal, like zeroing header, will not be broken without memset. I don't see issues other then FillContentsMode tests.

Having that we don't expect this branch running without QEMU, I guess it's fine to keep as is.

vitalybuka marked an inline comment as done.Apr 22 2021, 10:33 AM
This revision was landed with ongoing or failed builds.Apr 22 2021, 10:40 AM
This revision was automatically updated to reflect the committed changes.
Harbormaster completed remote builds in B100325: Diff 339710.
pcc added a subscriber: pcc.Apr 22 2021, 5:03 PM

Have you reported this bug to QEMU? Unfortunately the comment you linked to is inaccurate about madvise being a hint on Linux [1].

[1] https://www.youtube.com/watch?v=bg6-LVCHmGM&t=3548s

Have you reported this bug to QEMU? Unfortunately the comment you linked to is inaccurate about madvise being a hint on Linux [1].

[1] https://www.youtube.com/watch?v=bg6-LVCHmGM&t=3548s

I agree that this does not match strange but documented behavior.
Unfortunately there is a lot of comments about that online but no new/closed bug on the QEMU tracker.
Created one https://bugs.launchpad.net/qemu/+bug/1926521