[PATCH] Come up with --param asan-stack-small-redzone (PR sanitizer/81715).

[Martin Liška]

On 9/25/18 12:10 PM, Martin Liška wrote:
> On 9/25/18 11:24 AM, Jakub Jelinek wrote:
>> On Tue, Sep 25, 2018 at 11:05:30AM +0200, Martin Liška wrote:
>>> As requested in PR81715, GCC emits bigger middle redzones for small variables.
>>> It's analyzed in following comment: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715#c28
>>
>> First of all, does LLVM make the variable sized red zone size only for
>> automatic variables, or also for global/local statics, or for alloca?
> 
> Yes, definitely for global variables, as seen here:
> 
> lib/Transforms/Instrumentation/AddressSanitizer.cpp:
>   2122      Type *Ty = G->getValueType();
>   2123      uint64_t SizeInBytes = DL.getTypeAllocSize(Ty);
>   2124      uint64_t MinRZ = MinRedzoneSizeForGlobal();
>   2125      // MinRZ <= RZ <= kMaxGlobalRedzone
>   2126      // and trying to make RZ to be ~ 1/4 of SizeInBytes.
>   2127      uint64_t RZ = std::max(
>   2128          MinRZ, std::min(kMaxGlobalRedzone, (SizeInBytes / MinRZ / 4) * MinRZ));
>   2129      uint64_t RightRedzoneSize = RZ;
>   2130      // Round up to MinRZ
>   2131      if (SizeInBytes % MinRZ) RightRedzoneSize += MinRZ - (SizeInBytes % MinRZ);
>   2132      assert(((RightRedzoneSize + SizeInBytes) % MinRZ) == 0);
>   2133      Type *RightRedZoneTy = ArrayType::get(IRB.getInt8Ty(), RightRedzoneSize);
> 
> So roughly to SizeInBytes / 4. Confirmed:
> 
> 0x000001180a7c is located 4 bytes to the left of global variable 'b' defined in 'main.c:2:6' (0x1180a80) of size 4096
> 0x000001180a7c is located 1020 bytes to the right of global variable 'a' defined in 'main.c:1:6' (0x117f680) of size 4096
> 
> size == 4096, rz_size == 1024.

...
For allocas, red zones are constant if I see correctly.

Martin

> 
>>
>> Have you considered also making the red zones larger for very large
>> variables?
> 
> I was mainly focused on shrinking as that's limiting usage of asan-stack in KASAN.
> But I'm open for it. Would you follow what LLVM does, or do you have a specific idea how
> to growth?
> 
>>
>>> For now I'm suggesting to shrink shadow memory for variables <= 16B to 32B (including variable storage).
>>> LLVM is more aggressive as they allocate just 16B of shadow memory for variables <= 4B. That would
>>> require bigger code refactoring in asan.c and I would like to avoid that.
>>
>> What exactly would need changing to support the 12-15 bytes long red zones
>> for 4-1 bytes long automatic vars?
>> Just asan_emit_stack_protection or something other?
> 
> Primarily this function, that would need a generalization. Apart from that we're
> also doing alignment to ASAN_RED_ZONE_SIZE:
> 
> 	      prev_offset = align_base (prev_offset,
> 					MAX (alignb, ASAN_RED_ZONE_SIZE),
> 					!FRAME_GROWS_DOWNWARD);
> 
> Maybe it has consequences I don't see right now?
> 
>>
>>> +	      poly_uint64 size = stack_vars[i].size;
>>> +	      /* For small variables shrink middle redzone (including
>>> +	       * variable store) just to ASAN_RED_ZONE_SIZE.  */
>>
>> We don't use this comment style (* at start of comment continuation lines).
> 
> Sure, easy to fix.
> 
>> Otherwise it looks reasonable, but I wouldn't stop here.
> 
> First feedback is positive:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715#c30
> 
> It's questionable whether handling of variables 1-4B wide worth further changes.
> 
> Martin
> 
>>
>> 	Jakub
>>
>