[llvm-dev] Fixing some StackProtector issues

via llvm-dev llvm-dev at lists.llvm.org
Thu Sep 19 07:36:38 PDT 2019


PR43308 describes a case where StackProtector fails to protect against
a fairly simple smash.  This problem started after r363169, which
removed StackProtector's own analysis function HasAddressTaken, and
used CaptureTracking's PointerMayBeCaptured instead.  The problem here
is that "pointer is captured" and "pointer could be used to smash the
stack" are not equivalent queries.  The header of CaptureTracking.cpp
says in part:
  A pointer value is captured if the function makes a copy of any part
  of the pointer that outlives the call.
Clearly, stacks can be smashed without a "capture" occurring; a smash
simply uses a pointer in a normal-looking way, but just oversteps the
bounds of the pointed-to data.

r363169 was intended to fix PR42238, so undoing the patch will have to
come up with some other fix for PR42238.  Also, there's PR40436, which
identified an infinite recursion in HasAddressTaken; there's D57149 to
fix that, which was abandoned as irrelevant after r363169.

Finally, aside from just fixing PR42238, the commit log for r363169
also notes that it "fixes some infrastructure issues to allow running
just the IR pass."  That fix should be preserved.

Overall, I think the appropriate measures are:

1. Revert r363169, which fixes PR43308 but re-introduces PR42238 and
   PR40436.
2. Re-add the "infrastructure" fix from r363169.
3. Fix PR42238 by enhancing HasAddressTaken to look at new instructions.
4. Fix PR40436, by reviving D57149.

I'll start working on a patch series for the first 3 items; the
original author of D57149 can decide about reviving that one.

--paulr



More information about the llvm-dev mailing list