Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: Dave Martin <Dave.Martin@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Kate Stewart <kstewart@linuxfoundation.org>,
	linux-doc@vger.kernel.org, Will Deacon <will.deacon@arm.com>,
	linux-mm@kvack.org, linux-kselftest@vger.kernel.org,
	Chintan Pandya <cpandya@codeaurora.org>,
	Shuah Khan <shuah@kernel.org>, Ingo Molnar <mingo@kernel.org>,
	linux-arch@vger.kernel.org, Jacob Bramley <Jacob.Bramley@arm.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Evgeniy Stepanov <eugenis@google.com>,
	Kees Cook <keescook@chromium.org>,
	Ruben Ayrapetyan <Ruben.Ayrapetyan@arm.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	Lee Smith <Lee.Smith@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	Kostya Serebryany <kcc@google.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
	Ramana Radhakrishnan <Ramana.Radhakrishnan@arm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Subject: Re: [PATCH v9 0/8] arm64: untag user pointers passed to the kernel
Date: Mon, 11 Feb 2019 11:35:12 +0000
Message-ID: <20190211113511.GA165128@arrakis.emea.arm.com> (raw)
In-Reply-To: <20181212170108.GZ3505@e103592.cambridge.arm.com>

Hi Dave,

On Wed, Dec 12, 2018 at 05:01:12PM +0000, Dave P Martin wrote:
> On Mon, Dec 10, 2018 at 01:50:57PM +0100, Andrey Konovalov wrote:
> > arm64 has a feature called Top Byte Ignore, which allows to embed pointer
> > tags into the top byte of each pointer. Userspace programs (such as
> > HWASan, a memory debugging tool [1]) might use this feature and pass
> > tagged user pointers to the kernel through syscalls or other interfaces.
[...]
> It looks like there's been a lot of progress made here towards smoking
> out most of the sites in the kernel where pointers need to be untagged.

In summary, based on last summer's analysis, there are two main (and
rather broad) scenarios of __user pointers use in the kernel: (a)
uaccess macros, together with access_ok() checks and (b) identifying
of user address ranges (find_vma() and related, some ioctls). The
patches here handle the former by allowing sign-extension in access_ok()
and subsequent uaccess routines work fine with tagged pointers.
Identifying the latter is a bit more problematic and the approach we
took was tracking down pointer to long conversion which seems to cover
the majority of cases. However, this approach doesn't scale as, for
example, we'd rather change get_user_pages() to sign-extend the address
rather than all the callers. In lots of other cases we don't even need
untagging as we don't expect user space to tag such pointers (i.e.
mmap() of device memory).

We might be able to improve the static analysis by introducing a
virt_addr_t but that's significant effort and we still won't cover all
cases (e.g. it doesn't necessarily catch tcp_zerocopy_receive() which
wouldn't use a pointer, just a u64 for address).

> However, I do think that we need a clear policy for how existing kernel
> interfaces are to be interpreted in the presence of tagged pointers.
> Unless we have that nailed down, we are likely to be able to make only
> vague guarantees to userspace about what works, and the ongoing risk
> of ABI regressions and inconsistencies seems high.

I agree.

> Can we define an opt-in for tagged-pointer userspace, that rejects all
> syscalls that we haven't checked and whitelisted (or that are
> uncheckable like ioctl)? 

