From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 89723C433E0 for ; Thu, 28 May 2020 18:36:06 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3BE5B20814 for ; Thu, 28 May 2020 18:36:06 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="NmpZ+aAW" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3BE5B20814 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id C471E8001A; Thu, 28 May 2020 14:36:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BF70680010; Thu, 28 May 2020 14:36:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AE60F8001A; Thu, 28 May 2020 14:36:05 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0185.hostedemail.com [216.40.44.185]) by kanga.kvack.org (Postfix) with ESMTP id 96ADF80010 for ; Thu, 28 May 2020 14:36:05 -0400 (EDT) Received: from smtpin02.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 574ECDD073 for ; Thu, 28 May 2020 18:36:05 +0000 (UTC) X-FDA: 76866982290.02.robin39_8c66009c7a328 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin02.hostedemail.com (Postfix) with ESMTP id 2EA89DD06C for ; Thu, 28 May 2020 18:36:05 +0000 (UTC) X-HE-Tag: robin39_8c66009c7a328 X-Filterd-Recvd-Size: 15858 Received: from mail-il1-f193.google.com (mail-il1-f193.google.com [209.85.166.193]) by imf26.hostedemail.com (Postfix) with ESMTP for ; Thu, 28 May 2020 18:36:04 +0000 (UTC) Received: by mail-il1-f193.google.com with SMTP id q18so1246199ilm.5 for ; Thu, 28 May 2020 11:36:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=J04xJphs20RSM8GaZG4Nx6XpGcbUnXZxb+v0CPz7yB4=; b=NmpZ+aAWdOi2Ld45kxHdFeTDw/PrA9XyeOZ/pqeDM7mj5hmCBB01T0ndMT0GO35jun T7LeW8AaCn2UWKrw1Us0w9f3pOmkNMVaivyYgBn9LM+iHCLNkY8iN44EJQrpZXKzlsix V/3+iPv9OuaMwjOCSztIoyON9yue2LPNIjgZIB3vCpQXuH5W7qCwsjD5IapvBjVpwoFj kT12eqGCQNMvWFGsZbhWbV3Pltaw/qkDxZiWLgn8NqZI0j/PSyJTQakP+q23vE6RK/rq dcG3pQmu0FgTu6VWUIGq+CiRl7EFswIGUVfuyNkRhnp2cK754T+k1PqYETDGB67WI2cc Iemw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=J04xJphs20RSM8GaZG4Nx6XpGcbUnXZxb+v0CPz7yB4=; b=psEzUJqdu0GMA6nwZi3Ga/IyFcgMwUJBIYmKIvKd8aEk8+gC3UxCC3jBIzwBbC71On YfeZLHHKq/uVP5QloYo6TeVeAVsK5z4X+BgJo5e77a0VQCYkLuojZYTwBnDlURHw8BO2 /dAjTMuvXFHiNdWXtHW/dh4QTG93seudamqnt5M7afg5lEUgeRmhXp7/BDInQ+a95v7Y zLZdzFRJTy0eQ9jwcutjsF3Nj65SyEhrw1gEp4SamwHcCsnzlo9nchSrti1kEXuZRM4o Vy5Ub/fwHJVH3k3oAoatt4VNhtggrmcghBdkM4xZwwP6Fpkz01VoQ9AspOFGvfcaXWRS vZfw== X-Gm-Message-State: AOAM532O+Ji/ftuKGJ4gYM30PgwjCP8NPLWlDc2c2JK2qLDALvRtt2w0 tUzJrcGVdZhZLdt7AyvOQC+4SzdX9RYqUple4+jk5Q== X-Google-Smtp-Source: ABdhPJxWtlWLH91vdDL/p7LXw5pxVDaoyqndzkG+uVf1JKOaP4pL9rutyAmOksRmB8xHUP3nCRiMlMHDmSuebgpfJrg= X-Received: by 2002:a92:c746:: with SMTP id y6mr3883281ilp.116.1590690963843; Thu, 28 May 2020 11:36:03 -0700 (PDT) MIME-Version: 1.0 References: <20200515171612.1020-1-catalin.marinas@arm.com> <20200515171612.1020-12-catalin.marinas@arm.com> <20200528091445.GA2961@gaia> <20200528110509.GA18623@arm.com> <20200528163412.GC2961@gaia> In-Reply-To: <20200528163412.GC2961@gaia> From: Evgenii Stepanov Date: Thu, 28 May 2020 11:35:50 -0700 Message-ID: Subject: Re: [PATCH v4 11/26] arm64: mte: Add PROT_MTE support to mmap() and mprotect() To: Catalin Marinas Cc: Szabolcs Nagy , Peter Collingbourne , Linux ARM , Linux Memory Management List , linux-arch , Will Deacon , Dave P Martin , Vincenzo Frascino , Kevin Brodsky , Andrey Konovalov , nd Content-Type: multipart/alternative; boundary="000000000000d9931405a6b99917" X-Rspamd-Queue-Id: 2EA89DD06C X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam01 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: --000000000000d9931405a6b99917 Content-Type: text/plain; charset="UTF-8" On Thu, May 28, 2020 at 9:34 AM Catalin Marinas wrote: > On Thu, May 28, 2020 at 12:05:09PM +0100, Szabolcs Nagy wrote: > > The 05/28/2020 10:14, Catalin Marinas wrote: > > > On Wed, May 27, 2020 at 11:57:39AM -0700, Peter Collingbourne wrote: > > > > On Fri, May 15, 2020 at 10:16 AM Catalin Marinas > > > > wrote: > > > > > To enable tagging on a memory range, the user must explicitly opt > in via > > > > > a new PROT_MTE flag passed to mmap() or mprotect(). Since this is > a new > > > > > memory type in the AttrIndx field of a pte, simplify the or'ing of > these > > > > > bits over the protection_map[] attributes by making MT_NORMAL > index 0. > > > > > > > > Should the userspace stack always be mapped as if with PROT_MTE if > the > > > > hardware supports it? Such a change would be invisible to non-MTE > > > > aware userspace since it would already need to opt in to tag checking > > > > via prctl. This would let userspace avoid a complex stack > > > > initialization sequence when running with stack tagging enabled on > the > > > > main thread. > > > > > > I don't think the stack initialisation is that difficult. On program > > > startup (can be the dynamic loader). Something like (untested): > > > > > > register unsigned long stack asm ("sp"); > > > unsigned long page_sz = sysconf(_SC_PAGESIZE); > > > > > > mprotect((void *)(stack & ~(page_sz - 1)), page_sz, > > > PROT_READ | PROT_WRITE | PROT_MTE | PROT_GROWSDOWN); > > > > > > (the essential part it PROT_GROWSDOWN so that you don't have to specify > > > a stack lower limit) > > > > does this work even if the currently mapped stack is more than page_sz? > > determining the mapped main stack area is i think non-trivial to do in > > userspace (requires parsing /proc/self/maps or similar). > > Because of PROT_GROWSDOWN, the kernel adjusts the start of the range > down automatically. It is potentially problematic if the top of the > stack is more than a page away and you want the whole stack coloured. I > haven't run a test but my reading of the kernel code is that the stack > vma would be split in this scenario, so the range beyond sp+page_sz > won't have PROT_MTE set. > > My assumption is that if you do this during program start, the stack is > smaller than a page. Alternatively, could we use argv or envp to > determine the top of the user stack (the bottom is taken care of by the > kernel)? > PROT_GROWSDOWN seems to work fine in our case, and the extra tag maintenance overhead sounds like a valid argument against setting PROT_MTE unconditionally. On the other hand, we may end up doing this in the userspace in every process. The reason is, PROT_MTE can not be set on a page that contains a live frame with stack tagging because of mismatching tags (IRG is not affected by PROT_MTE but STG is). So ideally, this should be done at (or near) the program entry point, while the stack is mostly empty. > > > > I'm fine, however, with enabling PROT_MTE on the main stack based on > > > some ELF note. > > > > note that would likely mean an elf note on the dynamic linker > > (because a dynamic linked executable may not be loaded by the > > kernel and ctors in loaded libs run before the executable entry > > code anyway, so the executable alone cannot be in charge of this > > decision) i.e. one global switch for all dynamic linked binaries. > > I guess parsing such note in the kernel is only useful for static > binaries. > > > i think a dynamic linker can map a new stack and switch to it > > if it needs to control the properties of the stack at runtime > > (it's wasteful though). > > There is already user code to check for HWCAP2_MTE and the prctl(), so > adding an mprotect() doesn't look like a significant overhead. > > > and i think there should be a runtime mechanism for the brk area: > > it should be possible to request that future brk expansions are > > mapped as PROT_MTE so an mte aware malloc implementation can use > > brk. i think this is not important in the initial design, but if > > a prctl flag can do it that may be useful to add (may be at a > > later time). > > Looking at the kernel code briefly, I think this would work. We do end > up with two vmas for the brk, only the expansion having PROT_MTE, and > I'd to find a way to store the extra flag. > > From a coding perspective, it's easier to just set PROT_MTE by default > on both brk and initial stack ;) (VM_DATA_DEFAULT_FLAGS). > > > (and eventually there should be a way to use PROT_MTE on > > writable global data and appropriate code generation that > > takes colors into account when globals are accessed, but > > that requires significant ELF, ld.so and compiler changes, > > that need not be part of the initial mte design). > > The .data section needs to be driven by the ELF information. It's also a > file mapping and we don't support PROT_MTE on them even if MAP_PRIVATE. > There are complications like DAX where the file you mmap for CoW may be > hosted on memory that does not support MTE (copied to RAM on write). > > Is there a use-case for global data to be tagged? > Yes, catching global buffer overflow bugs. They are not nearly as common as heap-based issues though. > > -- > Catalin > --000000000000d9931405a6b99917 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Thu, May 28, 2020 at 9:34 AM Catal= in Marinas <catalin.marinas@a= rm.com> wrote:
On Thu, May 28, 2020 at 12:05:09PM +0100, Szabolcs Nagy wrote:
> The 05/28/2020 10:14, Catalin Marinas wrote:
> > On Wed, May 27, 2020 at 11:57:39AM -0700, Peter Collingbourne wro= te:
> > > On Fri, May 15, 2020 at 10:16 AM Catalin Marinas
> > > <catalin.marinas@arm.com> wrote:
> > > > To enable tagging on a memory range, the user must expl= icitly opt in via
> > > > a new PROT_MTE flag passed to mmap() or mprotect(). Sin= ce this is a new
> > > > memory type in the AttrIndx field of a pte, simplify th= e or'ing of these
> > > > bits over the protection_map[] attributes by making MT_= NORMAL index 0.
> > >
> > > Should the userspace stack always be mapped as if with PROT_= MTE if the
> > > hardware supports it? Such a change would be invisible to no= n-MTE
> > > aware userspace since it would already need to opt in to tag= checking
> > > via prctl. This would let userspace avoid a complex stack > > > initialization sequence when running with stack tagging enab= led on the
> > > main thread.
> >
> > I don't think the stack initialisation is that difficult. On = program
> > startup (can be the dynamic loader). Something like (untested): > >
> >=C2=A0 =C2=A0 =C2=A0register unsigned long stack asm ("sp&quo= t;);
> >=C2=A0 =C2=A0 =C2=A0unsigned long page_sz =3D sysconf(_SC_PAGESIZE= );
> >
> >=C2=A0 =C2=A0 =C2=A0mprotect((void *)(stack & ~(page_sz - 1)),= page_sz,
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 PROT_READ | PROT_= WRITE | PROT_MTE | PROT_GROWSDOWN);
> >
> > (the essential part it PROT_GROWSDOWN so that you don't have = to specify
> > a stack lower limit)
>
> does this work even if the currently mapped stack is more than page_sz= ?
> determining the mapped main stack area is i think non-trivial to do in=
> userspace (requires parsing /proc/self/maps or similar).

Because of PROT_GROWSDOWN, the kernel adjusts the start of the range
down automatically. It is potentially problematic if the top of the
stack is more than a page away and you want the whole stack coloured. I
haven't run a test but my reading of the kernel code is that the stack<= br> vma would be split in this scenario, so the range beyond sp+page_sz
won't have PROT_MTE set.

My assumption is that if you do this during program start, the stack is
smaller than a page. Alternatively, could we use argv or envp to
determine the top of the user stack (the bottom is taken care of by the
kernel)?

PROT_GROWSDOWN seems to work fine in our case, and the extra tag maint= enance overhead sounds like a valid argument against setting PROT_MTE uncon= ditionally.

On the other hand, we may= end up doing this in the userspace in every process. The reason is, PROT_M= TE can not be set on a page that contains a live frame with stack tagging b= ecause of mismatching tags (IRG is not affected by PROT_MTE but STG is). So= ideally, this should be done at (or near) the program entry point, while t= he stack is mostly empty.
=C2=A0

> > I'm fine, however, with enabling PROT_MTE on the main stack b= ased on
> > some ELF note.
>
> note that would likely mean an elf note on the dynamic linker
> (because a dynamic linked executable may not be loaded by the
> kernel and ctors in loaded libs run before the executable entry
> code anyway, so the executable alone cannot be in charge of this
> decision) i.e. one global switch for all dynamic linked binaries.

I guess parsing such note in the kernel is only useful for static
binaries.

> i think a dynamic linker can map a new stack and switch to it
> if it needs to control the properties of the stack at runtime
> (it's wasteful though).

There is already user code to check for HWCAP2_MTE and the prctl(), so
adding an mprotect() doesn't look like a significant overhead.

> and i think there should be a runtime mechanism for the brk area:
> it should be possible to request that future brk expansions are
> mapped as PROT_MTE so an mte aware malloc implementation can use
> brk. i think this is not important in the initial design, but if
> a prctl flag can do it that may be useful to add (may be at a
> later time).

Looking at the kernel code briefly, I think this would work. We do end
up with two vmas for the brk, only the expansion having PROT_MTE, and
I'd to find a way to store the extra flag.

>From a coding perspective, it's easier to just set PROT_MTE by default<= br> on both brk and initial stack ;) (VM_DATA_DEFAULT_FLAGS).

> (and eventually there should be a way to use PROT_MTE on
> writable global data and appropriate code generation that
> takes colors into account when globals are accessed, but
> that requires significant ELF, ld.so and compiler changes,
> that need not be part of the initial mte design).

The .data section needs to be driven by the ELF information. It's also = a
file mapping and we don't support PROT_MTE on them even if MAP_PRIVATE.=
There are complications like DAX where the file you mmap for CoW may be
hosted on memory that does not support MTE (copied to RAM on write).

Is there a use-case for global data to be tagged?

=
Yes, catching global buffer overflow bugs. They are not nearly a= s common as heap-based issues though.
=C2=A0

--
Catalin
--000000000000d9931405a6b99917--