[PATCH] D13403: Better handling of function calls in SafeStack

[Peter Collingbourne via llvm-commits]

On Wed, Oct 28, 2015 at 10:51:03PM +0000, Evgeniy Stepanov wrote:
> eugenis added a comment.
> 
> In http://reviews.llvm.org/D13403#275791, @pcc wrote:
> 
> > I suppose a higher level point is whether it is sufficient to rely only on the `readonly` attribute. Consider this function:
> 
> 
> It would not protect us from information leaks, but it makes write-overflow impossible at least.
> 
> > 
> 
> > 
> 
> >   void *f(void **a, int x) {
> 
> >      return a[x];
> 
> >    }
> 
> > 
> 
> > The `a` argument would be `readonly` and `nocapture`, but it could leak the address of the safe stack given an appropriate `x`. We already protect against this kind of thing within a function (via the gep case in `SafeStack::IsSafeStackAlloca`), but not across function call boundaries.
> 
> 
> In fact, this is not handled correctly within a function. For example, bitcast to larger type + load are not checked at all.

That's true.

> > Maybe we do in fact need something like a global analysis like `FunctionAttrs` that attaches `safestack` attributes to parameters.
> 
> 
> I think we will need to do something like this in the end, but let's start with something simple. I'll stop relying on "readonly" attribute, keeping just a special case for memory intrinsics.

Seems reasonable to me.

Would you mind also adding FIXMEs for the things discussed in this thread
so that we don't lost track of them, please?

> Would be great if this "safestack" attribute specified the range of memory accesses based on the parameter, so that it could describe memset(), for example.

Maybe, but we might just get away with special casing functions like memset().

Thanks,
-- 
Peter