Defining an opt-in is not a problem, however, rejecting all syscalls
that we haven't whitelisted is not feasible. We can have an opt-in per
process (that's what we were going to do with MTE) but the only thing
we can reasonably do is change the behaviour of access_ok(). That's too
big a knob and a new syscall that we haven't got around to whitelist may
just work. This eventually leads to de-facto ABI and our whitelist would
simply be ignored.

I'm not really keen on a big syscall shim in the arm64 kernel which
checks syscall arguments, including in-struct values. If we are to do
this, I'd rather keep it in user space as part of the C library.

> In the meantime, I think we really need to nail down the kernel's
> policies on
> 
>  * in the default configuration (without opt-in), is the presence of
> non-address bits in pointers exchanged with the kernel simply
> considered broken?  (Even with this series, the de factor answer
> generally seems to be "yes", although many specific things will now
> work fine)

Without these patches, passing non-address bits in pointers is
considered broken. I couldn't find a case where it would still work with
non-zero tag but maybe I haven't looked hard enough.

>  * if not, how do we tighten syscall / interface specifications to
> describe what happens with pointers containing non-address bits, while
> keeping the existing behaviour for untagged pointers?
> 
> We would want a general recipe that gives clear guidance on what
> userspace should expect an arbitrarily chosen syscall to do with its
> pointers, without having to enumerate each and every case.

That's what we are aiming with the pointer origins, to move away from a
syscall whitelist to a generic definition. That said, the two approaches
are orthogonal, we can use the pointer origins as the base rule for
which syscalls can be whitelisted.

If we step back a bit to look at the use-case for TBI (and MTE), the
normal application programmer shouldn't really care about this ABI
(well, most of the time). The app gets a tagged pointer from the C
library as a result of a malloc()/realloc() (possibly alloca()) call and
it expects to be able to pass it back into the kernel (usually via the C
library) without any awareness of the non-address bits. Now, we can't
define a user/kernel ABI based on the provenance of the pointer in user
space (i.e. we only support tags for heap and stack), so we are trying
to generalise this based where the pointer originated from in the kernel
(e.g. anonymous mmap()).

> There may already be some background on these topics -- can you throw me
> a link if so?

That's an interesting sub-thread to read:

https://lore.kernel.org/lkml/5d54526e5ff2e5ad63d0dfdd9ab17cf359afa4f2.1535629099.git.andreyknvl@google.com/

-- 
Catalin

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

  parent reply index

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-10 12:50 Andrey Konovalov
2018-12-10 12:50 ` [PATCH v9 1/8] arm64: add type casts to untagged_addr macro Andrey Konovalov
2018-12-10 12:50 ` [PATCH v9 2/8] uaccess: add untagged_addr definition for other arches Andrey Konovalov
2018-12-10 12:51 ` [PATCH v9 3/8] arm64: untag user addresses in access_ok and __uaccess_mask_ptr Andrey Konovalov
2018-12-10 12:51 ` [PATCH v9 4/8] mm, arm64: untag user addresses in mm/gup.c Andrey Konovalov
2018-12-10 12:51 ` [PATCH v9 5/8] lib, arm64: untag addrs passed to strncpy_from_user and strnlen_user Andrey Konovalov
2018-12-10 12:51 ` [PATCH v9 6/8] fs, arm64: untag user address in copy_mount_options Andrey Konovalov
2018-12-10 12:51 ` [PATCH v9 7/8] arm64: update Documentation/arm64/tagged-pointers.txt Andrey Konovalov
2018-12-10 12:51 ` [PATCH v9 8/8] selftests, arm64: add a selftest for passing tagged pointers to kernel Andrey Konovalov
2018-12-10 14:30 ` [RFC][PATCH 0/3] arm64 relaxed ABI Vincenzo Frascino
2018-12-10 14:30   ` [RFC][PATCH 1/3] elf: Make AT_FLAGS arch configurable Vincenzo Frascino
2018-12-10 14:30   ` [RFC][PATCH 2/3] arm64: Define Documentation/arm64/elf_at_flags.txt Vincenzo Frascino
2018-12-12 17:34     ` Dave Martin
2019-01-09 13:05       ` Vincenzo Frascino
2018-12-10 14:30   ` [RFC][PATCH 3/3] arm64: elf: Advertise relaxed ABI Vincenzo Frascino
2018-12-12 14:23   ` [RFC][PATCH 0/3] arm64 " Andrey Konovalov
2018-12-12 15:02     ` Catalin Marinas
2018-12-18 15:03       ` Andrey Konovalov
2018-12-18 17:59         ` Catalin Marinas
2018-12-19 12:52           ` Dave Martin
2019-02-11 17:28             ` Kevin Brodsky
2019-02-11 20:32               ` Evgenii Stepanov
2019-02-12 18:02                 ` Catalin Marinas
2019-02-13 14:58                   ` Dave Martin
2019-02-13 16:42                     ` Kevin Brodsky
2019-02-13 17:43                       ` Dave Martin
2019-02-13 21:41                         ` Evgenii Stepanov
2019-02-14 11:22                           ` Kevin Brodsky
2019-02-19 18:38                   ` Szabolcs Nagy
2019-02-25 16:57                     ` Catalin Marinas
2019-02-25 18:02                       ` Szabolcs Nagy
2019-02-26 17:30                         ` Kevin Brodsky
2018-12-12 17:01 ` [PATCH v9 0/8] arm64: untag user pointers passed to the kernel Dave Martin
2018-12-18 17:17   ` Andrey Konovalov
2019-02-11 11:35   ` Catalin Marinas [this message]
2019-02-11 17:02     ` 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=20190211113511.GA165128@arrakis.emea.arm.com \
    --to=catalin.marinas@arm.com \
    --cc=Dave.Martin@arm.com \
    --cc=Jacob.Bramley@arm.com \
    --cc=Lee.Smith@arm.com \
    --cc=Ramana.Radhakrishnan@arm.com \
    --cc=Ruben.Ayrapetyan@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@google.com \
    --cc=cpandya@codeaurora.org \
    --cc=dvyukov@google.com \
    --cc=eugenis@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kcc@google.com \
    --cc=keescook@chromium.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kstewart@linuxfoundation.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luc.vanoostenryck@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=mingo@kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=shuah@kernel.org \
    --cc=will.deacon@arm.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