binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Dmitry Vyukov]

Hello,

The recent "binfmt_elf: use ELF_ET_DYN_BASE only for PIE" patch:
https://github.com/torvalds/linux/commit/eab09532d40090698b05a07c1c87f39fdbc5fab5
breaks user-space AddressSanitizer. AddressSanitizer makes assumptions
about address space layout for substantial performance gains. There
are multiple people complaining about this already:
https://github.com/google/sanitizers/issues/837
https://twitter.com/kayseesee/status/894594085608013825
https://bugzilla.kernel.org/show_bug.cgi?id=196537
AddressSanitizer maps shadow memory at [0x00007fff7000-0x10007fff7fff]
expecting that non-pie binaries will be below 2GB and pie
binaries/modules will be at 0x55 or 0x7f. This is not the first time
kernel address space shuffling breaks sanitizers. The last one was the
move to 0x55.

Is it possible to make this change less aggressive and keep the
executable under 2GB?

In future please be mindful of user-space sanitizers and talk to
address-sanitizer@googlegroups.com before shuffling address space.

Thanks

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kostya Serebryany]

[-- Attachment #1: Type: text/plain, Size: 1301 bytes --]

+Jakub Jelinek, who helped us migrate asan's shadow from high addresses
to 0x7fff7000 for a significant ~5% performance and code size gain.
(a few years ago)

--kcc

On Mon, Aug 7, 2017 at 10:24 AM, Dmitry Vyukov <dvyukov@google.com> wrote:

> Hello,
>
> The recent "binfmt_elf: use ELF_ET_DYN_BASE only for PIE" patch:
> https://github.com/torvalds/linux/commit/eab09532d40090698b05a07c1c87f3
> 9fdbc5fab5
> breaks user-space AddressSanitizer. AddressSanitizer makes assumptions
> about address space layout for substantial performance gains. There
> are multiple people complaining about this already:
> https://github.com/google/sanitizers/issues/837
> https://twitter.com/kayseesee/status/894594085608013825
> https://bugzilla.kernel.org/show_bug.cgi?id=196537
> AddressSanitizer maps shadow memory at [0x00007fff7000-0x10007fff7fff]
> expecting that non-pie binaries will be below 2GB and pie
> binaries/modules will be at 0x55 or 0x7f. This is not the first time
> kernel address space shuffling breaks sanitizers. The last one was the
> move to 0x55.
>
> Is it possible to make this change less aggressive and keep the
> executable under 2GB?
>
> In future please be mindful of user-space sanitizers and talk to
> address-sanitizer@googlegroups.com before shuffling address space.
>
> Thanks
>

[-- Attachment #2: Type: text/html, Size: 2378 bytes --]

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kees Cook]

On Mon, Aug 7, 2017 at 10:24 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> The recent "binfmt_elf: use ELF_ET_DYN_BASE only for PIE" patch:
> https://github.com/torvalds/linux/commit/eab09532d40090698b05a07c1c87f39fdbc5fab5
> breaks user-space AddressSanitizer. AddressSanitizer makes assumptions
> about address space layout for substantial performance gains. There
> are multiple people complaining about this already:
> https://github.com/google/sanitizers/issues/837
> https://twitter.com/kayseesee/status/894594085608013825
> https://bugzilla.kernel.org/show_bug.cgi?id=196537
> AddressSanitizer maps shadow memory at [0x00007fff7000-0x10007fff7fff]
> expecting that non-pie binaries will be below 2GB and pie
> binaries/modules will be at 0x55 or 0x7f. This is not the first time
> kernel address space shuffling breaks sanitizers. The last one was the
> move to 0x55.

What are the requirements for 32-bit and 64-bit memory layouts for
ASan currently, so we can adjust the ET_DYN base to work with existing
ASan?

I would note that on 64-bit the ELF_ET_DYN_BASE adjustment avoids the
entire 2GB space to stay out of the way of 32-bit address-using VMs,
for example.

What ranges should be avoided currently? We need to balance this
against the need to keep the PIE away from a growing heap...

> Is it possible to make this change less aggressive and keep the
> executable under 2GB?

_Under_ 2GB? It's possible we're going to need some VM tunable to
adjust these things if we're facing incompatible requirements...

ASan does seem especially fragile about these kinds of changes. Can
future versions of ASan be more dynamic about this?

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kostya Serebryany]

