Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan

[Kostya Serebryany <kcc@google.com>]

[-- 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 --]