Page MenuHomePhabricator

[asan] Fix internal CHECK failure on double free in recovery mode.
ClosedPublic

Authored by m.ostapenko on Dec 29 2015, 6:11 AM.

Details

Summary

As reported in https://github.com/google/sanitizers/issues/639, currently ASan fails with internal CHECK in allocator on double free in recovery mode. Although there isn't reasonable expected behavior on double free (recent Glibc aborts in this case), we still want to proceed execution in hope to find more bugs (of course, without corrupting ASan internal invariants).

This patch tries to overcome the issue by adding return after ReportInvalidFree to avoid CHECK failure and pushing freed chunk into quarantine only if has CHUNK_ALLOCATED status.

Diff Detail

Repository
rL LLVM

Event Timeline

m.ostapenko retitled this revision from to [asan] Fix internal CHECK failure on double free in recovery mode..
m.ostapenko updated this object.
m.ostapenko added reviewers: samsonov, eugenis.
m.ostapenko set the repository for this revision to rL LLVM.
m.ostapenko added a project: lld.
m.ostapenko added subscribers: ygribov, llvm-commits.

I'm not sure upstream people will take this but probably still makes sense to publish FTR.

I'm not sure upstream people will take this but probably still makes sense to publish FTR.

Yeah, perhaps this looks dirty. But still, failing with CHECK error looks a bit scary for users, perhaps we could just exit after ReportInvalidFree call if continuing execution doesn't look reasonable?

But still, failing with CHECK error looks a bit scary for users

In normal mode the CHECK will never happen. More user-friendly error handling is needed only by recovery mode which is kind of a second class citizen.

kcc added a subscriber: kcc.Jan 4 2016, 5:48 PM

I would prefer a force exit in ReportInvalidFree, just because this is simpler.
In either case, test please.

Max, what about test?

m.ostapenko added a reviewer: kcc.
m.ostapenko removed a project: lld.

Added testcase for the bug. I didn't updated the patch itself because we still want continue execution after double free was detected. Can this approach be acceptable for recovery mode?

kcc accepted this revision.Feb 1 2016, 2:14 PM
kcc edited edge metadata.

LGTM with a nit

lib/asan/asan_allocator.cc
461 ↗(On Diff #45986)

s/ones/chunks

This revision is now accepted and ready to land.Feb 1 2016, 2:14 PM
This revision was automatically updated to reflect the committed changes.