All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heiko Carstens <heiko.carstens@de.ibm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-api <linux-api@vger.kernel.org>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	Dave Watson <davejwatson@fb.com>, Paul Turner <pjt@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Russell King <linux@arm.linux.org.uk>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Andi Kleen <andi@firstfloor.org>, Chris Lameter <cl@linux.com>,
	Ben Maurer <bmaurer@fb.com>, rostedt <rostedt@goodmis.org>,
	Josh Triplett <josh@joshtriplett.org>,
	Catalin Marinas <catalin.marinas@arm.com>
Subject: Re: [RFC PATCH for 4.18] rseq: use __u64 for rseq_cs fields, validate user inputs
Date: Tue, 3 Jul 2018 10:29:55 +0200	[thread overview]
Message-ID: <20180703082955.GH3704@osiris> (raw)
In-Reply-To: <20180703081449.GT2494@hirez.programming.kicks-ass.net>

On Tue, Jul 03, 2018 at 10:14:49AM +0200, Peter Zijlstra wrote:
> On Mon, Jul 02, 2018 at 10:30:09PM -0400, Mathieu Desnoyers wrote:
> > > Use "get_user()". It works for 64-bit objects too, and it will be
> > > atomic in the 32-bit sub-parts on a 32-bit architecture.
> > 
> > Is it really ? Last time we had this discussion, not all architectures
> > guaranteed that reading a 64-bit integer would happen in two atomic
> > 32-bit sub-parts. This was the main motivation for the LINUX_FIELD_u32_u64()
> > macro as it stands today (rather than using a union).
> 
> Just state, as a requirement for supporting rseq, that the arch
> {get,put}_user(u64) on 32bit targets must be exactly 2 u32 loads/stores.
> 
> We're piece-wise enabling rseq across architectures anyway, and when the
> relevant maintains do this, they can have a look at their
> {get,put}_user() implementations and fix them.
> 
> If you rely on get_user(u64) working, that means microblaze is already
> broken, but I suppose it already was, since their rseq enablement patch
> is extremely dodgy. Michal?

s390 uses the mvcos instruction to implement get_user(). That instruction
is not defined to be atomic, but may copy bytes piecemeal.. I had the
impression that the rseq fields are supposed to be updated within the
context of a single thread (user + kernel space).

However if another user space thread is allowed to do this as well, then
the get_user() approach won't fly on s390.

That leaves the question: does it even make sense for a thread to update
the rseq structure of a different thread?

WARNING: multiple messages have this Message-ID (diff)
From: Heiko Carstens <heiko.carstens@de.ibm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-api <linux-api@vger.kernel.org>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	Dave Watson <davejwatson@fb.com>, Paul Turner <pjt@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Russell King <linux@arm.linux.org.uk>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Andi Kleen <andi@firstfloor.org>, Chris Lameter <cl@linux.com>,
	Ben Maurer <bmaurer@fb.com>, rostedt <rostedt@goodmis.org>,
	Josh Triplett <josh@joshtriplett.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Michael Kerrisk <mtk.manpages@gmail.com>,
	Joel Fernandes <joelaf@google.com>,
	michal.simek@xilinx.com,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>
Subject: Re: [RFC PATCH for 4.18] rseq: use __u64 for rseq_cs fields, validate user inputs
Date: Tue, 3 Jul 2018 10:29:55 +0200	[thread overview]
Message-ID: <20180703082955.GH3704@osiris> (raw)
In-Reply-To: <20180703081449.GT2494@hirez.programming.kicks-ass.net>

On Tue, Jul 03, 2018 at 10:14:49AM +0200, Peter Zijlstra wrote:
> On Mon, Jul 02, 2018 at 10:30:09PM -0400, Mathieu Desnoyers wrote:
> > > Use "get_user()". It works for 64-bit objects too, and it will be
> > > atomic in the 32-bit sub-parts on a 32-bit architecture.
> > 
> > Is it really ? Last time we had this discussion, not all architectures
> > guaranteed that reading a 64-bit integer would happen in two atomic
> > 32-bit sub-parts. This was the main motivation for the LINUX_FIELD_u32_u64()
> > macro as it stands today (rather than using a union).
> 
> Just state, as a requirement for supporting rseq, that the arch
> {get,put}_user(u64) on 32bit targets must be exactly 2 u32 loads/stores.
> 
> We're piece-wise enabling rseq across architectures anyway, and when the
> relevant maintains do this, they can have a look at their
> {get,put}_user() implementations and fix them.
> 
> If you rely on get_user(u64) working, that means microblaze is already
> broken, but I suppose it already was, since their rseq enablement patch
> is extremely dodgy. Michal?

s390 uses the mvcos instruction to implement get_user(). That instruction
is not defined to be atomic, but may copy bytes piecemeal.. I had the
impression that the rseq fields are supposed to be updated within the
context of a single thread (user + kernel space).

However if another user space thread is allowed to do this as well, then
the get_user() approach won't fly on s390.