[-- Attachment #1: Type: text/plain, Size: 3265 bytes --]

+eugenis@ for msan

On Mon, Aug 7, 2017 at 10:33 AM, Kees Cook <keescook@google.com> wrote:

> On Mon, Aug 7, 2017 at 10:24 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> > The recent "binfmt_elf: use ELF_ET_DYN_BASE only for PIE" patch:
> > https://github.com/torvalds/linux/commit/eab09532d40090698b05a07c1c87f3
> 9fdbc5fab5
> > breaks user-space AddressSanitizer. AddressSanitizer makes assumptions
> > about address space layout for substantial performance gains. There
> > are multiple people complaining about this already:
> > https://github.com/google/sanitizers/issues/837
> > https://twitter.com/kayseesee/status/894594085608013825
> > https://bugzilla.kernel.org/show_bug.cgi?id=196537
> > AddressSanitizer maps shadow memory at [0x00007fff7000-0x10007fff7fff]
> > expecting that non-pie binaries will be below 2GB and pie
> > binaries/modules will be at 0x55 or 0x7f. This is not the first time
> > kernel address space shuffling breaks sanitizers. The last one was the
> > move to 0x55.
>
> What are the requirements for 32-bit and 64-bit memory layouts for
> ASan currently, so we can adjust the ET_DYN base to work with existing
> ASan?
>


64-bit asan shadow is 0x00007fff8000 - 0x10007fff8000
32-bit asan shadow is 0x20000000 - 0x40000000


% cat dummy.c
int main(){}
% clang -fsanitize=address dummy.c && ASAN_OPTIONS=verbosity=1 ./a.out
 2>&1 | grep '||'
|| `[0x10007fff8000, 0x7fffffffffff]` || HighMem    ||
|| `[0x02008fff7000, 0x10007fff7fff]` || HighShadow ||
|| `[0x00008fff7000, 0x02008fff6fff]` || ShadowGap  ||
|| `[0x00007fff8000, 0x00008fff6fff]` || LowShadow  ||
|| `[0x000000000000, 0x00007fff7fff]` || LowMem     ||
%

% clang -fsanitize=address dummy.c -m32 && ASAN_OPTIONS=verbosity=1 ./a.out
 2>&1 | grep '||'
|| `[0x40000000, 0xffffffff]` || HighMem    ||
|| `[0x28000000, 0x3fffffff]` || HighShadow ||
|| `[0x24000000, 0x27ffffff]` || ShadowGap  ||
|| `[0x20000000, 0x23ffffff]` || LowShadow  ||
|| `[0x00000000, 0x1fffffff]` || LowMem     ||
%





>
> I would note that on 64-bit the ELF_ET_DYN_BASE adjustment avoids the
> entire 2GB space


Correct, but sadly it overlaps with the asan shadow (see above)


> to stay out of the way of 32-bit address-using VMs,
> for example.
>
> What ranges should be avoided currently? We need to balance this
> against the need to keep the PIE away from a growing heap...
>

See above.


>
> > Is it possible to make this change less aggressive and keep the
> > executable under 2GB?
>
> _Under_ 2GB? It's possible we're going to need some VM tunable to
> adjust these things if we're facing incompatible requirements...
>
> ASan does seem especially fragile about these kinds of changes. Can
> future versions of ASan be more dynamic about this?
>

ASan already has the dynamic shadow as an option, and it's default mode
on 64-bit windows, where the kernel is actively hostile to asan.
On Linux, we could enable it by
  clang -fsanitize=address -O dummy.cc -mllvm -asan-force-dynamic-shadow=1
(not heavily tested though).

The problem is that this comes at a cost that we are very reluctant to pay.
Dynamic shadow means one extra load and one extra register stolen per
function,
which increases the CPU usage and code size.



--kcc




>
> -Kees
>
> --
> Kees Cook
> Pixel Security
>

[-- Attachment #2: Type: text/html, Size: 5645 bytes --]

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Evgenii Stepanov]

MSan is 64-bit only and does not allow any mappings _outside_ of these regions:
000000000000 - 010000000000 app-1
510000000000 - 600000000000 app-2
700000000000 - 800000000000 app-3

https://github.com/google/sanitizers/issues/579

It sounds like the ELF_ET_DYN_BASE change should not break MSan.


On Mon, Aug 7, 2017 at 11:26 AM, Kostya Serebryany <kcc@google.com> wrote:
> +eugenis@ for msan
>
> On Mon, Aug 7, 2017 at 10:33 AM, Kees Cook <keescook@google.com> wrote:
>>
>> On Mon, Aug 7, 2017 at 10:24 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> > The recent "binfmt_elf: use ELF_ET_DYN_BASE only for PIE" patch:
>> >
>> > https://github.com/torvalds/linux/commit/eab09532d40090698b05a07c1c87f39fdbc5fab5
>> > breaks user-space AddressSanitizer. AddressSanitizer makes assumptions
>> > about address space layout for substantial performance gains. There
>> > are multiple people complaining about this already:
>> > https://github.com/google/sanitizers/issues/837
>> > https://twitter.com/kayseesee/status/894594085608013825
>> > https://bugzilla.kernel.org/show_bug.cgi?id=196537
>> > AddressSanitizer maps shadow memory at [0x00007fff7000-0x10007fff7fff]
>> > expecting that non-pie binaries will be below 2GB and pie
>> > binaries/modules will be at 0x55 or 0x7f. This is not the first time
>> > kernel address space shuffling breaks sanitizers. The last one was the
>> > move to 0x55.
>>
>> What are the requirements for 32-bit and 64-bit memory layouts for
>> ASan currently, so we can adjust the ET_DYN base to work with existing
>> ASan?
>
>
>
> 64-bit asan shadow is 0x00007fff8000 - 0x10007fff8000
> 32-bit asan shadow is 0x20000000 - 0x40000000
>
>
> % cat dummy.c
> int main(){}
> % clang -fsanitize=address dummy.c && ASAN_OPTIONS=verbosity=1 ./a.out  2>&1
> | grep '||'
> || `[0x10007fff8000, 0x7fffffffffff]` || HighMem    ||
> || `[0x02008fff7000, 0x10007fff7fff]` || HighShadow ||
> || `[0x00008fff7000, 0x02008fff6fff]` || ShadowGap  ||
> || `[0x00007fff8000, 0x00008fff6fff]` || LowShadow  ||
> || `[0x000000000000, 0x00007fff7fff]` || LowMem     ||
> %
>
> % clang -fsanitize=address dummy.c -m32 && ASAN_OPTIONS=verbosity=1 ./a.out
> 2>&1 | grep '||'
> || `[0x40000000, 0xffffffff]` || HighMem    ||
> || `[0x28000000, 0x3fffffff]` || HighShadow ||
> || `[0x24000000, 0x27ffffff]` || ShadowGap  ||
> || `[0x20000000, 0x23ffffff]` || LowShadow  ||
> || `[0x00000000, 0x1fffffff]` || LowMem     ||
> %
>
>
>
>
>>
>>
>> I would note that on 64-bit the ELF_ET_DYN_BASE adjustment avoids the
>> entire 2GB space
>
>
> Correct, but sadly it overlaps with the asan shadow (see above)
>
>>
>> to stay out of the way of 32-bit address-using VMs,
>> for example.
>>
>> What ranges should be avoided currently? We need to balance this
>> against the need to keep the PIE away from a growing heap...
>
>
> See above.
>
>>
>>
>> > Is it possible to make this change less aggressive and keep the
>> > executable under 2GB?
>>
>> _Under_ 2GB? It's possible we're going to need some VM tunable to
>> adjust these things if we're facing incompatible requirements...
>>
>> ASan does seem especially fragile about these kinds of changes. Can
>> future versions of ASan be more dynamic about this?
>
>
> ASan already has the dynamic shadow as an option, and it's default mode
> on 64-bit windows, where the kernel is actively hostile to asan.
> On Linux, we could enable it by
>   clang -fsanitize=address -O dummy.cc -mllvm -asan-force-dynamic-shadow=1
> (not heavily tested though).
>
> The problem is that this comes at a cost that we are very reluctant to pay.
> Dynamic shadow means one extra load and one extra register stolen per
> function,
> which increases the CPU usage and code size.
>
>
>
> --kcc
>
>
>
>>
>>
>> -Kees
>>
>> --
>> Kees Cook
>> Pixel Security
>
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kees Cook]

On Mon, Aug 7, 2017 at 11:36 AM, Evgenii Stepanov <eugenis@google.com> wrote:
> MSan is 64-bit only and does not allow any mappings _outside_ of these regions:
> 000000000000 - 010000000000 app-1
> 510000000000 - 600000000000 app-2
> 700000000000 - 800000000000 app-3
>
> https://github.com/google/sanitizers/issues/579
>
> It sounds like the ELF_ET_DYN_BASE change should not break MSan.

Hah, so the proposed move to 0x1000 8000 0000 for ASan would break
MSan. Lovely! :P

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Evgenii Stepanov]

On Mon, Aug 7, 2017 at 11:40 AM, Kees Cook <keescook@google.com> wrote:
> On Mon, Aug 7, 2017 at 11:36 AM, Evgenii Stepanov <eugenis@google.com> wrote:
>> MSan is 64-bit only and does not allow any mappings _outside_ of these regions:
>> 000000000000 - 010000000000 app-1
>> 510000000000 - 600000000000 app-2
>> 700000000000 - 800000000000 app-3
>>
>> https://github.com/google/sanitizers/issues/579
>>
>> It sounds like the ELF_ET_DYN_BASE change should not break MSan.
>
> Hah, so the proposed move to 0x1000 8000 0000 for ASan would break
> MSan. Lovely! :P

That's unfortunate.
This will not help existing binaries, but going forward the mapping
can be adjusted at runtime to anything like
000000000000 .. A
500000000000 + A .. 600000000000
700000000000 .. 800000000000
i.e. we can look at where the binary is mapped and set A to anything
in the range of [0, 1000 0000 0000). That's still not compatible with
0x1000 8000 0000 though.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kees Cook]

On Mon, Aug 7, 2017 at 11:51 AM, Evgenii Stepanov <eugenis@google.com> wrote:
> On Mon, Aug 7, 2017 at 11:40 AM, Kees Cook <keescook@google.com> wrote:
>> On Mon, Aug 7, 2017 at 11:36 AM, Evgenii Stepanov <eugenis@google.com> wrote:
>>> MSan is 64-bit only and does not allow any mappings _outside_ of these regions:
>>> 000000000000 - 010000000000 app-1
>>> 510000000000 - 600000000000 app-2
>>> 700000000000 - 800000000000 app-3
>>>
>>> https://github.com/google/sanitizers/issues/579
>>>
>>> It sounds like the ELF_ET_DYN_BASE change should not break MSan.
>>
>> Hah, so the proposed move to 0x1000 8000 0000 for ASan would break
>> MSan. Lovely! :P
>
> That's unfortunate.
> This will not help existing binaries, but going forward the mapping
> can be adjusted at runtime to anything like
> 000000000000 .. A
> 500000000000 + A .. 600000000000
> 700000000000 .. 800000000000
> i.e. we can look at where the binary is mapped and set A to anything
> in the range of [0, 1000 0000 0000). That's still not compatible with
> 0x1000 8000 0000 though.

So A is considered to be < 0x1000 0000 0000? And a future MSan could
handle a PIE base of 0x2000 0000 0000? If ASan an TSan can handle that
too, then we could use that as the future PIE base. Existing systems
will need some sort of reversion.

The primary concerns with the CVEs fixed with the PIE base commit was
for 32-bit. While it is possible to collide on 64-bit, it is much more
rare. As long as we have no problems with the new 32-bit PIE base, we
can revert the 64-bit base default back to 0x5555 5555 4000.

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kostya Serebryany]

