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

[Walter Lee via llvm-dev]

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
_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

[Kostya Serebryany via llvm-dev]

+ more asan folks, please CC them to the code reviews. 

Also please make sure llvm-commits is CC-ed (cfe-commits for clang changes)

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. 

[Walter Lee via llvm-dev]

Thanks Kostya for the feedback.

On Tue, Oct 31, 2017 at 6:00 PM Kostya Serebryany <k...@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

[Walter Lee via llvm-dev]

I've finished my initial set of patches to make 32-byte shadow
granularity work on x86. Here is a summary of the changes from last
week:

- As discussed, I added a full redzone after every stack variable.

- We discussed adding a -fsanitize-address-granularity=N flag, but I
found the following existing flag has been sufficient for my
purposes: -asan-mapping-scale N. If anyone thinks I should add the
flag anyways, possibly replacing the latter, please let me know.

- I've modified the build so that we always run the ASan
instrumentation test suite for shadow scale values of 3 and 5.

- I've gone through the asan test suites to make them run cleanly for
both shadow scale=3 and shadow scale=5. Here are the tests I have
disabled, grouped by categories:

- Instrumentation/AddressSanitizer tests. Most tests work out of
the box, and I ported some basic tests, leaving the following:

llvm/test/Instrumentation/AddressSanitizer/lifetime-throw.ll
llvm/test/Instrumentation/AddressSanitizer/lifetime-uar-uas.ll
llvm/test/Instrumentation/AddressSanitizer/lifetime.ll
llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime-be.ll
llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
llvm/test/Instrumentation/AddressSanitizer/stack-poisoning.ll
llvm/test/Instrumentation/AddressSanitizer/stack_layout.ll

- Asm instrumentation not supported:
Instrumentation/AddressSanitizer/X86/*
compiler-rt/lib/asan/tests/asan_asm_test.cc
compiler-rt/test/asan/TestCases/Linux/asan-asm-stacktrace-test.cc

- Prelinking not supported:
compiler-rt/test/asan/TestCases/Linux/asan_prelink_test.cc

- Intra-object padding not supported:
compiler-rt/test/asan/TestCases/intra-object-overflow.cc

- Calls __asan_poison_memory_region in middle of shadow byte:
compiler-rt/lib/asan/tests/asan_interface_test.cc
SimplePoisonMemoryRegionTest
OverlappingPoisonMemoryRegionTest
PoisoningStressTest
compiler-rt/test/asan/TestCases/small_memcpy_test.cc
compiler-rt/test/asan/TestCases/strtol_strict.c
compiler-rt/test/asan/TestCases/strtoll_strict.c

- Has hardwired memory map:
compiler-rt/test/asan/TestCases/Linux/cuda_test.cc
compiler-rt/test/asan/TestCases/Linux/kernel-area.cc
compiler-rt/test/asan/TestCases/Linux/nohugepage_test.cc

- Miscellaneous:
compiler-rt/test/asan/TestCases/Linux/allocator_oom_test.cc
This test assumes amount of available memory.
compiler-rt/test/asan/TestCases/stack-buffer-overflow-with-position.cc
This fails because we don't have enough redzones to disambiguate
overflow of one stack object vs underflow of the next stack
object.

- Here is the full list of revisions. I'll add review requests
shortly.

[asan] Add CMake hook to override default shadow scale
https://reviews.llvm.org/D39469

[asan] Fix size/alignment issues with non-default shadow scale

https://reviews.llvm.org/D39470

[asan] Fix small X86_64 ShadowOffset for non-default shadow scale

https://reviews.llvm.org/D39471

[asan] Ensure that the minimum redzone is at least SHADOW_GRANULARITY

https://reviews.llvm.org/D39472

[sanitizers] Increase alignment of low level allocator

https://reviews.llvm.org/D39473

[asan] Avoid assert failure for non-default shadow scale

https://reviews.llvm.org/D39474
[asan] Add full redzone after every stack variable
https://reviews.llvm.org/D39475
[gtest] Increase stack size for child process in EXPECT_DEATH implementation
https://reviews.llvm.org/D39771
[asan] Add lit feature for custom shadow scale
https://reviews.llvm.org/D39772
[asan] Port tests to shadow scale of 5
https://reviews.llvm.org/D39773
[asan] Disable unsupported tests for custom shadow scale
https://reviews.llvm.org/D39774
[asan] Test ASan instrumentation for shadow scale value of 5
https://reviews.llvm.org/D39775

[Evgenii Stepanov via llvm-dev]

On Tue, Nov 7, 2017 at 4:50 PM, Walter Lee <wa...@google.com> wrote:
> I've finished my initial set of patches to make 32-byte shadow
> granularity work on x86. Here is a summary of the changes from last
> week:
>
> - As discussed, I added a full redzone after every stack variable.
>
> - We discussed adding a -fsanitize-address-granularity=N flag, but I
> found the following existing flag has been sufficient for my
> purposes: -asan-mapping-scale N. If anyone thinks I should add the
> flag anyways, possibly replacing the latter, please let me know.

IMO a clang flag would mean that non-standard setting for address
granularity is a supported configuration. That would require the
driver to link correct runtime library, which means we either build
two copies of libclang_rt.asan for each platform and somehow encode
the granularity value in the library name; or export that value from
instrumented code through a global, but then it stops being a
compile-time constant, and that may have effect on performance. Either
way would be an ABI break.

I think what you really want is to test shadow scale = 5 on
linux/x86_64 as a substitute for testing on the real hardware. For
that, a cmake variable in compiler-rt and an LLVM flag
(asan-mapping-scale) is more than enough.

[Walter Lee via llvm-dev]

On Wed, Nov 8, 2017 at 4:10 PM, Evgenii Stepanov <eug...@google.com> wrote:
> I think what you really want is to test shadow scale = 5 on
> linux/x86_64 as a substitute for testing on the real hardware. For
> that, a cmake variable in compiler-rt and an LLVM flag
> (asan-mapping-scale) is more than enough.

Thanks for the feedback.  I've made the changes accordingly and

uploaded them for reviews.

Walter

[Walter Lee via llvm-dev]

On Tue, Nov 7, 2017 at 7:50 PM Walter Lee <wa...@google.com> wrote:
>
> I've finished my initial set of patches to make 32-byte shadow
> granularity work on x86. Here is a summary of the changes from last
> week:

Thanks everyone for the feedback and reviews (especially Vitaly who did
the bulk of it). The patch series have been committed.