From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw0-f199.google.com (mail-yw0-f199.google.com [209.85.161.199]) by kanga.kvack.org (Postfix) with ESMTP id A1F516B02F3 for ; Mon, 7 Aug 2017 15:10:30 -0400 (EDT) Received: by mail-yw0-f199.google.com with SMTP id l82so18875305ywc.1 for ; Mon, 07 Aug 2017 12:10:30 -0700 (PDT) Received: from mail-yw0-x229.google.com (mail-yw0-x229.google.com. [2607:f8b0:4002:c05::229]) by mx.google.com with ESMTPS id o83si2170705yba.634.2017.08.07.12.10.29 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 07 Aug 2017 12:10:29 -0700 (PDT) Received: by mail-yw0-x229.google.com with SMTP id l82so8489199ywc.2 for ; Mon, 07 Aug 2017 12:10:29 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: From: Kostya Serebryany Date: Mon, 7 Aug 2017 12:10:28 -0700 Message-ID: Subject: Re: binfmt_elf: use ELF_ET_DYN_BASE only for PIE breaks asan Content-Type: multipart/alternative; boundary="001a114c8b569bb05005562e99e2" Sender: owner-linux-mm@kvack.org List-ID: To: Kees Cook Cc: Evgenii Stepanov , Dmitry Vyukov , Daniel Micay , Michal Hocko , Andrew Morton , "linux-mm@kvack.org" , Rik van Riel , Reid Kleckner , Peter Collingbourne --001a114c8b569bb05005562e99e2 Content-Type: text/plain; charset="UTF-8" On Mon, Aug 7, 2017 at 12:06 PM, Kees Cook wrote: > On Mon, Aug 7, 2017 at 12:03 PM, Kostya Serebryany wrote: > > > > > > On Mon, Aug 7, 2017 at 11:57 AM, Kees Cook wrote: > >> > >> On Mon, Aug 7, 2017 at 11:51 AM, Evgenii Stepanov > >> wrote: > >> > On Mon, Aug 7, 2017 at 11:40 AM, Kees Cook > 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 > --001a114c8b569bb05005562e99e2 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Mon, Aug 7, 2017 at 12:06 PM, Kees Cook <keescook@google.com&= gt; wrote:
On Mon, Aug 7, 2017 at 12:0= 3 PM, Kostya Serebryany <kcc@google.co= m> 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 wo= uld break
>> >> MSan. Lovely! :P
>> >
>> > That's unfortunate.
>> > This will not help existing binaries, but going forward the m= apping
>> > 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 a= nything
>> > in the range of [0, 1000 0000 0000). That's still not com= patible 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 syste= ms
>> 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 testin= g 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)
>=C2=A0 =C2=A0 =C2=A0 echo "int main(){}" |=C2=A0 clang=C2=A0 = -x c++ -=C2=A0 =C2=A0-fsanitize=3Daddress && ./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

<= div>:)=C2=A0
=C2=A0
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?)
=C2=A0
gcc-based test would be preferred, of course.

gcc implementation lags behind.=C2=A0
Namely,=C2=A0
<= div>gcc has: asan, tsan
gcc does not have msan, asan with dynamic= shadow base, dfsan

So, while having a gcc-only te= sting would have prevented this particular regression, it wouldn't solv= e some similar ones.=C2=A0

--kcc=C2=A0
<= br>

=C2=A0

-Kees

--
Kees Cook
Pixel Security

--001a114c8b569bb05005562e99e2-- -- 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: email@kvack.org