[-- Attachment #1: Type: text/plain, Size: 2126 bytes --]

On Mon, Aug 7, 2017 at 11:57 AM, Kees Cook <keescook@google.com> wrote:

> On Mon, Aug 7, 2017 at 11:51 AM, Evgenii Stepanov <eugenis@google.com>
> wrote:
> > On Mon, Aug 7, 2017 at 11:40 AM, Kees Cook <keescook@google.com> wrote:
> >> On Mon, Aug 7, 2017 at 11:36 AM, Evgenii Stepanov <eugenis@google.com>
> wrote:
> >>> MSan is 64-bit only and does not allow any mappings _outside_ of these
> regions:
> >>> 000000000000 - 010000000000 app-1
> >>> 510000000000 - 600000000000 app-2
> >>> 700000000000 - 800000000000 app-3
> >>>
> >>> https://github.com/google/sanitizers/issues/579
> >>>
> >>> It sounds like the ELF_ET_DYN_BASE change should not break MSan.
> >>
> >> Hah, so the proposed move to 0x1000 8000 0000 for ASan would break
> >> MSan. Lovely! :P
> >
> > That's unfortunate.
> > This will not help existing binaries, but going forward the mapping
> > can be adjusted at runtime to anything like
> > 000000000000 .. A
> > 500000000000 + A .. 600000000000
> > 700000000000 .. 800000000000
> > i.e. we can look at where the binary is mapped and set A to anything
> > in the range of [0, 1000 0000 0000). That's still not compatible with
> > 0x1000 8000 0000 though.
>
> So A is considered to be < 0x1000 0000 0000? And a future MSan could
> handle a PIE base of 0x2000 0000 0000? If ASan an TSan can handle that
> too, then we could use that as the future PIE base. Existing systems
> will need some sort of reversion.
>
> The primary concerns with the CVEs fixed with the PIE base commit was
> for 32-bit. While it is possible to collide on 64-bit, it is much more
> rare. As long as we have no problems with the new 32-bit PIE base, we
> can revert the 64-bit base default back to 0x5555 5555 4000.
>

Yes, please!!

Also, would it be possible to introduce some kind of regression testing
into the kernel testing process to avoid such breakages in future?
It would be as simple as running a handful of commands like this (for gcc
and clang, for asan/tsan/msan, for 32-bit and 64-bit)
     echo "int main(){}" |  clang  -x c++ -   -fsanitize=address && ./a.out






>
> -Kees
>
> --
> Kees Cook
> Pixel Security
>

[-- Attachment #2: Type: text/html, Size: 3359 bytes --]

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kees Cook]

On Mon, Aug 7, 2017 at 12:03 PM, Kostya Serebryany <kcc@google.com> wrote:
>
>
> On Mon, Aug 7, 2017 at 11:57 AM, Kees Cook <keescook@google.com> wrote:
>>
>> On Mon, Aug 7, 2017 at 11:51 AM, Evgenii Stepanov <eugenis@google.com>
>> wrote:
>> > On Mon, Aug 7, 2017 at 11:40 AM, Kees Cook <keescook@google.com> wrote:
>> >> On Mon, Aug 7, 2017 at 11:36 AM, Evgenii Stepanov <eugenis@google.com>
>> >> wrote:
>> >>> MSan is 64-bit only and does not allow any mappings _outside_ of these
>> >>> regions:
>> >>> 000000000000 - 010000000000 app-1
>> >>> 510000000000 - 600000000000 app-2
>> >>> 700000000000 - 800000000000 app-3
>> >>>
>> >>> https://github.com/google/sanitizers/issues/579
>> >>>
>> >>> It sounds like the ELF_ET_DYN_BASE change should not break MSan.
>> >>
>> >> Hah, so the proposed move to 0x1000 8000 0000 for ASan would break
>> >> MSan. Lovely! :P
>> >
>> > That's unfortunate.
>> > This will not help existing binaries, but going forward the mapping
>> > can be adjusted at runtime to anything like
>> > 000000000000 .. A
>> > 500000000000 + A .. 600000000000
>> > 700000000000 .. 800000000000
>> > i.e. we can look at where the binary is mapped and set A to anything
>> > in the range of [0, 1000 0000 0000). That's still not compatible with
>> > 0x1000 8000 0000 though.
>>
>> So A is considered to be < 0x1000 0000 0000? And a future MSan could
>> handle a PIE base of 0x2000 0000 0000? If ASan an TSan can handle that
>> too, then we could use that as the future PIE base. Existing systems
>> will need some sort of reversion.
>>
>> The primary concerns with the CVEs fixed with the PIE base commit was
>> for 32-bit. While it is possible to collide on 64-bit, it is much more
>> rare. As long as we have no problems with the new 32-bit PIE base, we
>> can revert the 64-bit base default back to 0x5555 5555 4000.
>
>
> Yes, please!!
>
> Also, would it be possible to introduce some kind of regression testing into
> the kernel testing process to avoid such breakages in future?
> It would be as simple as running a handful of commands like this (for gcc
> and clang, for asan/tsan/msan, for 32-bit and 64-bit)
>      echo "int main(){}" |  clang  -x c++ -   -fsanitize=address && ./a.out

I was actually going to ask why the *San developers didn't notice the
change? It lived in linux-next for months. :P Can you please add a
test for what you need to the tools/testing/selftests/ tree? A
gcc-based test would be preferred, of course.

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kostya Serebryany]

[-- Attachment #1: Type: text/plain, Size: 3047 bytes --]

On Mon, Aug 7, 2017 at 12:06 PM, Kees Cook <keescook@google.com> wrote:

> On Mon, Aug 7, 2017 at 12:03 PM, Kostya Serebryany <kcc@google.com> wrote:
> >
> >
> > On Mon, Aug 7, 2017 at 11:57 AM, Kees Cook <keescook@google.com> wrote:
> >>
> >> On Mon, Aug 7, 2017 at 11:51 AM, Evgenii Stepanov <eugenis@google.com>
> >> wrote:
> >> > On Mon, Aug 7, 2017 at 11:40 AM, Kees Cook <keescook@google.com>
> wrote:
> >> >> On Mon, Aug 7, 2017 at 11:36 AM, Evgenii Stepanov <
> eugenis@google.com>
> >> >> wrote:
> >> >>> MSan is 64-bit only and does not allow any mappings _outside_ of
> these
> >> >>> regions:
> >> >>> 000000000000 - 010000000000 app-1
> >> >>> 510000000000 - 600000000000 app-2
> >> >>> 700000000000 - 800000000000 app-3
> >> >>>
> >> >>> https://github.com/google/sanitizers/issues/579
> >> >>>
> >> >>> It sounds like the ELF_ET_DYN_BASE change should not break MSan.
> >> >>
> >> >> Hah, so the proposed move to 0x1000 8000 0000 for ASan would break
> >> >> MSan. Lovely! :P
> >> >
> >> > That's unfortunate.
> >> > This will not help existing binaries, but going forward the mapping
> >> > can be adjusted at runtime to anything like
> >> > 000000000000 .. A
> >> > 500000000000 + A .. 600000000000
> >> > 700000000000 .. 800000000000
> >> > i.e. we can look at where the binary is mapped and set A to anything
> >> > in the range of [0, 1000 0000 0000). That's still not compatible with
> >> > 0x1000 8000 0000 though.
> >>
> >> So A is considered to be < 0x1000 0000 0000? And a future MSan could
> >> handle a PIE base of 0x2000 0000 0000? If ASan an TSan can handle that
> >> too, then we could use that as the future PIE base. Existing systems
> >> will need some sort of reversion.
> >>
> >> The primary concerns with the CVEs fixed with the PIE base commit was
> >> for 32-bit. While it is possible to collide on 64-bit, it is much more
> >> rare. As long as we have no problems with the new 32-bit PIE base, we
> >> can revert the 64-bit base default back to 0x5555 5555 4000.
> >
> >
> > Yes, please!!
> >
> > Also, would it be possible to introduce some kind of regression testing
> into
> > the kernel testing process to avoid such breakages in future?
> > It would be as simple as running a handful of commands like this (for gcc
> > and clang, for asan/tsan/msan, for 32-bit and 64-bit)
> >      echo "int main(){}" |  clang  -x c++ -   -fsanitize=address &&
> ./a.out
>
> I was actually going to ask why the *San developers didn't notice the
> change? It lived in linux-next for months. :P


:)


> Can you please add a
> test for what you need to the tools/testing/selftests/ tree? A
>
We'll do it, thanks for the pointer (Dmitry, could you do this, please?)


> gcc-based test would be preferred, of course.
>

