All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>,
	Andy Lutomirski <luto@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-api <linux-api@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.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.deaco>
Subject: Re: [RFC PATCH for 4.18 1/2] rseq: validate rseq_cs fields are < TASK_SIZE
Date: Fri, 29 Jun 2018 12:07:12 -0400 (EDT)	[thread overview]
Message-ID: <247789350.9741.1530288432573.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <CA+55aFwJWxsE_LL=-8WgB7uH_mYeMSu=boWF4N5KdOW673g4hA@mail.gmail.com>

----- On Jun 29, 2018, at 11:54 AM, Linus Torvalds torvalds@linux-foundation.org wrote:

> On Fri, Jun 29, 2018 at 8:27 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> You simply can't have it both ways.
> 
> Put another way.
> 
> This is ok in the native path:
> 
>        if ((unsigned long) rseq_cs->abort_ip != rseq_cs->abort_ip)
>                return -EINVAL;
> 
> because it's checking that the value fits in the native register size
> (and it also ends up being a no-op if the native size is the same size
> as abort_ip).
> 
> And this is very much ok in a compat syscall:
> 
>        if (rseq_cs->abort_ip & ~(unsigned long)-1u)
>                return -EINVAL;
> 
> because it's checking that the pointer doesn't have (invalid in
> compat) high bits set.
> 
> But it is NOT OK to say "the rseq system call doesn't have any compat
> syscall, but we'll do that compat check in the native case, because we
> worry about compat issues".
> 
> See what I'm saying? Either you worry about compat issues (and have a
> compat syscall), or you don't.
> 
> The whole "let's not do a compat syscall, but then check compat issues
> at run-time in the native system call because compat processes will
> use it" is braindamage.

This code is not invoked from syscalls, but rather on return from
interrupt/trap after a preemption.

So a compat system call does not solve it. Unless we grab the "compat"
state on rseq registration, save it in a rseq_compat flag within the
task struct, and then use it on return from interrupt/trap/syscall.
Otherwise we need to figure out whether we are dealing with a compat
task when interrupt and trap context return to userspace. We had
is_compat_task() for that before, but now it has vanished from x86.
We could use user_64bit_mode(struct pt_regs *) on x86, but it does not
exist on other architectures.

One possibility is to introduce a new API that calls user_64bit_mode()
on x86, and is_compat_task() on other archs.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

WARNING: multiple messages have this Message-ID (diff)
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>,
	Andy Lutomirski <luto@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-api <linux-api@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.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>
Subject: Re: [RFC PATCH for 4.18 1/2] rseq: validate rseq_cs fields are < TASK_SIZE
Date: Fri, 29 Jun 2018 12:07:12 -0400 (EDT)	[thread overview]
Message-ID: <247789350.9741.1530288432573.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <CA+55aFwJWxsE_LL=-8WgB7uH_mYeMSu=boWF4N5KdOW673g4hA@mail.gmail.com>

----- On Jun 29, 2018, at 11:54 AM, Linus Torvalds torvalds@linux-foundation.org wrote:

> On Fri, Jun 29, 2018 at 8:27 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> You simply can't have it both ways.
> 
> Put another way.
> 
> This is ok in the native path:
> 
>        if ((unsigned long) rseq_cs->abort_ip != rseq_cs->abort_ip)
>                return -EINVAL;
> 
> because it's checking that the value fits in the native register size
> (and it also ends up being a no-op if the native size is the same size
> as abort_ip).
> 
> And this is very much ok in a compat syscall:
> 
>        if (rseq_cs->abort_ip & ~(unsigned long)-1u)
>                return -EINVAL;
> 
> because it's checking that the pointer doesn't have (invalid in
> compat) high bits set.
> 
> But it is NOT OK to say "the rseq system call doesn't have any compat
> syscall, but we'll do that compat check in the native case, because we
> worry about compat issues".
> 
> See what I'm saying? Either you worry about compat issues (and have a
> compat syscall), or you don't.
> 
> The whole "let's not do a compat syscall, but then check compat issues
> at run-time in the native system call because compat processes will
> use it" is braindamage.

This code is not invoked from syscalls, but rather on return from
interrupt/trap after a preemption.

So a compat system call does not solve it. Unless we grab the "compat"
state on rseq registration, save it in a rseq_compat flag within the
task struct, and then use it on return from interrupt/trap/syscall.
Otherwise we need to figure out whether we are dealing with a compat
task when interrupt and trap context return to userspace. We had
is_compat_task() for that before, but now it has vanished from x86.
We could use user_64bit_mode(struct pt_regs *) on x86, but it does not
exist on other architectures.

