Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: Khalid Aziz <khalid.aziz@oracle.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	kvm@vger.kernel.org, Christian Koenig <Christian.Koenig@amd.com>,
	Szabolcs Nagy <Szabolcs.Nagy@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	dri-devel@lists.freedesktop.org, linux-mm@kvack.org,
	Lee Smith <Lee.Smith@arm.com>,
	linux-kselftest@vger.kernel.org,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Jacob Bramley <Jacob.Bramley@arm.com>,
	Leon Romanovsky <leon@kernel.org>,
	linux-rdma@vger.kernel.org, amd-gfx@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org,
	Dave Martin <Dave.Martin@arm.com>,
	Evgeniy Stepanov <eugenis@google.com>,
	linux-media@vger.kernel.org, Kees Cook <keescook@chromium.org>,
	Ruben Ayrapetyan <Ruben.Ayrapetyan@arm.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	Kevin Brodsky <kevin.brodsky@arm.com>,
	Alex Williamson <alex.williamson@redhat.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Kostya Serebryany <kcc@google.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Felix Kuehling <Felix.Kuehling@amd.com>,
	linux-kernel@vger.kernel.org,
	Jens Wiklander <jens.wiklander@linaro.org>,
	Ramana Radhakrishnan <Ramana.Radhakrishnan@arm.com>,
	Alexander Deucher <Alexander.Deucher@amd.com>,
	Andrew Murray <andrew.murray@arm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Yishai Hadas <yishaih@mellanox.com>,
	Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Subject: Re: [PATCH v15 05/17] arms64: untag user pointers passed to memory syscalls
Date: Thu, 30 May 2019 16:11:07 +0100
Message-ID: <20190530151105.GA35418@arrakis.emea.arm.com> (raw)
In-Reply-To: <b2753e81-7b57-481f-0095-3c6fecb1a74c@oracle.com>