gcc implementation lags behind.
Namely,
gcc has: asan, tsan
gcc does not have msan, asan with dynamic shadow base, dfsan

So, while having a gcc-only testing would have prevented this particular
regression, it wouldn't solve some similar ones.

--kcc




>
> -Kees
>
> --
> Kees Cook
> Pixel Security
>

[-- Attachment #2: Type: text/html, Size: 5052 bytes --]

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kees Cook]

On Mon, Aug 7, 2017 at 12:03 PM, Kostya Serebryany <kcc@google.com> wrote:
>
>
> On Mon, Aug 7, 2017 at 11:57 AM, Kees Cook <keescook@google.com> wrote:
>>
>> On Mon, Aug 7, 2017 at 11:51 AM, Evgenii Stepanov <eugenis@google.com>
>> wrote:
>> > On Mon, Aug 7, 2017 at 11:40 AM, Kees Cook <keescook@google.com> wrote:
>> >> On Mon, Aug 7, 2017 at 11:36 AM, Evgenii Stepanov <eugenis@google.com>
>> >> wrote:
>> >>> MSan is 64-bit only and does not allow any mappings _outside_ of these
>> >>> regions:
>> >>> 000000000000 - 010000000000 app-1
>> >>> 510000000000 - 600000000000 app-2
>> >>> 700000000000 - 800000000000 app-3
>> >>>
>> >>> https://github.com/google/sanitizers/issues/579
>> >>>
>> >>> It sounds like the ELF_ET_DYN_BASE change should not break MSan.
>> >>
>> >> Hah, so the proposed move to 0x1000 8000 0000 for ASan would break
>> >> MSan. Lovely! :P
>> >
>> > That's unfortunate.
>> > This will not help existing binaries, but going forward the mapping
>> > can be adjusted at runtime to anything like
>> > 000000000000 .. A
>> > 500000000000 + A .. 600000000000
>> > 700000000000 .. 800000000000
>> > i.e. we can look at where the binary is mapped and set A to anything
>> > in the range of [0, 1000 0000 0000). That's still not compatible with
>> > 0x1000 8000 0000 though.
>>
>> So A is considered to be < 0x1000 0000 0000? And a future MSan could
>> handle a PIE base of 0x2000 0000 0000? If ASan an TSan can handle that
>> too, then we could use that as the future PIE base. Existing systems
>> will need some sort of reversion.
>>
>> The primary concerns with the CVEs fixed with the PIE base commit was
>> for 32-bit. While it is possible to collide on 64-bit, it is much more
>> rare. As long as we have no problems with the new 32-bit PIE base, we
>> can revert the 64-bit base default back to 0x5555 5555 4000.
>
>
> Yes, please!!

For the revert, can you clarify which architectures this is a problem
for? And confirm that the current 32-bit PIE base is not bugged? I'd
like to minimize the revert.

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kostya Serebryany]

[-- Attachment #1: Type: text/plain, Size: 2668 bytes --]

On Mon, Aug 7, 2017 at 12:24 PM, Kees Cook <keescook@google.com> wrote:

> On Mon, Aug 7, 2017 at 12:03 PM, Kostya Serebryany <kcc@google.com> wrote:
> >
> >
> > On Mon, Aug 7, 2017 at 11:57 AM, Kees Cook <keescook@google.com> wrote:
> >>
> >> On Mon, Aug 7, 2017 at 11:51 AM, Evgenii Stepanov <eugenis@google.com>
> >> wrote:
> >> > On Mon, Aug 7, 2017 at 11:40 AM, Kees Cook <keescook@google.com>
> wrote:
> >> >> On Mon, Aug 7, 2017 at 11:36 AM, Evgenii Stepanov <
> eugenis@google.com>
> >> >> wrote:
> >> >>> MSan is 64-bit only and does not allow any mappings _outside_ of
> these
> >> >>> regions:
> >> >>> 000000000000 - 010000000000 app-1
> >> >>> 510000000000 - 600000000000 app-2
> >> >>> 700000000000 - 800000000000 app-3
> >> >>>
> >> >>> https://github.com/google/sanitizers/issues/579
> >> >>>
> >> >>> It sounds like the ELF_ET_DYN_BASE change should not break MSan.
> >> >>
> >> >> Hah, so the proposed move to 0x1000 8000 0000 for ASan would break
> >> >> MSan. Lovely! :P
> >> >
> >> > That's unfortunate.
> >> > This will not help existing binaries, but going forward the mapping
> >> > can be adjusted at runtime to anything like
> >> > 000000000000 .. A
> >> > 500000000000 + A .. 600000000000
> >> > 700000000000 .. 800000000000
> >> > i.e. we can look at where the binary is mapped and set A to anything
> >> > in the range of [0, 1000 0000 0000). That's still not compatible with
> >> > 0x1000 8000 0000 though.
> >>
> >> So A is considered to be < 0x1000 0000 0000? And a future MSan could
> >> handle a PIE base of 0x2000 0000 0000? If ASan an TSan can handle that
> >> too, then we could use that as the future PIE base. Existing systems
> >> will need some sort of reversion.
> >>
> >> The primary concerns with the CVEs fixed with the PIE base commit was
> >> for 32-bit. While it is possible to collide on 64-bit, it is much more
> >> rare. As long as we have no problems with the new 32-bit PIE base, we
> >> can revert the 64-bit base default back to 0x5555 5555 4000.
> >
> >
> > Yes, please!!
>
> For the revert, can you clarify which architectures this is a problem
> for?


We have seen complaints on x86_64 only.
This is also the only arch where we use the small shadow offset 7fff8000.

But I can't verify this on other platforms. :(
On ARM/Android we use zero shadow base.
eugenis@, do we have the problem on Android?

Do we have the solution that will also keep the existing msan usable?


> And confirm that the current 32-bit PIE base is not bugged? I'd
> like to minimize the revert.
>

0x40000000?
Yes, that doesn't affect asan (and tsan/msan are not present in 32-bit).




>
> -Kees
>
> --
> Kees Cook
> Pixel Security
>

[-- Attachment #2: Type: text/html, Size: 4483 bytes --]

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Evgenii Stepanov]

On Mon, Aug 7, 2017 at 11:57 AM, Kees Cook <keescook@google.com> wrote:
> On Mon, Aug 7, 2017 at 11:51 AM, Evgenii Stepanov <eugenis@google.com> wrote:
>> On Mon, Aug 7, 2017 at 11:40 AM, Kees Cook <keescook@google.com> wrote:
>>> On Mon, Aug 7, 2017 at 11:36 AM, Evgenii Stepanov <eugenis@google.com> wrote:
>>>> MSan is 64-bit only and does not allow any mappings _outside_ of these regions:
>>>> 000000000000 - 010000000000 app-1
>>>> 510000000000 - 600000000000 app-2
>>>> 700000000000 - 800000000000 app-3
>>>>
>>>> https://github.com/google/sanitizers/issues/579
>>>>
>>>> It sounds like the ELF_ET_DYN_BASE change should not break MSan.
>>>
>>> Hah, so the proposed move to 0x1000 8000 0000 for ASan would break
>>> MSan. Lovely! :P
>>
>> That's unfortunate.
>> This will not help existing binaries, but going forward the mapping
>> can be adjusted at runtime to anything like
>> 000000000000 .. A
>> 500000000000 + A .. 600000000000
>> 700000000000 .. 800000000000
>> i.e. we can look at where the binary is mapped and set A to anything
>> in the range of [0, 1000 0000 0000). That's still not compatible with
>> 0x1000 8000 0000 though.
>
> So A is considered to be < 0x1000 0000 0000? And a future MSan could
> handle a PIE base of 0x2000 0000 0000? If ASan an TSan can handle that
> too, then we could use that as the future PIE base. Existing systems
> will need some sort of reversion.

We can not handle 2000 0000 0000. We can support at most 0 .. 1000
0000 0000 and 5000 0000 0000 .. 6000 0000 0000, but at runtime we have
to choose A and disable parts of both ranges.

>
> The primary concerns with the CVEs fixed with the PIE base commit was
> for 32-bit. While it is possible to collide on 64-bit, it is much more
> rare. As long as we have no problems with the new 32-bit PIE base, we
> can revert the 64-bit base default back to 0x5555 5555 4000.
>
> -Kees
>
> --
> Kees Cook
> Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Daniel Micay]

