llvm dev - Oct 2017 - [RFC] ASan: patches to support 32-byte shadow granularity

I've prepared a preliminary set of patches that makes ASan work with
32-byte shadow granularity, and I would like to get some feedback on
those patches as well as my general plan.

Some background: I am porting ASan to the Myriad platform.  I'm
looking to break up that port into components that may be
useful/relevant to other platforms -- the first of those pieces is the
ability to use a coarser shadow granularity.  This is important for us
because Myriad has a limited amount of physical memory and no virtual
memory, so it is important to limit the amount of shadow memory
required.

My end-goal for this part is to be able to configure a build that
overrides the default shadow granularity, that can cleanly run the
clang/llvm test suite for 32-byte shadow granularity on i386/x86_64,
so we can set up buildbot for that configuration.

My basic plan:
1. Add build support to override of default shadow scale.
2. Fix various issues with 32-byte shadow granularity.
3. Propose improvements to 32-byte shadow granularity support.
4. Make test suite run cleanly for 32-byte shadow granularity.
5. Set up build bot for 32-byte shadow granularity on i386/x86_64.

My initial set of patches adds the build support and makes some
essential fixes to the compiler and run-time.  They are:

https://reviews.llvm.org/D39469 [asan] Add cmake hook to override
default shadow scale
https://reviews.llvm.org/D39470 [asan] Fix size/alignment issues with
non-default shadow scale
https://reviews.llvm.org/D39471 [asan] Fix small X86_64 ShadowOffset
for non-default shadow scale
https://reviews.llvm.org/D39472 [asan] Ensure that the minimum redzone
is at least SHADOW_GRANULARITY
https://reviews.llvm.org/D39473 [sanitizers] Increase alignment of low
level allocator
https://reviews.llvm.org/D39474 [asan] Avoid assert failure for
non-default shadow scale
https://reviews.llvm.org/D39475 [asan] Improve stack error reports for
large shadow granularity

The following features don't work yet, but can be fixed:

- i386/x86_64 assembly instrumentation.

- Prelink support.  The memory map with MidMem for this configuration
  is more complicated that that expected by compiler_rt.  The current
  HighShadow would overlap with MidMem so would need to be adjusted.
  It's not clear to me that whether is an important feature?

- Intra object overflow.  This appears to be an experimental feature,
  and I have not ported the padding insertion for different
  granularity.

Some features will not work as well with the larger shadow
granularity:

- Stack errors: it seems sensible not to insert 32-byte sentinels
  between every object, but the result is that some stack overflow
  gets reported as unknown or use-after-scope.  I have a patch that
  improves on the default behavior, but there remains cases where the
  error reports will not be as good.

- __asan_poison_memory_region is now more limited.  A typical case
  that doesn't work is the poisoning of 8-byte or 16-byte that maps to
  the middle of a shadow byte.

For testing, I have a few questions:

- Would it make sense to provide an internal compiler flag to set the
  shadow granularity, so that there we can at least run the
  instrumentation tests for 32-byte granularity in normal builds?

- Is there a reasonable subset of tests I can port to 32-byte
  granularity to provide reasonable coverage, or should I aim to port
  all tests?

For now, I've tested my changes against check-all (for default shadow
granularity), and also set up a 32-byte shadow granularity build and
manually inspected the failures to ensure that they are not
unexpected.

Thanks,

Walter

+ more asan folks, please CC them to the code reviews.
Also please make sure llvm-commits is CC-ed (cfe-commits for clang changes)