One possibility is to introduce a new API that calls user_64bit_mode()
on x86, and is_compat_task() on other archs.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

  reply	other threads:[~2018-06-29 16:07 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-28 16:23 [RFC PATCH for 4.18 1/2] rseq: validate rseq_cs fields are < TASK_SIZE Mathieu Desnoyers
2018-06-28 16:23 ` Mathieu Desnoyers
2018-06-28 16:23 ` [RFC PATCH for 4.18 2/2] rseq: check that rseq->rseq_cs padding is zero Mathieu Desnoyers
2018-06-28 16:23   ` Mathieu Desnoyers
2018-06-28 16:53   ` Will Deacon
2018-06-28 16:53     ` Will Deacon
2018-06-28 20:55     ` Mathieu Desnoyers
2018-06-28 20:55       ` Mathieu Desnoyers
2018-06-28 20:22 ` [RFC PATCH for 4.18 1/2] rseq: validate rseq_cs fields are < TASK_SIZE Andy Lutomirski
2018-06-28 20:22   ` Andy Lutomirski
2018-06-28 20:56   ` Mathieu Desnoyers
2018-06-28 20:56     ` Mathieu Desnoyers
2018-06-28 21:22   ` Linus Torvalds
2018-06-28 21:22     ` Linus Torvalds
2018-06-28 22:29     ` Mathieu Desnoyers
2018-06-28 22:29       ` Mathieu Desnoyers
2018-06-28 23:29     ` Andy Lutomirski
2018-06-28 23:29       ` Andy Lutomirski
2018-06-29  0:18       ` Linus Torvalds
2018-06-29  0:18         ` Linus Torvalds
2018-06-29  0:54         ` Mathieu Desnoyers
2018-06-29  0:54           ` Mathieu Desnoyers
2018-06-29  1:08         ` Andy Lutomirski
2018-06-29  1:08           ` Andy Lutomirski
2018-06-29 14:02           ` Linus Torvalds
2018-06-29 14:02             ` Linus Torvalds
2018-06-29 14:05             ` Mathieu Desnoyers
2018-06-29 14:05               ` Mathieu Desnoyers
2018-06-29 14:17               ` Linus Torvalds
2018-06-29 14:17                 ` Linus Torvalds
2018-06-29 15:03                 ` Mathieu Desnoyers
2018-06-29 15:03                   ` Mathieu Desnoyers
     [not found]                   ` <CA+55aFw==YnFJn7iGnKMW=RbPT74YHNa0QDF96mEdMPA2oX9SA@mail.gmail.com>
2018-06-29 15:54                     ` Linus Torvalds
2018-06-29 15:54                       ` Linus Torvalds
2018-06-29 16:07                       ` Mathieu Desnoyers [this message]
2018-06-29 16:07                         ` Mathieu Desnoyers
2018-06-29 17:03                         ` Linus Torvalds
2018-06-29 17:03                           ` Linus Torvalds
2018-06-29 19:48                           ` Mathieu Desnoyers
2018-06-29 19:48                             ` Mathieu Desnoyers
2018-06-29 20:39                             ` Andy Lutomirski
2018-06-29 20:39                               ` Andy Lutomirski
2018-07-02 14:32                               ` Mathieu Desnoyers
2018-07-02 14:32                                 ` Mathieu Desnoyers
2018-07-02 16:04                                 ` Mathieu Desnoyers
2018-07-02 16:04                                   ` Mathieu Desnoyers
2018-07-02 17:11                                 ` Andy Lutomirski
2018-07-02 17:11                                   ` Andy Lutomirski
2018-07-02 19:00                                   ` Mathieu Desnoyers
2018-07-02 19:00                                     ` Mathieu Desnoyers
2018-07-02 19:02                                     ` Andy Lutomirski
2018-07-02 19:02                                       ` Andy Lutomirski
2018-07-02 19:31                                       ` Linus Torvalds
2018-07-02 19:31                                         ` Linus Torvalds
2018-07-02 20:12                                         ` Andy Lutomirski
2018-07-02 20:12                                           ` Andy Lutomirski
2018-07-02 20:22                                           ` Linus Torvalds
2018-07-02 20:22                                             ` Linus Torvalds
2018-06-29 16:07                     ` Andy Lutomirski
2018-06-29 16:07                       ` Andy Lutomirski
2018-06-29 13:55       ` Mathieu Desnoyers
2018-06-29 13:55         ` 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=247789350.9741.1530288432573.JavaMail.zimbra@efficios.com \
    --to=mathieu.desnoyers@efficios.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=luto@kernel.org \
    --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.