> ASan already has the dynamic shadow as an option, and it's default
> mode
> on 64-bit windows, where the kernel is actively hostile to asan. 
> On Linux, we could enable it by
>   clang -fsanitize=address -O dummy.cc -mllvm -asan-force-dynamic-
> shadow=1
> (not heavily tested though). 
> 
> The problem is that this comes at a cost that we are very reluctant to
> pay. 
> Dynamic shadow means one extra load and one extra register stolen per
> function, 
> which increases the CPU usage and code size.

Can libraries compiled with dynamic be mixed with an executable or other
shared objects without it?

It could be the default with -fPIC / -fPIE without changing the default
for position dependent executables. Code isn't really PIC if it can't be
mapped within a large range. The performance hit would be paid by people
using dynamic libraries + PIE by default, but not non-PIE executables
and static libraries (unless they get compiled with -fPIC to allow
linking in a dynamic library, which is uncommon).

I'm sure a fix can be found either in the kernel or the sanitizers for
this specific PIE base move, but the problem isn't limited to this.

There are currently other issues. Try:

sysctl vm.mmap_rnd_bits=32
sysctl vm.mmap_rnd_compat_bits=16

IIRC that breaks some sanitizers at least for 32-bit executables.

Similar issues happen with certain arm64 address space configs since it
offers a bunch of choices (3 vs. 4 level page tables, different page
sizes).

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Daniel Micay]

> There are currently other issues. Try:
> 
> sysctl vm.mmap_rnd_bits=32
> sysctl vm.mmap_rnd_compat_bits=16
> 
> IIRC that breaks some sanitizers at least for 32-bit executables.

Also, stack mapping rand isn't yet tied to that sysctl but is rather
hard-wired to 11 bits on 32-bit and 20 bits (IIRC) on 64-bit. Once it's
tied to the sysctl (or a different sysctl, if keeping the same defaults
is desired) that will be able to use significantly more address space.

It might be setting it to the maximum + better stack rand that breaks
sanitizers, rather than just setting the entropy higher.

If anyone wants to test some other changes though...

https://github.com/copperhead/linux-hardened/commit/31ebed471d31a437cc551b1bfae03c9e7f58117d.patch
https://github.com/copperhead/linux-hardened/commit/073329e7b541b89172833f61fb84d81f32389d6e.patch

They haven't been submitted for inclusion upstream, but that's the plan:
reaching parity with the ASLR PaX has provided for years when the
entropy values are set to max.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kees Cook]

On Mon, Aug 7, 2017 at 11:26 AM, Kostya Serebryany <kcc@google.com> wrote:
> +eugenis@ for msan
>
> On Mon, Aug 7, 2017 at 10:33 AM, Kees Cook <keescook@google.com> wrote:
>>
>> On Mon, Aug 7, 2017 at 10:24 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> > The recent "binfmt_elf: use ELF_ET_DYN_BASE only for PIE" patch:
>> >
>> > https://github.com/torvalds/linux/commit/eab09532d40090698b05a07c1c87f39fdbc5fab5
>> > breaks user-space AddressSanitizer. AddressSanitizer makes assumptions
>> > about address space layout for substantial performance gains. There
>> > are multiple people complaining about this already:
>> > https://github.com/google/sanitizers/issues/837
>> > https://twitter.com/kayseesee/status/894594085608013825
>> > https://bugzilla.kernel.org/show_bug.cgi?id=196537
>> > AddressSanitizer maps shadow memory at [0x00007fff7000-0x10007fff7fff]
>> > expecting that non-pie binaries will be below 2GB and pie
>> > binaries/modules will be at 0x55 or 0x7f. This is not the first time
>> > kernel address space shuffling breaks sanitizers. The last one was the
>> > move to 0x55.
>>
>> What are the requirements for 32-bit and 64-bit memory layouts for
>> ASan currently, so we can adjust the ET_DYN base to work with existing
>> ASan?
>
>
> 32-bit asan shadow is 0x20000000 - 0x40000000
>
> % clang -fsanitize=address dummy.c -m32 && ASAN_OPTIONS=verbosity=1 ./a.out
> 2>&1 | grep '||'
> || `[0x40000000, 0xffffffff]` || HighMem    ||
> || `[0x28000000, 0x3fffffff]` || HighShadow ||
> || `[0x24000000, 0x27ffffff]` || ShadowGap  ||
> || `[0x20000000, 0x23ffffff]` || LowShadow  ||
> || `[0x00000000, 0x1fffffff]` || LowMem     ||
> %

For 32-bit, it looks like the new PIE base is fine, yes? 0x000400000UL

> 64-bit asan shadow is 0x00007fff8000 - 0x10007fff8000
>
> % cat dummy.c
> int main(){}
> % clang -fsanitize=address dummy.c && ASAN_OPTIONS=verbosity=1 ./a.out  2>&1
> | grep '||'
> || `[0x10007fff8000, 0x7fffffffffff]` || HighMem    ||
> || `[0x02008fff7000, 0x10007fff7fff]` || HighShadow ||
> || `[0x00008fff7000, 0x02008fff6fff]` || ShadowGap  ||
> || `[0x00007fff8000, 0x00008fff6fff]` || LowShadow  ||
> || `[0x000000000000, 0x00007fff7fff]` || LowMem     ||
> %

Okay, so, for 64-bit PIE base, we want to avoid the entire 2GB space
(as mentioned in the commit and by Daniel on this thread). And for
ASan we want to basically avoid 0x 8000 0000 to 0x1000 8000 0000?
That's a huge span of memory. Still, 0x1000 8000 0000 would be an
improvement over 0x5555 5555 4000.

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Daniel Micay]

On Mon, 2017-08-07 at 11:39 -0700, Kees Cook wrote:
> On Mon, Aug 7, 2017 at 11:26 AM, Kostya Serebryany <kcc@google.com>
> wrote:
> > +eugenis@ for msan
> > 
> > On Mon, Aug 7, 2017 at 10:33 AM, Kees Cook <keescook@google.com>
> > wrote:
> > > 
> > > On Mon, Aug 7, 2017 at 10:24 AM, Dmitry Vyukov <dvyukov@google.com
> > > > wrote:
> > > > The recent "binfmt_elf: use ELF_ET_DYN_BASE only for PIE" patch:
> > > > 
> > > > https://github.com/torvalds/linux/commit/eab09532d40090698b05a07
> > > > c1c87f39fdbc5fab5
> > > > breaks user-space AddressSanitizer. AddressSanitizer makes
> > > > assumptions
> > > > about address space layout for substantial performance gains.
> > > > There
> > > > are multiple people complaining about this already:
> > > > https://github.com/google/sanitizers/issues/837
> > > > https://twitter.com/kayseesee/status/894594085608013825
> > > > https://bugzilla.kernel.org/show_bug.cgi?id=196537
> > > > AddressSanitizer maps shadow memory at [0x00007fff7000-
> > > > 0x10007fff7fff]
> > > > expecting that non-pie binaries will be below 2GB and pie
> > > > binaries/modules will be at 0x55 or 0x7f. This is not the first
> > > > time
> > > > kernel address space shuffling breaks sanitizers. The last one
> > > > was the
> > > > move to 0x55.
> > > 
> > > What are the requirements for 32-bit and 64-bit memory layouts for
> > > ASan currently, so we can adjust the ET_DYN base to work with
> > > existing
> > > ASan?
> > 
> > 
> > 32-bit asan shadow is 0x20000000 - 0x40000000
> > 
> > % clang -fsanitize=address dummy.c -m32 && ASAN_OPTIONS=verbosity=1
> > ./a.out
> > 2>&1 | grep '||'
> > > > `[0x40000000, 0xffffffff]` || HighMem    ||
> > > > `[0x28000000, 0x3fffffff]` || HighShadow ||
> > > > `[0x24000000, 0x27ffffff]` || ShadowGap  ||
> > > > `[0x20000000, 0x23ffffff]` || LowShadow  ||
> > > > `[0x00000000, 0x1fffffff]` || LowMem     ||
> > 
> > %
> 
> For 32-bit, it looks like the new PIE base is fine, yes? 0x000400000UL

