Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Dave Martin <Dave.Martin@arm.com>
To: Andrey Konovalov <andreyknvl@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Kate Stewart <kstewart@linuxfoundation.org>,
	linux-doc@vger.kernel.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Kostya Serebryany <kcc@google.com>,
	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>,
	Ramana Radhakrishnan <Ramana.Radhakrishnan@arm.com>,
	linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org,
	Luc Van Oostenryck <luc.vanoostenryck@gmail.com>,
	Lee Smith <Lee.Smith@arm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Robin Murphy <robin.murphy@arm.com>,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH v9 0/8] arm64: untag user pointers passed to the kernel
Date: Wed, 12 Dec 2018 17:01:12 +0000
Message-ID: <20181212170108.GZ3505@e103592.cambridge.arm.com> (raw)
In-Reply-To: <cover.1544445454.git.andreyknvl@google.com>

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.
> 
> Right now the kernel is already able to handle user faults with tagged
> pointers, due to these patches:
> 
> 1. 81cddd65 ("arm64: traps: fix userspace cache maintenance emulation on a
>              tagged pointer")
> 2. 7dcd9dd8 ("arm64: hw_breakpoint: fix watchpoint matching for tagged
>               pointers")
> 3. 276e9327 ("arm64: entry: improve data abort handling of tagged
>               pointers")
> 
> When passing tagged pointers to syscalls, there's a special case of such a
> pointer being passed to one of the memory syscalls (mmap, mprotect, etc.).
> These syscalls don't do memory accesses but rather deal with memory
> ranges, hence an untagged pointer is better suited.
> 
> This patchset extends tagged pointer support to non-memory syscalls. This
> is done by reusing the untagged_addr macro to untag user pointers when the
> kernel performs pointer checking to find out whether the pointer comes
> from userspace (most notably in access_ok). The untagging is done only
> when the pointer is being checked, the tag is preserved as the pointer
> makes its way through the kernel.
> 
> One of the alternative approaches to untagging that was considered is to
> completely strip the pointer tag as the pointer enters the kernel with
> some kind of a syscall wrapper, but that won't work with the countless
> number of different ioctl calls. With this approach we would need a custom
> wrapper for each ioctl variation, which doesn't seem practical.
> 
> The following testing approaches has been taken to find potential issues
> with user pointer untagging:
> 
> 1. Static testing (with sparse [2] and separately with a custom static
>    analyzer based on Clang) to track casts of __user pointers to integer
>    types to find places where untagging needs to be done.
> 
> 2. Dynamic testing: adding BUG_ON(has_tag(addr)) to find_vma() and running
>    a modified syzkaller version that passes tagged pointers to the kernel.
> 
> Based on the results of the testing the requried patches have been added
> to the patchset.
> 
> This patchset has been merged into the Pixel 2 kernel tree and is now
> being used to enable testing of Pixel 2 phones with HWASan.

Do you have an idea of how much of the user/kernel interface is covered
by this workload?

> This patchset is a prerequisite for ARM's memory tagging hardware feature
> support [3].

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.

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 don't really see how we can advertise a full system interface if we
know some subset of it doesn't work for foreseeable userspace
environments.  I feel that presenting the current changes as an ABI
relaxation may be a recipe for future problems, since the forwards
compatibility guarantees we're able to make today are few and rather
vague.

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)?  This reflects the reality that we don't have
a regular userspace environment in which standards-compliant software
that uses address tags in a reasonable way will just work.

It might be feasible to promote this to be enabled by default later on,
if it becomes sufficiently complete.


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)

 * 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.

To be sustainable, we would also need to solve that in a way that
doesn't need to be reintented per-arch.


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

Cheers
---Dave

_______________________________________________
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 ` Dave Martin [this message]
2018-12-18 17:17   ` [PATCH v9 0/8] arm64: untag user pointers passed to the kernel Andrey Konovalov
2019-02-11 11:35   ` Catalin Marinas
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=20181212170108.GZ3505@e103592.cambridge.arm.com \
    --to=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=catalin.marinas@arm.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