That leaves the question: does it even make sense for a thread to update
the rseq structure of a different thread?


  reply	other threads:[~2018-07-03  8:29 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-02 22:31 [RFC PATCH for 4.18] rseq: use __u64 for rseq_cs fields, validate user inputs Mathieu Desnoyers
2018-07-02 22:31 ` Mathieu Desnoyers
2018-07-02 22:45 ` Linus Torvalds
2018-07-02 22:45   ` Linus Torvalds
2018-07-02 23:00   ` Mathieu Desnoyers
2018-07-02 23:00     ` Mathieu Desnoyers
2018-07-02 23:06     ` Linus Torvalds
2018-07-02 23:06       ` Linus Torvalds
2018-07-02 23:16       ` Mathieu Desnoyers
2018-07-02 23:16         ` Mathieu Desnoyers
2018-07-02 23:22         ` Linus Torvalds
2018-07-02 23:22           ` Linus Torvalds
2018-07-02 23:25           ` Mathieu Desnoyers
2018-07-02 23:25             ` Mathieu Desnoyers
2018-07-02 23:22         ` Mathieu Desnoyers
2018-07-02 23:22           ` Mathieu Desnoyers
2018-07-02 23:37           ` Andy Lutomirski
2018-07-02 23:37             ` Andy Lutomirski
2018-07-03  1:19             ` Mathieu Desnoyers
2018-07-03  1:19               ` Mathieu Desnoyers
2018-07-03  2:01               ` Mathieu Desnoyers
2018-07-03  2:01                 ` Mathieu Desnoyers
2018-07-03  2:18                 ` Linus Torvalds
2018-07-03  2:18                   ` Linus Torvalds
2018-07-03  2:30                   ` Mathieu Desnoyers
2018-07-03  2:30                     ` Mathieu Desnoyers
2018-07-03  2:33                     ` Andy Lutomirski
2018-07-03  2:33                       ` Andy Lutomirski
2018-07-03  2:44                     ` Linus Torvalds
2018-07-03  2:44                       ` Linus Torvalds
2018-07-03  8:14                     ` Peter Zijlstra
2018-07-03  8:14                       ` Peter Zijlstra
2018-07-03  8:29                       ` Heiko Carstens [this message]
2018-07-03  8:29                         ` Heiko Carstens
2018-07-03  8:43                         ` Peter Zijlstra
2018-07-03  8:43                           ` Peter Zijlstra
2018-07-03  8:55                           ` Heiko Carstens
2018-07-03  8:55                             ` Heiko Carstens
2018-07-03  9:17                             ` Heiko Carstens
2018-07-03  9:17                               ` Heiko Carstens
2018-07-03  9:24                               ` Peter Zijlstra
2018-07-03  9:24                                 ` Peter Zijlstra
2018-07-03  9:21                             ` Peter Zijlstra
2018-07-03  9:21                               ` Peter Zijlstra
2018-07-03 16:40                               ` Andi Kleen
2018-07-03 16:40                                 ` Andi Kleen
2018-07-03 17:02                                 ` Peter Zijlstra
2018-07-03 17:02                                   ` Peter Zijlstra
2018-07-03 17:06                                 ` Andy Lutomirski
2018-07-03 17:06                                   ` Andy Lutomirski
2018-07-03 17:10                                 ` Linus Torvalds
2018-07-03 17:10                                   ` Linus Torvalds
2018-07-03 17:26                                   ` Mathieu Desnoyers
2018-07-03 17:26                                     ` Mathieu Desnoyers
2018-07-03 17:34                                   ` Peter Zijlstra
2018-07-03 17:34                                     ` Peter Zijlstra
2018-07-03 17:38                                     ` Mathieu Desnoyers
2018-07-03 17:38                                       ` Mathieu Desnoyers
2018-07-03 17:48                                       ` Peter Zijlstra
2018-07-03 17:48                                         ` Peter Zijlstra
2018-07-03 17:58                                         ` Mathieu Desnoyers
2018-07-03 17:58                                           ` Mathieu Desnoyers
2018-07-03 18:11                                           ` Peter Zijlstra
2018-07-03 18:11                                             ` Peter Zijlstra
2018-07-03 18:15                                             ` Mathieu Desnoyers
2018-07-03 18:15                                               ` Mathieu Desnoyers
2018-07-03 18:28                                               ` Peter Zijlstra
2018-07-03 18:28                                                 ` Peter Zijlstra
2018-07-03 18:41                                                 ` Mathieu Desnoyers
2018-07-03 18:41                                                   ` Mathieu Desnoyers
2018-07-03 19:08                                                   ` Peter Zijlstra
2018-07-03 19:08                                                     ` Peter Zijlstra
2018-07-03 17:59                                         ` Linus Torvalds
2018-07-03 17:59                                           ` Linus Torvalds
2018-07-03 18:09                                           ` Mathieu Desnoyers
2018-07-03 18:09                                             ` Mathieu Desnoyers
2018-07-03 18:10                                           ` Peter Zijlstra
2018-07-03 18:10                                             ` Peter Zijlstra
2018-07-03  0:19         ` Christopher Lameter
2018-07-03  0:19           ` Christopher Lameter
2018-07-03  0:23           ` Mathieu Desnoyers
2018-07-03  0:23             ` Mathieu Desnoyers
2018-07-03  0:35             ` Christopher Lameter
2018-07-03  0:35               ` Christopher Lameter
2018-07-03  1:17               ` Mathieu Desnoyers
2018-07-03  1:17                 ` Mathieu Desnoyers

Reply instructions:

You may reply publicly 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=20180703082955.GH3704@osiris \
    --to=heiko.carstens@de.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=bmaurer@fb.com \
    --cc=boqun.feng@gmail.com \
    --cc=catalin.marinas@arm.com \
    --cc=cl@linux.com \
    --cc=davejwatson@fb.com \
    --cc=hpa@zytor.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=luto@amacapital.net \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=pjt@google.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.