On Tue, Oct 31, 2017 at 2:29 PM, Walter Lee <waltl at google.com> wrote:
> I've prepared a preliminary set of patches that makes ASan work with
> 32-byte shadow granularity, and I would like to get some feedback on
> those patches as well as my general plan.
>
> Some background: I am porting ASan to the Myriad platform.  I'm
> looking to break up that port into components that may be
> useful/relevant to other platforms -- the first of those pieces is the
> ability to use a coarser shadow granularity.  This is important for us
> because Myriad has a limited amount of physical memory and no virtual
> memory, so it is important to limit the amount of shadow memory
> required.
>
> My end-goal for this part is to be able to configure a build that
> overrides the default shadow granularity, that can cleanly run the
> clang/llvm test suite for 32-byte shadow granularity on i386/x86_64,
> so we can set up buildbot for that configuration.
>
> My basic plan:
> 1. Add build support to override of default shadow scale.
> 2. Fix various issues with 32-byte shadow granularity.
> 3. Propose improvements to 32-byte shadow granularity support.
> 4. Make test suite run cleanly for 32-byte shadow granularity.
> 5. Set up build bot for 32-byte shadow granularity on i386/x86_64.
>
> My initial set of patches adds the build support and makes some
> essential fixes to the compiler and run-time.  They are:
>
> https://reviews.llvm.org/D39469 [asan] Add cmake hook to override
> default shadow scale
> https://reviews.llvm.org/D39470 [asan] Fix size/alignment issues with
> non-default shadow scale
> https://reviews.llvm.org/D39471 [asan] Fix small X86_64 ShadowOffset
> for non-default shadow scale
> https://reviews.llvm.org/D39472 [asan] Ensure that the minimum redzone
> is at least SHADOW_GRANULARITY
> https://reviews.llvm.org/D39473 [sanitizers] Increase alignment of low
> level allocator
> https://reviews.llvm.org/D39474 [asan] Avoid assert failure for
> non-default shadow scale
> https://reviews.llvm.org/D39475 [asan] Improve stack error reports for
> large shadow granularity
>
> The following features don't work yet, but can be fixed:
>
> - i386/x86_64 assembly instrumentation.
>
That's fine. It doesn't really work in regular mode either.
>
> - Prelink support.  The memory map with MidMem for this configuration
>   is more complicated that that expected by compiler_rt.  The current
>   HighShadow would overlap with MidMem so would need to be adjusted.
>   It's not clear to me that whether is an important feature?
>
you may safely ignore this for 32-bit granularity, but please keep it
working in regular mode.

>
> - Intra object overflow.  This appears to be an experimental feature,
>   and I have not ported the padding insertion for different
>   granularity.
>
Yep, ignore it.
>
> Some features will not work as well with the larger shadow
> granularity:
>
> - Stack errors: it seems sensible not to insert 32-byte sentinels
>   between every object, but the result is that some stack overflow
>   gets reported as unknown or use-after-scope.  I have a patch that
>   improves on the default behavior, but there remains cases where the
>   error reports will not be as good.
>
Hmm. Not sure what's the problem here. It's totally fine to insert
32-byte
redzone around stack objects.
(in 32-byte granularity mode)

>
> - __asan_poison_memory_region is now more limited.  A typical case
>   that doesn't work is the poisoning of 8-byte or 16-byte that maps to
>   the middle of a shadow byte.
>
yep.

>
> For testing, I have a few questions:
>
> - Would it make sense to provide an internal compiler flag to set the
>   shadow granularity, so that there we can at least run the
>   instrumentation tests for 32-byte granularity in normal builds?
>
I'd prefer a proper flag, like -fsanitize-address-granularity=N (8,16,32)

>
> - Is there a reasonable subset of tests I can port to 32-byte
>   granularity to provide reasonable coverage, or should I aim to port
>   all tests?
>
Let's see what tests won't work out of the box and decide.
We can mark all failing tests as
   UNSUPPORTED: 32-bit-granularity
but ideally we shouldn't have to mark too many of those.

>
> For now, I've tested my changes against check-all (for default shadow
> granularity), and also set up a 32-byte shadow granularity build and
> manually inspected the failures to ensure that they are not
> unexpected.
>
> Thanks,
>
> Walter
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171031/fdfc6c5b/attachment.html>

Thanks Kostya for the feedback.

On Tue, Oct 31, 2017 at 6:00 PM Kostya Serebryany <kcc at google.com>
wrote:
>
>> - Stack errors: it seems sensible not to insert 32-byte sentinels
>>   between every object, but the result is that some stack overflow
>>   gets reported as unknown or use-after-scope.  I have a patch that
>>   improves on the default behavior, but there remains cases where the
>>   error reports will not be as good.
>>
>
> Hmm. Not sure what's the problem here. It's totally fine to insert
32-byte
> redzone around stack objects.
> (in 32-byte granularity mode)
>
I was concerned about stack overhead, but I will go with your suggestion
for now, and revisit when I have more data.
>
>> - Would it make sense to provide an internal compiler flag to set the
>>   shadow granularity, so that there we can at least run the
>>   instrumentation tests for 32-byte granularity in normal builds?
>>
>
> I'd prefer a proper flag, like -fsanitize-address-granularity=N
(8,16,32)
>
Ok I'll do that.

>
>>
>> - Is there a reasonable subset of tests I can port to 32-byte
>>   granularity to provide reasonable coverage, or should I aim to port
>>   all tests?
>>
>
> Let's see what tests won't work out of the box and decide.
> We can mark all failing tests as
>    UNSUPPORTED: 32-bit-granularity
> but ideally we shouldn't have to mark too many of those.
>
I will write up a classification of the failures.   There are many tests
that fail because they assume the shadow granularity; I will propose fixes
for them.

Thanks,

Walter
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171101/0e52ebdc/attachment.html>