Need to consider the ASLR shift which is up to 1M with a default kernel
configuration but up to 256M with the maximum configurable entropy.

On 64-bit, it's a lot larger... and the goal is also tying the stack
base to that so that's a further significant change, increasing the
address space used when the maximum configurable entropy is used.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kees Cook]

On Mon, Aug 7, 2017 at 11:48 AM, Daniel Micay <danielmicay@gmail.com> wrote:
> On Mon, 2017-08-07 at 11:39 -0700, Kees Cook wrote:
>> On Mon, Aug 7, 2017 at 11:26 AM, Kostya Serebryany <kcc@google.com>
>> wrote:
>> > +eugenis@ for msan
>> >
>> > On Mon, Aug 7, 2017 at 10:33 AM, Kees Cook <keescook@google.com>
>> > wrote:
>> > >
>> > > On Mon, Aug 7, 2017 at 10:24 AM, Dmitry Vyukov <dvyukov@google.com
>> > > > wrote:
>> > > > The recent "binfmt_elf: use ELF_ET_DYN_BASE only for PIE" patch:
>> > > >
>> > > > https://github.com/torvalds/linux/commit/eab09532d40090698b05a07
>> > > > c1c87f39fdbc5fab5
>> > > > breaks user-space AddressSanitizer. AddressSanitizer makes
>> > > > assumptions
>> > > > about address space layout for substantial performance gains.
>> > > > There
>> > > > are multiple people complaining about this already:
>> > > > https://github.com/google/sanitizers/issues/837
>> > > > https://twitter.com/kayseesee/status/894594085608013825
>> > > > https://bugzilla.kernel.org/show_bug.cgi?id=196537
>> > > > AddressSanitizer maps shadow memory at [0x00007fff7000-
>> > > > 0x10007fff7fff]
>> > > > expecting that non-pie binaries will be below 2GB and pie
>> > > > binaries/modules will be at 0x55 or 0x7f. This is not the first
>> > > > time
>> > > > kernel address space shuffling breaks sanitizers. The last one
>> > > > was the
>> > > > move to 0x55.
>> > >
>> > > What are the requirements for 32-bit and 64-bit memory layouts for
>> > > ASan currently, so we can adjust the ET_DYN base to work with
>> > > existing
>> > > ASan?
>> >
>> >
>> > 32-bit asan shadow is 0x20000000 - 0x40000000
>> >
>> > % clang -fsanitize=address dummy.c -m32 && ASAN_OPTIONS=verbosity=1
>> > ./a.out
>> > 2>&1 | grep '||'
>> > > > `[0x40000000, 0xffffffff]` || HighMem    ||
>> > > > `[0x28000000, 0x3fffffff]` || HighShadow ||
>> > > > `[0x24000000, 0x27ffffff]` || ShadowGap  ||
>> > > > `[0x20000000, 0x23ffffff]` || LowShadow  ||
>> > > > `[0x00000000, 0x1fffffff]` || LowMem     ||
>> >
>> > %
>>
>> For 32-bit, it looks like the new PIE base is fine, yes? 0x000400000UL
>
> Need to consider the ASLR shift which is up to 1M with a default kernel
> configuration but up to 256M with the maximum configurable entropy.
>
> On 64-bit, it's a lot larger... and the goal is also tying the stack
> base to that so that's a further significant change, increasing the
> address space used when the maximum configurable entropy is used.

We've got two things to do upstream:

- fix the default kernel for ASan

- maximize the entropy optionally

I.e. the first is a userspace regression that needs to be fixed for
existing ASan user. The second is developing a future path to
maximizing the non-default entropy, for which new versions of *San
would want to detect and use.

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kostya Serebryany]

