This is an archive of the discontinued LLVM Phabricator instance.

[DFSan] Add force_zero_label abilist option to DFSan. This can be used as a work-around for overtainting.
ClosedPublic

Authored by browneee on Sep 15 2021, 1:02 PM.

Diff Detail

Event Timeline

browneee created this revision.Sep 15 2021, 1:02 PM
browneee requested review of this revision.Sep 15 2021, 1:02 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 15 2021, 1:02 PM
Herald added subscribers: llvm-commits, Restricted Project. · View Herald Transcript

What's the motivation for this feature?

I think discard already returns 0 labels. When do we also need to store 0 labels?

compiler-rt/test/dfsan/Inputs/force_zero_abilist.txt
1 ↗(On Diff #372780)

Why a separate abilist.txt?

compiler-rt/test/dfsan/force_zero.c
26

Wouldn't this also pass if we used uninstrumented or discard in the ABI list?

The motivation for this change is to remove taint in functions which write out their return data...

e.g.

void GenerateData(char* out_buf, int out_buf_len) { ... }

This feature allows us to untaint the data produced by this function.

compiler-rt/test/dfsan/Inputs/force_zero_abilist.txt
1 ↗(On Diff #372780)

For separate test. Happy to combine them if you prefer.

compiler-rt/test/dfsan/force_zero.c
26

For the return value yes, but not for the shadow of the contents of the the out pointer.

compiler-rt/test/dfsan/Inputs/force_zero_abilist.txt
1 ↗(On Diff #372780)

Yes, let's combine them for simplicity.

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
149

Nit: Could we make the distinction between uninstrumented and instrumented functions a little clearer?

I think we only use functional and discard with uninstrumented, while we only use force_zero_labels with instrumented.

1218

For consistency.

llvm/test/Instrumentation/DataFlowSanitizer/Inputs/force_zero_abilist.txt
1 ↗(On Diff #372780)

Let's also reuse the existing ABI list for llvm tests.

llvm/test/Instrumentation/DataFlowSanitizer/force_zero.ll
10

Let's check that 0 is being stored specifically to SHADOW_PTR.

browneee updated this revision to Diff 373041.Sep 16 2021, 1:00 PM
browneee marked 7 inline comments as done.

Address review comments.

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
152
155
Herald added a project: Restricted Project. · View Herald TranscriptSep 16 2021, 2:12 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
morehouse accepted this revision.Sep 17 2021, 8:42 AM
morehouse added inline comments.
clang/docs/DataFlowSanitizer.rst
141 ↗(On Diff #373065)
This revision is now accepted and ready to land.Sep 17 2021, 8:42 AM
browneee updated this revision to Diff 373274.Sep 17 2021, 10:21 AM
browneee marked 3 inline comments as done.

Update comments.

This revision was landed with ongoing or failed builds.Sep 17 2021, 12:58 PM
This revision was automatically updated to reflect the committed changes.