Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Dave Martin <Dave.Martin@arm.com>
To: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Kate Stewart <kstewart@linuxfoundation.org>,
	"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
	Will Deacon <will.deacon@arm.com>,
	Kostya Serebryany <kcc@google.com>, Lee Smith <Lee.Smith@arm.com>,
	"open list:KERNEL SELFTEST FRAMEWORK"
	<linux-kselftest@vger.kernel.org>,
	Chintan Pandya <cpandya@codeaurora.org>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Shuah Khan <shuah@kernel.org>, Ingo Molnar <mingo@kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	Jacob Bramley <Jacob.Bramley@arm.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Evgenii Stepanov <eugenis@google.com>,
	Kees Cook <keescook@chromium.org>,
	Ruben Ayrapetyan <Ruben.Ayrapetyan@arm.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	Kevin Brodsky <kevin.brodsky@arm.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Linux Memory Management List <linux-mm@kvack.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	LKML <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: [RFC][PATCH 0/3] arm64 relaxed ABI
Date: Wed, 13 Feb 2019 14:58:37 +0000
Message-ID: <20190213145834.GJ3567@e103592.cambridge.arm.com> (raw)
In-Reply-To: <20190212180223.GD199333@arrakis.emea.arm.com>

On Tue, Feb 12, 2019 at 06:02:24PM +0000, Catalin Marinas wrote:
> On Mon, Feb 11, 2019 at 12:32:55PM -0800, Evgenii Stepanov wrote:
> > On Mon, Feb 11, 2019 at 9:28 AM Kevin Brodsky <kevin.brodsky@arm.com> wrote:
> > > On 19/12/2018 12:52, Dave Martin wrote:

[...]

> > > >   * A single C object should be accessed using a single, fixed
> > > >     pointer tag throughout its entire lifetime.
> > >
> > > Agreed.  Allocators themselves may need to be excluded though,
> > > depending on how they represent their managed memory.
> > >
> > > >   * Tags can be changed only when there are no outstanding pointers to
> > > >     the affected object or region that may be used to access the object
> > > >     or region (i.e., if the object were allocated from the C heap and
> > > >     is it safe to realloc() it, then it is safe to change the tag; for
> > > >     other types of allocation, analogous arguments can be applied).
> > >
> > > Tags can only be changed at the point of deallocation/
> > > reallocation.  Pointers to the object become invalid and cannot
> > > be used after it has been deallocated; memory tagging allows to
> > > catch such invalid usage.
>
> All the above sound well but that's mostly a guideline on what a C
> library can do. It doesn't help much with defining the kernel ABI.
> Anyway, it's good to clarify the use-cases.

My aim was to clarify the use case in userspace, since I wasn't directly
involved in that.  The kernel ABI needs to be compatible with the the
use case, but doesn't need to specify must of it.

I'm wondering whether we can piggy-back on existing concepts.

We could say that recolouring memory is safe when and only when
unmapping of the page or removing permissions on the page (via
munmap/mremap/mprotect) would be safe.  Otherwise, the resulting
behaviour of the process is undefined.

Hopefully there are friendly fuzzers testing this kind of thing.

[...]

> > It would also be valuable to narrow down the set of "relaxed" (i.e.
> > not tag-checking) syscalls as reasonably possible. We would want to
> > provide tag-checking userspace wrappers for any important calls that
> > are not checked in the kernel. Is it correct to assume that anything
> > that goes through copy_from_user  / copy_to_user is checked?
> 
> I lost track of the context of this thread but if it's just about
> relaxing the ABI for hwasan, the kernel has no idea of the compiler
> generated structures in user space, so nothing is checked.
> 
> If we talk about tags in the context of MTE, than yes, with the current
> proposal the tag would be checked by copy_*_user() functions.

Also put_user() and friends?

It might be reasonable to do the check in access_ok() and skip it in
__put_user() etc.

(I seem to remember some separate discussion about abolishing
__put_user() and friends though, due to the accident risk they pose.)

> > For aio* operations it would be nice if the tag was checked at the
> > time of the actual userspace read/write, either instead of or in
> > addition to at the time of the system call.
> 
> With aio* (and synchronous iovec-based syscalls), the kernel may access
> the memory while the corresponding user process is scheduled out. Given
> that such access is not done in the context of the user process (and
> using the user VA like copy_*_user), the kernel cannot handle potential
> tag faults. Moreover, the transfer may be done by DMA and the device
> does not understand tags.
> 
> I'd like to keep tags as a property of the pointer in a specific virtual
> address space. The moment you convert it to a different address space
> (e.g. kernel linear map, physical address), the tag property is stripped
> and I don't think we should re-build it (and have it checked).

This is probably reasonable.

Ideally we would check the tag at the point of stripping it off, but
most likely it's going to be rather best-effort.

If memory tagging is essential a debugging feature then this seems
an acceptable compromise.

> > > >   * For purposes other than dereference, the kernel shall accept any
> > > >     legitimately tagged pointer (according to the above rules) as
> > > >     identifying the associated memory location.
> > > >
> > > >     So, mprotect(some_page_aligned_object, ...); is valid irrespective
> > > >     of where page_aligned_object() came from.  There is no implicit
> > > >     derefence by the kernel here, hence no tag check.
> > > >
> > > >     The kernel does not guarantee to work correctly if the wrong tag
> > > >     is used, but there is not always a well-defined "right" tag, so
> > > >     we can't really guarantee to check it.  So a pointer derived by
> > > >     any reasonable means by userspace has to be treated as equally
> > > >     valid.
> > >
> > > This is a disputed point :) In my opinion, this is the the most
> > > reasonable approach.
> > 
> > Yes, it would be nice if the kernel explicitly promised, ex.
> > mprotect() over a range of differently tagged pages to be allowed
> > (i.e. address tag should be unchecked).
> 
> I don't think mprotect() over differently tagged pages was ever a
> problem. I originally asked that mprotect() and friends do not accept
> tagged pointers since these functions deal with memory ranges rather
> than dereferencing such pointer (the reason being minimal kernel
> changes). However, given how complicated it is to specify an ABI, I came
> to the conclusion that a pointer passed to such function should be
> allowed to have non-zero top byte. It would be the kernel's
> responsibility to strip it out as appropriate.

I think that if the page range is all the same colour then it should be
legitimate to pass a matching tag.

But it doesn't seem reasonable for the kernel to require this.  If
free() calls munmap(), the page(s) will contain possibly randomly-
coloured garbage.  There's no correct tag to pass in such a case.

The most obvious solution is just to ignore the tags passed by userspace
to such syscalls.  This would imply that the kernel must explicitly
strip it out, as you suggest.

The number of affected syscalls is relatively small though.

Cheers
---Dave

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

  reply index

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-10 12:50 [PATCH v9 0/8] arm64: untag user pointers passed to the kernel 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 [this message]
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
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=20190213145834.GJ3567@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=kevin.brodsky@arm.com \
    --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=vincenzo.frascino@arm.com \
    --cc=viro@zeniv.linux.org.uk \
    --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
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

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