On Wed, May 29, 2019 at 01:16:37PM -0600, Khalid Aziz wrote:
> On 5/29/19 8:20 AM, Catalin Marinas wrote:
> > On Tue, May 28, 2019 at 05:33:04PM -0600, Khalid Aziz wrote:
> >> Steps 1 and 2 are accomplished by userspace by calling mprotect() with
> >> PROT_ADI. Tags are set by storing tags in a loop, for example:
> >>
> >>         version = 10;
> >>         tmp_addr = shmaddr;
> >>         end = shmaddr + BUFFER_SIZE;
> >>         while (tmp_addr < end) {
> >>                 asm volatile(
> >>                         "stxa %1, [%0]0x90\n\t"
> >>                         :
> >>                         : "r" (tmp_addr), "r" (version));
> >>                 tmp_addr += adi_blksz;
> >>         }
> > 
> > On arm64, a sequence similar to the above would live in the libc. So a
> > malloc() call will tag the memory and return the tagged address to thePre-coloring could easily be done by 
> > user.
> > 
> > We were not planning for a PROT_ADI/MTE but rather have MTE enabled for
> > all user memory ranges. We may revisit this before we upstream the MTE
> > support (probably some marginal benefit for the hardware not fetching
> > the tags from memory if we don't need to, e.g. code sections).
> > 
> > Given that we already have the TBI feature and with MTE enabled the top
> > byte is no longer ignored, we are planning for an explicit opt-in by the
> > user via prctl() to enable MTE.
> 
> OK. I had initially proposed enabling ADI for a process using prctl().
> Feedback I got was prctl was not a desirable interface and I ended up
> making mprotect() with PROT_ADI enable ADI on the process instead. Just
> something to keep in mind.

Thanks for the feedback. We'll keep this in mind when adding MTE
support. In the way we plan to deploy this, it would be a libc decision
to invoke the mmap() with the right flag.

This could actually simplify the automatic page faulting below brk(),
basically no tagged/coloured memory allowed implicitly. It needs
feedback from the bionic/glibc folk.

> >> With these semantics, giving mmap() or shamat() a tagged address is
> >> meaningless since no tags have been stored at the addresses mmap() will
> >> allocate and one can not store tags before memory range has been
> >> allocated. If we choose to allow tagged addresses to come into mmap()
> >> and shmat(), sparc code can strip the tags unconditionally and that may
> >> help simplify ABI and/or code.
> > 
> > We could say that with TBI (pre-MTE support), the top byte is actually
> > ignored on mmap(). Now, if you pass a MAP_FIXED with a tagged address,
> > should the user expect the same tagged address back or stripping the tag
> > is acceptable? If we want to keep the current mmap() semantics, I'd say
> > the same tag is returned. However, with MTE this also implies that the
> > memory was coloured.
> 
> Is assigning a tag aprivileged operationon ARM64? I am thinking not
> since you mentioned libc could do it in a loop for malloc'd memory.

Indeed it's not, the user can do it.

> mmap() can return the same tagged address but I am uneasy about kernel
> pre-coloring the pages. Database can mmap 100's of GB of memory. That is
> lot of work being offloaded to the kernel to pre-color the page even if
> done in batches as pages are faulted in.

For anonymous mmap() for example, the kernel would have to zero the
faulted in pages anyway. We can handle the colouring at the same time in
clear_user_page() (as I said below, we have to clear the colour anyway
from previous uses, so it's simply extending this to support something
other than tag/colour 0 by default with no additional overhead).

> > Since the user can probe the pre-existing colour in a faulted-in page
> > (either with some 'ldxa' instruction or by performing a tag-checked
> > access), the kernel should always pre-colour (even if colour 0) any
> > allocated page. There might not be an obvious security risk but I feel
> > uneasy about letting colours leak between address spaces (different user
> > processes or between kernel and user).
> 
> On sparc, tags 0 and 15 are special in that 0 means untagged memory and
> 15 means match any tag in the address. Colour 0 is the default for any
> newly faulted in page on sparc.

With MTE we don't have match-all/any tag in memory, only in the virtual
address/pointer. So if we turn on MTE for all pages and the user
accesses an address with a 0 tag, the underlying memory needs to be
coloured with the same 0 value.

> > Since we already need such loop in the kernel, we might as well allow
> > user space to require a certain colour. This comes in handy for large
> > malloc() and another advantage is that the C library won't be stuck
> > trying to paint the whole range (think GB).
> 
> If kernel is going to pre-color all pages in a vma, we will need to
> store the default tag in the vma. It will add more time to page fault
> handling code. On sparc M7, kernel will need to execute additional 128
> stxa instructions to set the tags on a normal page.

As I said, since the user can retrieve an old colour using ldxa, the
kernel should perform this operation anyway on any newly allocated page
(unless you clear the existing colour on page freeing).

> >> We can try to store tags for an entire region in vma but that is
> >> expensive, plus on sparc tags are set in userspace with no
> >> participation from kernel and now we need a way for userspace to
> >> communicate the tags to kernel.
> > 
> > We can't support finer granularity through the mmap() syscall and, as
> > you said, the vma is not the right thing to store the individual tags.
> > With the above extension to mmap(), we'd have to store a colour per vma
> > and prevent merging if different colours (we could as well use the
> > pkeys mechanism we already have in the kernel but use a colour per vma
> > instead of a key).
> 
> Since tags can change on any part of mmap region on sparc at any time
> without kernel being involved, I am not sure I see much reason for
> kernel to enforce any tag related restrictions.

It's not enforcing a tag, more like the default colour for a faulted in
page. Anyway, if sparc is going with default 0/untagged, that's fine as
well. We may add this mmap() option to arm64 only.

> >> From sparc point of view, making kernel responsible for assigning tags
> >> to a page on page fault is full of pitfalls.
> > 
> > This could be just some arm64-specific but if you plan to deploy it more
> > generically for sparc (at the C library level), you may find this
> > useful.
> 
> Common semantics from app developer point of view will be very useful to
> maintain. If arm64 says mmap with MAP_FIXED and a tagged address will
> return a pre-colored page, I would rather have it be the same on any
> architecture. Is there a use case that justifies kernel doing this extra
> work?

So if a database program is doing an anonymous mmap(PROT_TBI) of 100GB,
IIUC for sparc the faulted-in pages will have random colours (on 64-byte
granularity). Ignoring the information leak from prior uses of such
pages, it would be the responsibility of the db program to issue the
stxa. On arm64, since we also want to do this via malloc(), any large
allocation would require all pages to be faulted in so that malloc() can
set the write colour before being handed over to the user. That's what
we want to avoid and the user is free to repaint the memory as it likes.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply index

Thread overview: 99+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-06 16:30 [PATCH v15 00/17] arm64: untag user pointers passed to the kernel Andrey Konovalov
2019-05-06 16:30 ` [PATCH v15 01/17] uaccess: add untagged_addr definition for other arches Andrey Konovalov
2019-05-29 14:49   ` Khalid Aziz
2019-05-06 16:30 ` [PATCH v15 02/17] arm64: untag user pointers in access_ok and __uaccess_mask_ptr Andrey Konovalov
2019-05-06 16:30 ` [PATCH v15 03/17] lib, arm64: untag user pointers in strn*_user Andrey Konovalov
2019-05-22 10:41   ` Catalin Marinas
2019-05-06 16:30 ` [PATCH v15 04/17] mm: add ksys_ wrappers to memory syscalls Andrey Konovalov
2019-05-22 10:56   ` Catalin Marinas
2019-05-06 16:30 ` [PATCH v15 05/17] arms64: untag user pointers passed " Andrey Konovalov
2019-05-22 11:49   ` Catalin Marinas
2019-05-22 21:16     ` Evgenii Stepanov
2019-05-23  9:04       ` Catalin Marinas
2019-05-24  4:23         ` Evgenii Stepanov
2019-05-24 15:41   ` Andrew Murray
2019-05-25  9:57   ` Catalin Marinas
2019-05-27  9:42   ` Catalin Marinas
2019-05-27 14:37   ` Catalin Marinas
2019-05-28 14:54     ` Andrew Murray
2019-05-28 15:40       ` Catalin Marinas
2019-05-28 15:56         ` Dave Martin
2019-05-28 16:34           ` Catalin Marinas
2019-05-29 12:42             ` Dave Martin
2019-05-29 13:23               ` Catalin Marinas
2019-05-29 15:18                 ` Dave Martin
2019-05-28 23:33         ` Khalid Aziz
2019-05-29 14:20           ` Catalin Marinas
2019-05-29 19:16             ` Khalid Aziz
2019-05-30 15:11               ` Catalin Marinas [this message]
2019-05-30 16:05                 ` Khalid Aziz
2019-05-30 16:57                   ` Catalin Marinas
2019-05-28 13:05   ` Catalin Marinas
2019-05-06 16:30 ` [PATCH v15 06/17] mm: untag user pointers in do_pages_move Andrey Konovalov
2019-05-22 11:51   ` Catalin Marinas
2019-05-06 16:30 ` [PATCH v15 07/17] mm, arm64: untag user pointers in mm/gup.c Andrey Konovalov
2019-05-22 11:56   ` Catalin Marinas
2019-05-06 16:30 ` [PATCH v15 08/17] mm, arm64: untag user pointers in get_vaddr_frames Andrey Konovalov
2019-05-06 16:30 ` [PATCH v15 09/17] fs, arm64: untag user pointers in copy_mount_options Andrey Konovalov
2019-05-22 12:09   ` Catalin Marinas
2019-05-06 16:30 ` [PATCH v15 10/17] fs, arm64: untag user pointers in fs/userfaultfd.c Andrey Konovalov
2019-05-06 16:30 ` [PATCH v15 11/17] drm/amdgpu, arm64: untag user pointers Andrey Konovalov
2019-05-07 16:43   ` Kuehling, Felix
2019-05-06 16:30 ` [PATCH v15 12/17] drm/radeon, arm64: untag user pointers in radeon_gem_userptr_ioctl Andrey Konovalov
2019-05-07 16:44   ` Kuehling, Felix
2019-05-06 16:30 ` [PATCH v15 13/17] IB, arm64: untag user pointers in ib_uverbs_(re)reg_mr() Andrey Konovalov
2019-05-06 19:50   ` Jason Gunthorpe
2019-05-07  6:33     ` Leon Romanovsky
2019-05-06 16:31 ` [PATCH v15 14/17] media/v4l2-core, arm64: untag user pointers in videobuf_dma_contig_user_get Andrey Konovalov
2019-05-24 13:13   ` Mauro Carvalho Chehab
2019-05-06 16:31 ` [PATCH v15 15/17] tee, arm64: untag user pointers in tee_shm_register Andrey Konovalov
2019-05-06 16:31 ` [PATCH v15 16/17] vfio/type1, arm64: untag user pointers in vaddr_get_pfn Andrey Konovalov
2019-05-06 16:31 ` [PATCH v15 17/17] selftests, arm64: add a selftest for passing tagged pointers to kernel Andrey Konovalov
2019-05-22 14:16   ` Catalin Marinas
2019-05-31 14:21     ` Andrey Konovalov
2019-05-31 16:22       ` Catalin Marinas
2019-05-17 14:49 ` [PATCH v15 00/17] arm64: untag user pointers passed to the kernel Catalin Marinas
2019-05-20 23:53   ` Evgenii Stepanov
2019-05-21 18:29     ` Catalin Marinas
2019-05-22  0:04       ` Kees Cook
2019-05-22 10:11         ` Catalin Marinas
2019-05-22 15:30           ` enh
2019-05-22 16:35             ` Catalin Marinas
2019-05-22 16:58               ` enh
2019-05-23 15:21                 ` Catalin Marinas
2019-05-22 20:47               ` Kees Cook
2019-05-22 23:03                 ` Evgenii Stepanov
2019-05-22 23:09                   ` enh
2019-05-23  7:34                     ` Catalin Marinas
2019-05-23 14:44                 ` Catalin Marinas
2019-05-23 15:44                   ` enh
2019-05-23 17:00                     ` Catalin Marinas
2019-05-23 16:38                   ` Kees Cook
2019-05-23 17:43                     ` Catalin Marinas
2019-05-23 21:31                       ` Kees Cook
2019-05-24 11:20                         ` Catalin Marinas
2019-05-28 17:02                         ` Catalin Marinas
2019-06-02  5:06                           ` Kees Cook
2019-05-22 19:21             ` Kees Cook
2019-05-22 20:15               ` enh
2019-05-23 15:08               ` Catalin Marinas
2019-05-23 17:51         ` Khalid Aziz
2019-05-23 20:11           ` Catalin Marinas
2019-05-23 21:42             ` Khalid Aziz
2019-05-23 21:49             ` Khalid Aziz
2019-05-24 10:11               ` Catalin Marinas
2019-05-24 14:25                 ` Khalid Aziz
2019-05-28 14:14                   ` Andrey Konovalov
2019-05-29  6:11                     ` Christoph Hellwig
2019-05-29 12:12                       ` Catalin Marinas
2019-05-30 17:15                     ` Catalin Marinas
2019-05-31 14:29                       ` Andrey Konovalov
2019-05-31 16:19                         ` Catalin Marinas
2019-05-31 16:24                           ` Andrey Konovalov
2019-05-31 16:46                             ` Catalin Marinas
2019-05-21 18:48   ` Jason Gunthorpe
2019-05-22 13:49     ` Dave Martin
2019-05-23  0:20       ` Jason Gunthorpe
2019-05-23 10:42         ` Dave Martin
2019-05-23 16:57           ` Catalin Marinas
2019-05-24 14:23             ` Dave Martin

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190530151105.GA35418@arrakis.emea.arm.com \
    --to=catalin.marinas@arm.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=Christian.Koenig@amd.com \
    --cc=Dave.Martin@arm.com \
    --cc=Felix.Kuehling@amd.com \
    --cc=Jacob.Bramley@arm.com \
    --cc=Lee.Smith@arm.com \
    --cc=Ramana.Radhakrishnan@arm.com \
    --cc=Ruben.Ayrapetyan@arm.com \
    --cc=Szabolcs.Nagy@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex.williamson@redhat.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=andrew.murray@arm.com \
    --cc=andreyknvl@google.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=dvyukov@google.com \
    --cc=eugenis@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jens.wiklander@linaro.org \
    --cc=kcc@google.com \
    --cc=keescook@chromium.org \
    --cc=kevin.brodsky@arm.com \
    --cc=khalid.aziz@oracle.com \
    --cc=kvm@vger.kernel.org \
    --cc=leon@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=luc.vanoostenryck@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=mchehab@kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=vincenzo.frascino@arm.com \
    --cc=will.deacon@arm.com \
    --cc=yishaih@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org infradead-linux-arm-kernel@archiver.kernel.org
	public-inbox-index linux-arm-kernel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox