Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
From: Evgenii Stepanov <eugenis@google.com>
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: Andrey Konovalov <andreyknvl@google.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Linux Memory Management List <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-rdma@vger.kernel.org, linux-media@vger.kernel.org,
	kvm@vger.kernel.org,
	"open list:KERNEL SELFTEST FRAMEWORK" 
	<linux-kselftest@vger.kernel.org>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Kees Cook <keescook@chromium.org>,
	Yishai Hadas <yishaih@mellanox.com>,
	Felix Kuehling <Felix.Kuehling@amd.com>,
	Alexander Deucher <Alexander.Deucher@amd.com>,
	Christian Koenig <Christian.Koenig@amd.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Jens Wiklander <jens.wiklander@linaro.org>,
	Alex Williamson <alex.williamson@redhat.com>,
	Leon Romanovsky <leon@kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Kostya Serebryany <kcc@google.com>, Lee Smith <Lee.Smith@arm.com>,
	Ramana Radhakrishnan <Ramana.Radhakrishnan@arm.com>,
	Jacob Bramley <Jacob.Bramley@arm.com>,
	Ruben Ayrapetyan <Ruben.Ayrapetyan@arm.com>,
	Robin Murphy <robin.murphy@arm.com>,
	Luc Van Oostenryck <luc.vanoostenryck@gmail.com>,
	Dave Martin <Dave.Martin@arm.com>,
	Kevin Brodsky <kevin.brodsky@arm.com>,
	Szabolcs Nagy <Szabolcs.Nagy@arm.com>
Subject: Re: [PATCH v15 05/17] arms64: untag user pointers passed to memory syscalls
Date: Wed, 22 May 2019 14:16:57 -0700
Message-ID: <CAFKCwrjyP+x0JJy=qpBFsp4pub3He6UkvU0qnf1UOKt6W1LPRQ@mail.gmail.com> (raw)
In-Reply-To: <20190522114910.emlckebwzv2qz42i@mbp>

On Wed, May 22, 2019 at 4:49 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Mon, May 06, 2019 at 06:30:51PM +0200, Andrey Konovalov wrote:
> > This patch is a part of a series that extends arm64 kernel ABI to allow to
> > pass tagged user pointers (with the top byte set to something else other
> > than 0x00) as syscall arguments.
> >
> > This patch allows tagged pointers to be passed to the following memory
> > syscalls: brk, get_mempolicy, madvise, mbind, mincore, mlock, mlock2,
> > mmap, mmap_pgoff, mprotect, mremap, msync, munlock, munmap,
> > remap_file_pages, shmat and shmdt.
> >
> > This is done by untagging pointers passed to these syscalls in the
> > prologues of their handlers.
>
> I'll go through them one by one to see if we can tighten the expected
> ABI while having the MTE in mind.
>
> > diff --git a/arch/arm64/kernel/sys.c b/arch/arm64/kernel/sys.c
> > index b44065fb1616..933bb9f3d6ec 100644
> > --- a/arch/arm64/kernel/sys.c
> > +++ b/arch/arm64/kernel/sys.c
> > @@ -35,10 +35,33 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
> >  {
> >       if (offset_in_page(off) != 0)
> >               return -EINVAL;
> > -
> > +     addr = untagged_addr(addr);
> >       return ksys_mmap_pgoff(addr, len, prot, flags, fd, off >> PAGE_SHIFT);
> >  }
>
> If user passes a tagged pointer to mmap() and the address is honoured
> (or MAP_FIXED is given), what is the expected return pointer? Does it
> need to be tagged with the value from the hint?

For HWASan the most convenient would be to use the tag from the hint.
But since in the TBI (not MTE) mode the kernel has no idea what
meaning userspace assigns to pointer tags, perhaps it should not try
to guess, and should return raw (zero-tagged) address instead.

> With MTE, we may want to use this as a request for the default colour of
> the mapped pages (still under discussion).

I like this - and in that case it would make sense to return the
pointer that can be immediately dereferenced without crashing the
process, i.e. with the matching tag.

> > +SYSCALL_DEFINE6(arm64_mmap_pgoff, unsigned long, addr, unsigned long, len,
> > +             unsigned long, prot, unsigned long, flags,
> > +             unsigned long, fd, unsigned long, pgoff)
> > +{
> > +     addr = untagged_addr(addr);
> > +     return ksys_mmap_pgoff(addr, len, prot, flags, fd, pgoff);
> > +}
>
> We don't have __NR_mmap_pgoff on arm64.
>
> > +SYSCALL_DEFINE5(arm64_mremap, unsigned long, addr, unsigned long, old_len,
> > +             unsigned long, new_len, unsigned long, flags,
> > +             unsigned long, new_addr)
> > +{
> > +     addr = untagged_addr(addr);
> > +     new_addr = untagged_addr(new_addr);
> > +     return ksys_mremap(addr, old_len, new_len, flags, new_addr);
> > +}
>
> Similar comment as for mmap(), do we want the tag from new_addr to be
> preserved? In addition, should we check that the two tags are identical
> or mremap() should become a way to repaint a memory region?
>
> > +SYSCALL_DEFINE2(arm64_munmap, unsigned long, addr, size_t, len)
> > +{
> > +     addr = untagged_addr(addr);
> > +     return ksys_munmap(addr, len);
> > +}
>
> This looks fine.
>
> > +SYSCALL_DEFINE1(arm64_brk, unsigned long, brk)
> > +{
> > +     brk = untagged_addr(brk);
> > +     return ksys_brk(brk);
> > +}
>
> I wonder whether brk() should simply not accept tags, and should not
> return them (similar to the prctl(PR_SET_MM) discussion). We could
> document this in the ABI requirements.
>
> > +SYSCALL_DEFINE5(arm64_get_mempolicy, int __user *, policy,
> > +             unsigned long __user *, nmask, unsigned long, maxnode,
> > +             unsigned long, addr, unsigned long, flags)
> > +{
> > +     addr = untagged_addr(addr);
> > +     return ksys_get_mempolicy(policy, nmask, maxnode, addr, flags);
> > +}
> > +
> > +SYSCALL_DEFINE3(arm64_madvise, unsigned long, start,
> > +             size_t, len_in, int, behavior)
> > +{
> > +     start = untagged_addr(start);
> > +     return ksys_madvise(start, len_in, behavior);
> > +}
> > +
> > +SYSCALL_DEFINE6(arm64_mbind, unsigned long, start, unsigned long, len,
> > +             unsigned long, mode, const unsigned long __user *, nmask,
> > +             unsigned long, maxnode, unsigned int, flags)
> > +{
> > +     start = untagged_addr(start);
> > +     return ksys_mbind(start, len, mode, nmask, maxnode, flags);
> > +}
> > +
> > +SYSCALL_DEFINE2(arm64_mlock, unsigned long, start, size_t, len)
> > +{
> > +     start = untagged_addr(start);
> > +     return ksys_mlock(start, len, VM_LOCKED);
> > +}
> > +
> > +SYSCALL_DEFINE2(arm64_mlock2, unsigned long, start, size_t, len)
> > +{
> > +     start = untagged_addr(start);
> > +     return ksys_mlock(start, len, VM_LOCKED);
> > +}
> > +
> > +SYSCALL_DEFINE2(arm64_munlock, unsigned long, start, size_t, len)
> > +{
> > +     start = untagged_addr(start);
> > +     return ksys_munlock(start, len);
> > +}
> > +
> > +SYSCALL_DEFINE3(arm64_mprotect, unsigned long, start, size_t, len,
> > +             unsigned long, prot)
> > +{
> > +     start = untagged_addr(start);
> > +     return ksys_mprotect_pkey(start, len, prot, -1);
> > +}
> > +
> > +SYSCALL_DEFINE3(arm64_msync, unsigned long, start, size_t, len, int, flags)
> > +{
> > +     start = untagged_addr(start);
> > +     return ksys_msync(start, len, flags);
> > +}
> > +
> > +SYSCALL_DEFINE3(arm64_mincore, unsigned long, start, size_t, len,
> > +             unsigned char __user *, vec)
> > +{
> > +     start = untagged_addr(start);
> > +     return ksys_mincore(start, len, vec);
> > +}
>
> These look fine.
>
> > +SYSCALL_DEFINE5(arm64_remap_file_pages, unsigned long, start,
> > +             unsigned long, size, unsigned long, prot,
> > +             unsigned long, pgoff, unsigned long, flags)
> > +{
> > +     start = untagged_addr(start);
> > +     return ksys_remap_file_pages(start, size, prot, pgoff, flags);
> > +}
>
> While this has been deprecated for some time, I presume user space still
> invokes it?
>
> > +SYSCALL_DEFINE3(arm64_shmat, int, shmid, char __user *, shmaddr, int, shmflg)
> > +{
> > +     shmaddr = untagged_addr(shmaddr);
> > +     return ksys_shmat(shmid, shmaddr, shmflg);
> > +}
> > +
> > +SYSCALL_DEFINE1(arm64_shmdt, char __user *, shmaddr)
> > +{
> > +     shmaddr = untagged_addr(shmaddr);
> > +     return ksys_shmdt(shmaddr);
> > +}
>
> Do we actually want to allow shared tagged memory? Who's going to tag
> it? If not, we can document it as not supported.
>
> --
> Catalin

  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 [this message]
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
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='CAFKCwrjyP+x0JJy=qpBFsp4pub3He6UkvU0qnf1UOKt6W1LPRQ@mail.gmail.com' \
    --to=eugenis@google.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=andreyknvl@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=dvyukov@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=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-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.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-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org linux-media@archiver.kernel.org
	public-inbox-index linux-media


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


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