[-- Attachment #1: Type: text/plain, Size: 3253 bytes --]

On Mon, Aug 7, 2017 at 11:52 AM, Kees Cook <keescook@google.com> wrote:

> On Mon, Aug 7, 2017 at 11:48 AM, Daniel Micay <danielmicay@gmail.com>
> wrote:
> > On Mon, 2017-08-07 at 11:39 -0700, Kees Cook wrote:
> >> On Mon, Aug 7, 2017 at 11:26 AM, Kostya Serebryany <kcc@google.com>
> >> wrote:
> >> > +eugenis@ for msan
> >> >
> >> > On Mon, Aug 7, 2017 at 10:33 AM, Kees Cook <keescook@google.com>
> >> > wrote:
> >> > >
> >> > > On Mon, Aug 7, 2017 at 10:24 AM, Dmitry Vyukov <dvyukov@google.com
> >> > > > wrote:
> >> > > > The recent "binfmt_elf: use ELF_ET_DYN_BASE only for PIE" patch:
> >> > > >
> >> > > > https://github.com/torvalds/linux/commit/eab09532d40090698b05a07
> >> > > > c1c87f39fdbc5fab5
> >> > > > breaks user-space AddressSanitizer. AddressSanitizer makes
> >> > > > assumptions
> >> > > > about address space layout for substantial performance gains.
> >> > > > There
> >> > > > are multiple people complaining about this already:
> >> > > > https://github.com/google/sanitizers/issues/837
> >> > > > https://twitter.com/kayseesee/status/894594085608013825
> >> > > > https://bugzilla.kernel.org/show_bug.cgi?id=196537
> >> > > > AddressSanitizer maps shadow memory at [0x00007fff7000-
> >> > > > 0x10007fff7fff]
> >> > > > expecting that non-pie binaries will be below 2GB and pie
> >> > > > binaries/modules will be at 0x55 or 0x7f. This is not the first
> >> > > > time
> >> > > > kernel address space shuffling breaks sanitizers. The last one
> >> > > > was the
> >> > > > move to 0x55.
> >> > >
> >> > > What are the requirements for 32-bit and 64-bit memory layouts for
> >> > > ASan currently, so we can adjust the ET_DYN base to work with
> >> > > existing
> >> > > ASan?
> >> >
> >> >
> >> > 32-bit asan shadow is 0x20000000 - 0x40000000
> >> >
> >> > % clang -fsanitize=address dummy.c -m32 && ASAN_OPTIONS=verbosity=1
> >> > ./a.out
> >> > 2>&1 | grep '||'
> >> > > > `[0x40000000, 0xffffffff]` || HighMem    ||
> >> > > > `[0x28000000, 0x3fffffff]` || HighShadow ||
> >> > > > `[0x24000000, 0x27ffffff]` || ShadowGap  ||
> >> > > > `[0x20000000, 0x23ffffff]` || LowShadow  ||
> >> > > > `[0x00000000, 0x1fffffff]` || LowMem     ||
> >> >
> >> > %
> >>
> >> For 32-bit, it looks like the new PIE base is fine, yes? 0x000400000UL
> >
> > Need to consider the ASLR shift which is up to 1M with a default kernel
> > configuration but up to 256M with the maximum configurable entropy.
> >
> > On 64-bit, it's a lot larger... and the goal is also tying the stack
> > base to that so that's a further significant change, increasing the
> > address space used when the maximum configurable entropy is used.
>
> We've got two things to do upstream:
>
> - fix the default kernel for ASan
>
> - maximize the entropy optionally
>

Is it possible to implement some userspace<=>kernel interface that will
allow applications (sanitizers)
to request *fixed* address ranges from the kernel at startup (so that the
kernel couldn't refuse)?

--kcc



>
> I.e. the first is a userspace regression that needs to be fixed for
> existing ASan user. The second is developing a future path to
> maximizing the non-default entropy, for which new versions of *San
> would want to detect and use.
>
> -Kees
>
> --
> Kees Cook
> Pixel Security
>

[-- Attachment #2: Type: text/html, Size: 5398 bytes --]

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kees Cook]

On Mon, Aug 7, 2017 at 11:56 AM, Kostya Serebryany <kcc@google.com> wrote:
> Is it possible to implement some userspace<=>kernel interface that will
> allow applications (sanitizers)
> to request *fixed* address ranges from the kernel at startup (so that the
> kernel couldn't refuse)?

Wouldn't building non-PIE accomplish this?

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Daniel Micay]

On Mon, 2017-08-07 at 11:59 -0700, Kees Cook wrote:
> On Mon, Aug 7, 2017 at 11:56 AM, Kostya Serebryany <kcc@google.com>
> wrote:
> > Is it possible to implement some userspace<=>kernel interface that
> > will
> > allow applications (sanitizers)
> > to request *fixed* address ranges from the kernel at startup (so
> > that the
> > kernel couldn't refuse)?
> 
> Wouldn't building non-PIE accomplish this?
> 
> -Kees
> 

Well that lets you map the executable in the desired location, and
sanitizers would be free to add a huge reserved mapping as part of the
executable, but if they want the mapping placed away from the exe there
is not really any way to do that reliably without it being dynamic.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kostya Serebryany]

[-- Attachment #1: Type: text/plain, Size: 686 bytes --]

On Mon, Aug 7, 2017 at 11:59 AM, Kees Cook <keescook@google.com> wrote:

> On Mon, Aug 7, 2017 at 11:56 AM, Kostya Serebryany <kcc@google.com> wrote:
> > Is it possible to implement some userspace<=>kernel interface that will
> > allow applications (sanitizers)
> > to request *fixed* address ranges from the kernel at startup (so that the
> > kernel couldn't refuse)?
>
> Wouldn't building non-PIE accomplish this?
>

Well, many asan users do need PIE.
Then, non-PIE only applies to the main executable, all DSOs are still
PIC and the old change that moved DSOs from 0x7fff to 0x5555 caused us
quite a bit of trouble too, even w/o PIE


>
> -Kees
>
> --
> Kees Cook
> Pixel Security
>

[-- Attachment #2: Type: text/html, Size: 1336 bytes --]

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kees Cook]

On Mon, Aug 7, 2017 at 12:05 PM, Kostya Serebryany <kcc@google.com> wrote:
>
>
> On Mon, Aug 7, 2017 at 11:59 AM, Kees Cook <keescook@google.com> wrote:
>>
>> On Mon, Aug 7, 2017 at 11:56 AM, Kostya Serebryany <kcc@google.com> wrote:
>> > Is it possible to implement some userspace<=>kernel interface that will
>> > allow applications (sanitizers)
>> > to request *fixed* address ranges from the kernel at startup (so that
>> > the
>> > kernel couldn't refuse)?
>>
>> Wouldn't building non-PIE accomplish this?
>
>
> Well, many asan users do need PIE.
> Then, non-PIE only applies to the main executable, all DSOs are still
> PIC and the old change that moved DSOs from 0x7fff to 0x5555 caused us quite
> a bit of trouble too, even w/o PIE

Hm? You can build non-PIE executables leaving all the DSOs PIC.

If what you want is to entirely disable userspace ASLR under *San, you
can just set the ADDR_NO_RANDOMIZE personality flag.

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kostya Serebryany]

[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]

On Mon, Aug 7, 2017 at 12:12 PM, Kees Cook <keescook@google.com> wrote:

> On Mon, Aug 7, 2017 at 12:05 PM, Kostya Serebryany <kcc@google.com> wrote:
> >
> >
> > On Mon, Aug 7, 2017 at 11:59 AM, Kees Cook <keescook@google.com> wrote:
> >>
> >> On Mon, Aug 7, 2017 at 11:56 AM, Kostya Serebryany <kcc@google.com>
> wrote:
> >> > Is it possible to implement some userspace<=>kernel interface that
> will
> >> > allow applications (sanitizers)
> >> > to request *fixed* address ranges from the kernel at startup (so that
> >> > the
> >> > kernel couldn't refuse)?
> >>
> >> Wouldn't building non-PIE accomplish this?
> >
> >
> > Well, many asan users do need PIE.
> > Then, non-PIE only applies to the main executable, all DSOs are still
> > PIC and the old change that moved DSOs from 0x7fff to 0x5555 caused us
> quite
> > a bit of trouble too, even w/o PIE
>
> Hm? You can build non-PIE executables leaving all the DSOs PIC.
>

Yes, but this won't help if the users actually want PIE executables.


>
> If what you want is to entirely disable userspace ASLR under *San, you
> can just set the ADDR_NO_RANDOMIZE personality flag.
>

Mmm. How? Could you please elaborate?
Do you suggest to call personality(ADDR_NO_RANDOMIZE) and re-execute the
process?
Or can I somehow set ADDR_NO_RANDOMIZE at link time?


>
> -Kees
>
> --
> Kees Cook
> Pixel Security
>

[-- Attachment #2: Type: text/html, Size: 2499 bytes --]

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kees Cook]

On Mon, Aug 7, 2017 at 12:16 PM, Kostya Serebryany <kcc@google.com> wrote:
>
>
> On Mon, Aug 7, 2017 at 12:12 PM, Kees Cook <keescook@google.com> wrote:
>>
>> On Mon, Aug 7, 2017 at 12:05 PM, Kostya Serebryany <kcc@google.com> wrote:
>> >
>> >
>> > On Mon, Aug 7, 2017 at 11:59 AM, Kees Cook <keescook@google.com> wrote:
>> >>
>> >> On Mon, Aug 7, 2017 at 11:56 AM, Kostya Serebryany <kcc@google.com>
>> >> wrote:
>> >> > Is it possible to implement some userspace<=>kernel interface that
>> >> > will
>> >> > allow applications (sanitizers)
>> >> > to request *fixed* address ranges from the kernel at startup (so that
>> >> > the
>> >> > kernel couldn't refuse)?
>> >>
>> >> Wouldn't building non-PIE accomplish this?
>> >
>> >
>> > Well, many asan users do need PIE.
>> > Then, non-PIE only applies to the main executable, all DSOs are still
>> > PIC and the old change that moved DSOs from 0x7fff to 0x5555 caused us
>> > quite
>> > a bit of trouble too, even w/o PIE
>>
>> Hm? You can build non-PIE executables leaving all the DSOs PIC.
>
>
> Yes, but this won't help if the users actually want PIE executables.

But who wants a PIE executable that isn't randomized? (Or did I
misunderstand you? You want to allow userspace to declare the
randomization range? Doesn't *San use fixed addresses already, so ASLR
isn't actually a security defense? And if we did have such an
interface it would just lead us right back to security vulnerabilities
like the one this fix was trying to deal with ...)

>> If what you want is to entirely disable userspace ASLR under *San, you
>> can just set the ADDR_NO_RANDOMIZE personality flag.
>
>
> Mmm. How? Could you please elaborate?
> Do you suggest to call personality(ADDR_NO_RANDOMIZE) and re-execute the
> process?
> Or can I somehow set ADDR_NO_RANDOMIZE at link time?

I've normally seen it done with a launcher that sets the personality
and execs the desired executable.

Another future path would be to collapse the PIE load range into the
DSO load range (as now done when a loader executes a PIE binary).

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kostya Serebryany]

[-- Attachment #1: Type: text/plain, Size: 2665 bytes --]

On Mon, Aug 7, 2017 at 12:21 PM, Kees Cook <keescook@google.com> wrote:

> On Mon, Aug 7, 2017 at 12:16 PM, Kostya Serebryany <kcc@google.com> wrote:
> >
> >
> > On Mon, Aug 7, 2017 at 12:12 PM, Kees Cook <keescook@google.com> wrote:
> >>
> >> On Mon, Aug 7, 2017 at 12:05 PM, Kostya Serebryany <kcc@google.com>
> wrote:
> >> >
> >> >
> >> > On Mon, Aug 7, 2017 at 11:59 AM, Kees Cook <keescook@google.com>
> wrote:
> >> >>
> >> >> On Mon, Aug 7, 2017 at 11:56 AM, Kostya Serebryany <kcc@google.com>
> >> >> wrote:
> >> >> > Is it possible to implement some userspace<=>kernel interface that
> >> >> > will
> >> >> > allow applications (sanitizers)
> >> >> > to request *fixed* address ranges from the kernel at startup (so
> that
> >> >> > the
> >> >> > kernel couldn't refuse)?
> >> >>
> >> >> Wouldn't building non-PIE accomplish this?
> >> >
> >> >
> >> > Well, many asan users do need PIE.
> >> > Then, non-PIE only applies to the main executable, all DSOs are still
> >> > PIC and the old change that moved DSOs from 0x7fff to 0x5555 caused us
> >> > quite
> >> > a bit of trouble too, even w/o PIE
> >>
> >> Hm? You can build non-PIE executables leaving all the DSOs PIC.
> >
> >
> > Yes, but this won't help if the users actually want PIE executables.
>
> But who wants a PIE executable that isn't randomized? (Or did I
> misunderstand you? You want to allow userspace to declare the
> randomization range?


Kind of.


> Doesn't *San use fixed addresses already, so ASLR
> isn't actually a security defense?


first of all, *San are not security mitigation tools, and if they weaken
ASLR -- that's fine.
(asan *may* be considered as a mitigation tool even though it weakens ASLR
because it provides stronger memory safety guarantees,
but it's still a weak mitigation, and an expensive one)


> And if we did have such an
> interface it would just lead us right back to security vulnerabilities
> like the one this fix was trying to deal with ...)
>
> >> If what you want is to entirely disable userspace ASLR under *San, you
> >> can just set the ADDR_NO_RANDOMIZE personality flag.
> >
> >
> > Mmm. How? Could you please elaborate?
> > Do you suggest to call personality(ADDR_NO_RANDOMIZE) and re-execute the
> > process?
> > Or can I somehow set ADDR_NO_RANDOMIZE at link time?
>
> I've normally seen it done with a launcher that sets the personality
> and execs the desired executable.
>

Oh, a launcher (e.g. just using setarch) would be a huge pain to deploy.


>
> Another future path would be to collapse the PIE load range into the
> DSO load range (as now done when a loader executes a PIE binary).
>
> -Kees
>
> --
> Kees Cook
> Pixel Security
>

[-- Attachment #2: Type: text/html, Size: 4277 bytes --]

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kees Cook]

(To be clear, this subthread is for dealing with _future_ changes; I'm
already preparing the revert, which is in the other subthread.)

On Mon, Aug 7, 2017 at 12:26 PM, Kostya Serebryany <kcc@google.com> wrote:
> Oh, a launcher (e.g. just using setarch) would be a huge pain to deploy.

Would loading the executable into the mmap region work? We could find
a way to mark executables that want this treatment.

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kostya Serebryany]

[-- Attachment #1: Type: text/plain, Size: 665 bytes --]

On Mon, Aug 7, 2017 at 12:34 PM, Kees Cook <keescook@google.com> wrote:

> (To be clear, this subthread is for dealing with _future_ changes; I'm
> already preparing the revert, which is in the other subthread.)
>
> On Mon, Aug 7, 2017 at 12:26 PM, Kostya Serebryany <kcc@google.com> wrote:
> > Oh, a launcher (e.g. just using setarch) would be a huge pain to deploy.
>
> Would loading the executable into the mmap region work?


This is beyond my knowledge. :(
Could you explain?

If we can do this w/o a launcher (and w/o re-executing), we should try.



> We could find
> a way to mark executables that want this treatment.
>
> --
> Kees Cook
> Pixel Security
>

[-- Attachment #2: Type: text/html, Size: 1322 bytes --]

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Daniel Micay]

On Mon, 2017-08-07 at 12:40 -0700, Kostya Serebryany wrote:
> 
> 
> On Mon, Aug 7, 2017 at 12:34 PM, Kees Cook <keescook@google.com>
> wrote:
> > (To be clear, this subthread is for dealing with _future_ changes;
> > I'm
> > already preparing the revert, which is in the other subthread.)
> > 
> > On Mon, Aug 7, 2017 at 12:26 PM, Kostya Serebryany <kcc@google.com>
> > wrote:
> > > Oh, a launcher (e.g. just using setarch) would be a huge pain to
> > deploy.
> > 
> > Would loading the executable into the mmap region work? 
> 
> This is beyond my knowledge. :( 
> Could you explain? 
> 
> If we can do this w/o a launcher (and w/o re-executing), we should
> try. 

If you launch a program with /usr/lib/ld-2.25.so /usr/bin/foo right now,
that probably already works because it disables the separate base.

There's probably a way to get ASan linked executables to disable the
separate PIE base automatically, making the exe get mapped with other
mmap allocations / libraries. I think that's the best approach.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kees Cook]

On Mon, Aug 7, 2017 at 12:40 PM, Kostya Serebryany <kcc@google.com> wrote:
>
>
> On Mon, Aug 7, 2017 at 12:34 PM, Kees Cook <keescook@google.com> wrote:
>>
>> (To be clear, this subthread is for dealing with _future_ changes; I'm
>> already preparing the revert, which is in the other subthread.)
>>
>> On Mon, Aug 7, 2017 at 12:26 PM, Kostya Serebryany <kcc@google.com> wrote:
>> > Oh, a launcher (e.g. just using setarch) would be a huge pain to deploy.
>>
>> Would loading the executable into the mmap region work?
>
> This is beyond my knowledge. :(
> Could you explain?

PIE has a separate randomization base (before at 0x5555 5555 4000,
currently 0x1 0000 0000) from the mmap (DSO) area (0x7f00 0000
0000-ish). The primary reason to keep PIE separate from mmap is to
avoid leaking ASLR offsets between them, but if that's less of a
concern for *San, then we could just load PIE into the mmap region.

> If we can do this w/o a launcher (and w/o re-executing), we should try.

Let me think about the best way to do this...

-Kees

-- 
Kees Cook
Pixel Security

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Daniel Micay]

On Mon, 2017-08-07 at 19:24 +0200, Dmitry Vyukov wrote:
> Hello,
> 
> The recent "binfmt_elf: use ELF_ET_DYN_BASE only for PIE" patch:
> https://github.com/torvalds/linux/commit/eab09532d40090698b05a07c1c87f
> 39fdbc5fab5
> breaks user-space AddressSanitizer. AddressSanitizer makes assumptions
> about address space layout for substantial performance gains. There
> are multiple people complaining about this already:
> https://github.com/google/sanitizers/issues/837
> https://twitter.com/kayseesee/status/894594085608013825
> https://bugzilla.kernel.org/show_bug.cgi?id=196537
> AddressSanitizer maps shadow memory at [0x00007fff7000-0x10007fff7fff]
> expecting that non-pie binaries will be below 2GB and pie
> binaries/modules will be at 0x55 or 0x7f. This is not the first time
> kernel address space shuffling breaks sanitizers. The last one was the
> move to 0x55.
> 
> Is it possible to make this change less aggressive and keep the
> executable under 2G?

Using < 4G will break Android's usage of 32-bit mappings for ART, and I
think it's fair for them to assume the initial 32-bit space to be unused
with PIE since they control the libc init code, linker, etc. I expect
there are other users of MAP_32BIT / manual low addr mappings that may
break if that area is used / fragmented too.

Starting the PIE base below 4G would be a break to the userspace ABI for
code that is otherwise properly position independent with PIE but uses
the 32-bit address space range in a special way. It also wouldn't solve
the problem here since the range of 64-bit ASLR shifts is very large, so
even if the initial value is 4M it can still be placed above 2G.

The Linux kernel's executable randomization has been broken for a while
since the executable range overlapping with the mmap range and it ends
up falling back to losing the randomization in some cases. It needs to
be lower in the address space to fix that. It doesn't need to be all the
way near the bottom, but it results in less fragmentation to do it that
way.

> In future please be mindful of user-space sanitizers and talk to
> address-sanitizer@googlegroups.com before shuffling address space.

ASan chooses to hard-wire the address range for performance instead of
making it dynamic. I think it was always clearly broken to have it
generate code that isn't position independent when the compiler is being
passed -fPIC or -fPIE. It could have been made dynamic for those while
using the optimized technique for position dependent executables. The
needed address space could even be reserved as part of the executable.

Even without this recent PIE base move, ASan is broken with larger than
default vm.mmap_rnd_bits / vm.mmap_rnd_compat_bits values. It's also
broken with various supported arm64 address space configurations.

Here's an earlier issue filed about this problem with PaX:

https://github.com/google/sanitizers/issues/228

It was closed as WONTFIX 4 years ago, and now mainline has a port of
their design for executable base randomization. PaX UDEREF for x86_64
isn't upstream (yet) and that's also incompatible, since it makes the
address space smaller.

The PIE base change can likely be adjusted to use a high enough address
to leave space for ASan while not usually colliding with the mmap base
range (ignoring non-default stack rlimit possibilities). However, the
problem isn't going to go away as long as ASan is hard-wiring a range
for PIC / PIE code.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>