From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: Joel Fernandes <joelaf@google.com>,
Peter Zijlstra <peterz@infradead.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Dave Watson <davejwatson@fb.com>,
Will Deacon <will.deacon@arm.com>,
Andi Kleen <andi@firstfloor.org>,
"H. Peter Anvin" <hpa@zytor.com>, Chris Lameter <cl@linux.com>,
Russell King <linux@arm.linux.org.uk>,
Andrew Hunter <ahh@google.com>, Ingo Molnar <mingo@redhat.com>,
Michael Kerrisk <mtk.manpages@gmail.com>,
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
Paul Turner <pjt@google.com>, Boqun Feng <boqun.feng@gmail.com>,
Josh Triplett <josh@joshtriplett.org>,
rostedt <rostedt@goodmis.org>, Ben Maurer <bmaurer@fb.com>,
Thomas Gleixner <tglx@linutronix.de>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
linux-api <linux-api@vger.kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
Andy Lutomirski <luto@amacapital.net>, Andrew Morton <akpm@l>
Subject: Re: [PATCH for 4.18 2/6] rseq: use get_user/put_user rather than __get_user/__put_user
Date: Tue, 10 Jul 2018 09:48:35 -0400 (EDT) [thread overview]
Message-ID: <1050939005.3002.1531230515472.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <87tvp7mx8j.fsf@concordia.ellerman.id.au>
----- On Jul 10, 2018, at 2:16 AM, Michael Ellerman mpe@ellerman.id.au wrote:
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> writes:
>> ----- On Jul 8, 2018, at 5:03 PM, Mathieu Desnoyers
>> mathieu.desnoyers@efficios.com wrote:
>>
>>> In preparation to use __u64 for the rseq_cs pointer field, 32-bit
>>> architectures need to read this 64-bit value located in user-space
>>> addresses.
>>>
>>> __get_user is used to read this value, given that its access check has
>>> already been performed with access_ok() on rseq registration.
>>>
>>> arm does not implement 8-byte __get_user. Rather than trying to
>>> improve __get_user on ARM, use get_user/put_user across rseq instead.
>>>
>>> If those end up showing up in benchmarks, the proper approach would be to
>>> use user_access_begin() / unsafe_get/put_user() / user_access_end()
>>> anyway.
>>
>> So, another twist to this story: ppc32 does not implement u64 get_user().
>
> Or __get_user() for that matter.
>
> But we should just fix it.
>
> We have the asm to do it, it's just the fact that __gu_val is unsigned
> long causes the size > sizeof(x) check here to fail:
>
> #define __get_user_size(x, ptr, size, retval) \
> do { \
> retval = 0; \
> __chk_user_ptr(ptr); \
> if (size > sizeof(x)) \
> (x) = __get_user_bad(); \
>
>
>
> We seem to be able to fix that with the __inttype() trick that x86 uses.
>
> That's probably not 4.18 material though. But if you want to go with
> copy_from_user() for now you could then switch to get_user() for 4.19.
I agree. Let's use copy_from_user() for 4.18. Once get_user() ends up supporting
u64 on ppc32 for 4.19, rseq will happily move back to it.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
WARNING: multiple messages have this Message-ID (diff)
From: mathieu.desnoyers@efficios.com (Mathieu Desnoyers)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH for 4.18 2/6] rseq: use get_user/put_user rather than __get_user/__put_user
Date: Tue, 10 Jul 2018 09:48:35 -0400 (EDT) [thread overview]
Message-ID: <1050939005.3002.1531230515472.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <87tvp7mx8j.fsf@concordia.ellerman.id.au>
----- On Jul 10, 2018, at 2:16 AM, Michael Ellerman mpe at ellerman.id.au wrote:
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> writes:
>> ----- On Jul 8, 2018, at 5:03 PM, Mathieu Desnoyers
>> mathieu.desnoyers at efficios.com wrote:
>>
>>> In preparation to use __u64 for the rseq_cs pointer field, 32-bit
>>> architectures need to read this 64-bit value located in user-space
>>> addresses.
>>>
>>> __get_user is used to read this value, given that its access check has
>>> already been performed with access_ok() on rseq registration.
>>>
>>> arm does not implement 8-byte __get_user. Rather than trying to
>>> improve __get_user on ARM, use get_user/put_user across rseq instead.
>>>
>>> If those end up showing up in benchmarks, the proper approach would be to
>>> use user_access_begin() / unsafe_get/put_user() / user_access_end()
>>> anyway.
>>
>> So, another twist to this story: ppc32 does not implement u64 get_user().
>
> Or __get_user() for that matter.
>
> But we should just fix it.
>
> We have the asm to do it, it's just the fact that __gu_val is unsigned
> long causes the size > sizeof(x) check here to fail:
>
> #define __get_user_size(x, ptr, size, retval) \
> do { \
> retval = 0; \
> __chk_user_ptr(ptr); \
> if (size > sizeof(x)) \
> (x) = __get_user_bad(); \
>
>
>
> We seem to be able to fix that with the __inttype() trick that x86 uses.
>
> That's probably not 4.18 material though. But if you want to go with
> copy_from_user() for now you could then switch to get_user() for 4.19.
I agree. Let's use copy_from_user() for 4.18. Once get_user() ends up supporting
u64 on ppc32 for 4.19, rseq will happily move back to it.
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: Michael Ellerman <mpe@ellerman.id.au>
Cc: 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>,
Andy Lutomirski <luto@amacapital.net>,
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>,
Linus Torvalds <torvalds@linux-foundation.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
Michael Kerrisk <mtk.manpages@gmail.com>,
Joel Fernandes <joelaf@google.com>,
Andrew Hunter <ahh@google.com>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH for 4.18 2/6] rseq: use get_user/put_user rather than __get_user/__put_user
Date: Tue, 10 Jul 2018 09:48:35 -0400 (EDT) [thread overview]
Message-ID: <1050939005.3002.1531230515472.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <87tvp7mx8j.fsf@concordia.ellerman.id.au>
----- On Jul 10, 2018, at 2:16 AM, Michael Ellerman mpe@ellerman.id.au wrote:
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> writes:
>> ----- On Jul 8, 2018, at 5:03 PM, Mathieu Desnoyers
>> mathieu.desnoyers@efficios.com wrote:
>>
>>> In preparation to use __u64 for the rseq_cs pointer field, 32-bit
>>> architectures need to read this 64-bit value located in user-space
>>> addresses.
>>>
>>> __get_user is used to read this value, given that its access check has
>>> already been performed with access_ok() on rseq registration.
>>>
>>> arm does not implement 8-byte __get_user. Rather than trying to
>>> improve __get_user on ARM, use get_user/put_user across rseq instead.
>>>
>>> If those end up showing up in benchmarks, the proper approach would be to
>>> use user_access_begin() / unsafe_get/put_user() / user_access_end()
>>> anyway.
>>
>> So, another twist to this story: ppc32 does not implement u64 get_user().
>
> Or __get_user() for that matter.
>
> But we should just fix it.
>
> We have the asm to do it, it's just the fact that __gu_val is unsigned
> long causes the size > sizeof(x) check here to fail:
>
> #define __get_user_size(x, ptr, size, retval) \
> do { \
> retval = 0; \
> __chk_user_ptr(ptr); \
> if (size > sizeof(x)) \
> (x) = __get_user_bad(); \
>
>
>
> We seem to be able to fix that with the __inttype() trick that x86 uses.
>
> That's probably not 4.18 material though. But if you want to go with
> copy_from_user() for now you could then switch to get_user() for 4.19.
I agree. Let's use copy_from_user() for 4.18. Once get_user() ends up supporting
u64 on ppc32 for 4.19, rseq will happily move back to it.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
next prev parent reply other threads:[~2018-07-10 13:48 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-08 21:03 [PATCH for 4.18 0/6] Restartable Sequences updates Mathieu Desnoyers
2018-07-08 21:03 ` Mathieu Desnoyers
2018-07-08 21:03 ` [PATCH for 4.18 1/6] rseq: use __u64 for rseq_cs fields, validate user inputs Mathieu Desnoyers
2018-07-08 21:03 ` Mathieu Desnoyers
2018-07-08 21:03 ` [PATCH for 4.18 2/6] rseq: use get_user/put_user rather than __get_user/__put_user Mathieu Desnoyers
2018-07-08 21:03 ` Mathieu Desnoyers
2018-07-08 21:03 ` Mathieu Desnoyers
2018-07-09 17:28 ` Mathieu Desnoyers
2018-07-09 17:28 ` Mathieu Desnoyers
2018-07-09 17:28 ` Mathieu Desnoyers
2018-07-09 18:04 ` Linus Torvalds
2018-07-09 18:04 ` Linus Torvalds
2018-07-09 18:04 ` Linus Torvalds
2018-07-09 18:19 ` Mathieu Desnoyers
2018-07-09 18:19 ` Mathieu Desnoyers
2018-07-09 18:19 ` Mathieu Desnoyers
2018-07-09 19:04 ` Linus Torvalds
2018-07-09 19:04 ` Linus Torvalds
2018-07-09 19:04 ` Linus Torvalds
2018-07-10 6:16 ` Michael Ellerman
2018-07-10 6:16 ` Michael Ellerman
2018-07-10 6:16 ` Michael Ellerman
2018-07-10 13:48 ` Mathieu Desnoyers [this message]
2018-07-10 13:48 ` Mathieu Desnoyers
2018-07-10 13:48 ` Mathieu Desnoyers
2018-07-08 21:03 ` [PATCH for 4.18 3/6] rseq: uapi: update uapi comments Mathieu Desnoyers
2018-07-08 21:03 ` Mathieu Desnoyers
2018-07-08 21:03 ` [PATCH for 4.18 4/6] rseq: uapi: declare rseq_cs field as union, update includes Mathieu Desnoyers
2018-07-08 21:03 ` Mathieu Desnoyers
2018-07-08 21:03 ` [PATCH for 4.18 5/6] rseq: remove unused types_32_64.h uapi header Mathieu Desnoyers
2018-07-08 21:03 ` Mathieu Desnoyers
2018-07-08 21:03 ` [PATCH for 4.18 6/6] rseq/selftests: cleanup: update comment above rseq_prepare_unload Mathieu Desnoyers
2018-07-08 21:03 ` Mathieu Desnoyers
2018-07-08 21:12 ` [PATCH for 4.18 0/6] Restartable Sequences updates Linus Torvalds
2018-07-08 21:12 ` Linus Torvalds
2018-07-09 18:04 ` Mathieu Desnoyers
2018-07-09 18:04 ` Mathieu Desnoyers
-- strict thread matches above, loose matches on Subject: below --
2018-07-09 19:51 [PATCH v2 " Mathieu Desnoyers
2018-07-09 19:51 ` [PATCH for 4.18 2/6] rseq: use get_user/put_user rather than __get_user/__put_user Mathieu Desnoyers
2018-07-09 19:51 ` Mathieu Desnoyers
2018-07-09 19:51 ` 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=1050939005.3002.1531230515472.JavaMail.zimbra@efficios.com \
--to=mathieu.desnoyers@efficios.com \
--cc=ahh@google.com \
--cc=akpm@l \
--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=joelaf@google.com \
--cc=josh@joshtriplett.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=luto@amacapital.net \
--cc=mingo@redhat.com \
--cc=mpe@ellerman.id.au \
--cc=mtk.manpages@gmail.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=peterz@infradead.org \
--cc=pjt@google.com \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--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
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.