Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Thu, Jan 18, 2018 at 08:49:31AM -0800, Linus Torvalds wrote: > On Thu, Jan 18, 2018 at 8:38 AM, Christoph Hellwig wrote: > > > > > But there are about ~100 set_fs() calls in generic code, and some of > > > those really are pretty fundamental. Doing things like

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Thu, Jan 18, 2018 at 08:49:31AM -0800, Linus Torvalds wrote: > On Thu, Jan 18, 2018 at 8:38 AM, Christoph Hellwig wrote: > > > > > But there are about ~100 set_fs() calls in generic code, and some of > > > those really are pretty fundamental. Doing things like "kernel_read()" > > > without

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Thu, Jan 18, 2018 at 8:38 AM, Christoph Hellwig wrote: > > > But there are about ~100 set_fs() calls in generic code, and some of > > those really are pretty fundamental. Doing things like "kernel_read()" > > without set_fs() is basically impossible. > > Not if we move to

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Thu, Jan 18, 2018 at 8:38 AM, Christoph Hellwig wrote: > > > But there are about ~100 set_fs() calls in generic code, and some of > > those really are pretty fundamental. Doing things like "kernel_read()" > > without set_fs() is basically impossible. > > Not if we move to iov_iter or

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 11:26:08AM -0800, Linus Torvalds wrote: > But there are about ~100 set_fs() calls in generic code, and some of > those really are pretty fundamental. Doing things like "kernel_read()" > without set_fs() is basically impossible. Not if we move to iov_iter or iov_iter-like

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 11:26:08AM -0800, Linus Torvalds wrote: > But there are about ~100 set_fs() calls in generic code, and some of > those really are pretty fundamental. Doing things like "kernel_read()" > without set_fs() is basically impossible. Not if we move to iov_iter or iov_iter-like

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 12:05 PM, Al Viro wrote: > On Wed, Jan 17, 2018 at 11:54:12AM -0800, Dan Williams wrote: >> On Wed, Jan 17, 2018 at 10:52 AM, Al Viro wrote: >> > On Wed, Jan 17, 2018 at 02:17:26PM +, Alan Cox wrote: >> [..] >> >

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 12:05 PM, Al Viro wrote: > On Wed, Jan 17, 2018 at 11:54:12AM -0800, Dan Williams wrote: >> On Wed, Jan 17, 2018 at 10:52 AM, Al Viro wrote: >> > On Wed, Jan 17, 2018 at 02:17:26PM +, Alan Cox wrote: >> [..] >> > Incidentally, what about copy_to_iter() and

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 11:54:12AM -0800, Dan Williams wrote: > On Wed, Jan 17, 2018 at 10:52 AM, Al Viro wrote: > > On Wed, Jan 17, 2018 at 02:17:26PM +, Alan Cox wrote: > [..] > > Incidentally, what about copy_to_iter() and friends? They > > check iov_iter

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 11:54:12AM -0800, Dan Williams wrote: > On Wed, Jan 17, 2018 at 10:52 AM, Al Viro wrote: > > On Wed, Jan 17, 2018 at 02:17:26PM +, Alan Cox wrote: > [..] > > Incidentally, what about copy_to_iter() and friends? They > > check iov_iter flavour and go either

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 11:26 AM, Linus Torvalds wrote: > On Wed, Jan 17, 2018 at 6:17 AM, Alan Cox wrote: >> >> Can we kill off the remaining users of set_fs() ? > > I would love to, but it's not going to happen short-term. If ever. > > Some

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 11:26 AM, Linus Torvalds wrote: > On Wed, Jan 17, 2018 at 6:17 AM, Alan Cox wrote: >> >> Can we kill off the remaining users of set_fs() ? > > I would love to, but it's not going to happen short-term. If ever. > > Some could be removed today: the code in

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 10:52 AM, Al Viro wrote: > On Wed, Jan 17, 2018 at 02:17:26PM +, Alan Cox wrote: [..] > Incidentally, what about copy_to_iter() and friends? They > check iov_iter flavour and go either into the "copy to kernel buffer" > or "copy to

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 10:52 AM, Al Viro wrote: > On Wed, Jan 17, 2018 at 02:17:26PM +, Alan Cox wrote: [..] > Incidentally, what about copy_to_iter() and friends? They > check iov_iter flavour and go either into the "copy to kernel buffer" > or "copy to userland" paths. Do we need

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 6:17 AM, Alan Cox wrote: > > Can we kill off the remaining users of set_fs() ? I would love to, but it's not going to happen short-term. If ever. Some could be removed today: the code in arch/x86/net/bpf_jit_comp.c seems to be literally the

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 6:17 AM, Alan Cox wrote: > > Can we kill off the remaining users of set_fs() ? I would love to, but it's not going to happen short-term. If ever. Some could be removed today: the code in arch/x86/net/bpf_jit_comp.c seems to be literally the ramblings of a diseased mind.

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Tue, Jan 16, 2018 at 8:30 PM, Dan Williams wrote: > > I think the access_ok() conversion to return a speculation sanitized > pointer or NULL is the way to go unless I'm missing something simpler. No, that's way too big of a conversion. Just make get_user() and

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Tue, Jan 16, 2018 at 8:30 PM, Dan Williams wrote: > > I think the access_ok() conversion to return a speculation sanitized > pointer or NULL is the way to go unless I'm missing something simpler. No, that's way too big of a conversion. Just make get_user() and friends (that currently use

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 02:17:26PM +, Alan Cox wrote: > On Tue, 2018-01-16 at 14:41 -0800, Linus Torvalds wrote: > > > > > > On Jan 16, 2018 14:23, "Dan Williams" > > wrote: > > > That said, for get_user specifically, can we do something even > > > cheaper. Dave H.

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Wed, Jan 17, 2018 at 02:17:26PM +, Alan Cox wrote: > On Tue, 2018-01-16 at 14:41 -0800, Linus Torvalds wrote: > > > > > > On Jan 16, 2018 14:23, "Dan Williams" > > wrote: > > > That said, for get_user specifically, can we do something even > > > cheaper. Dave H. reminds me that any valid

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Tue, Jan 16, 2018 at 10:50 PM, Dan Williams wrote: > On Tue, Jan 16, 2018 at 10:28 PM, Al Viro wrote: [..] >> Anything that open-codes copy_from_user() that way is *ALREADY* fucked if >> it cares about the overhead - recent x86 boxen will

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Tue, Jan 16, 2018 at 10:50 PM, Dan Williams wrote: > On Tue, Jan 16, 2018 at 10:28 PM, Al Viro wrote: [..] >> Anything that open-codes copy_from_user() that way is *ALREADY* fucked if >> it cares about the overhead - recent x86 boxen will have slowdown from >> hell on stac()/clac() pairs.

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Tue, 2018-01-16 at 14:41 -0800, Linus Torvalds wrote: > > > On Jan 16, 2018 14:23, "Dan Williams" > wrote: > > That said, for get_user specifically, can we do something even > > cheaper. Dave H. reminds me that any valid user pointer that gets > > past > > the

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Tue, 2018-01-16 at 14:41 -0800, Linus Torvalds wrote: > > > On Jan 16, 2018 14:23, "Dan Williams" > wrote: > > That said, for get_user specifically, can we do something even > > cheaper. Dave H. reminds me that any valid user pointer that gets > > past > > the address limit check will have

RE: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

From: Dan Williams > Sent: 17 January 2018 06:50 ... > > Anything that open-codes copy_from_user() that way is *ALREADY* fucked if > > it cares about the overhead - recent x86 boxen will have slowdown from > > hell on stac()/clac() pairs. Anything like that on a hot path is already > > deep in

RE: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

From: Dan Williams > Sent: 17 January 2018 06:50 ... > > Anything that open-codes copy_from_user() that way is *ALREADY* fucked if > > it cares about the overhead - recent x86 boxen will have slowdown from > > hell on stac()/clac() pairs. Anything like that on a hot path is already > > deep in

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Tue, Jan 16, 2018 at 10:28 PM, Al Viro wrote: > On Tue, Jan 16, 2018 at 08:30:17PM -0800, Dan Williams wrote: >> On Tue, Jan 16, 2018 at 2:23 PM, Dan Williams >> wrote: >> > On Sat, Jan 13, 2018 at 11:33 AM, Linus Torvalds >> [..] >> > I'll

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Tue, Jan 16, 2018 at 10:28 PM, Al Viro wrote: > On Tue, Jan 16, 2018 at 08:30:17PM -0800, Dan Williams wrote: >> On Tue, Jan 16, 2018 at 2:23 PM, Dan Williams >> wrote: >> > On Sat, Jan 13, 2018 at 11:33 AM, Linus Torvalds >> [..] >> > I'll respin this set along those lines, and drop the

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Tue, Jan 16, 2018 at 08:30:17PM -0800, Dan Williams wrote: > On Tue, Jan 16, 2018 at 2:23 PM, Dan Williams > wrote: > > On Sat, Jan 13, 2018 at 11:33 AM, Linus Torvalds > [..] > > I'll respin this set along those lines, and drop the ifence bits. > > So now I'm not

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Tue, Jan 16, 2018 at 08:30:17PM -0800, Dan Williams wrote: > On Tue, Jan 16, 2018 at 2:23 PM, Dan Williams > wrote: > > On Sat, Jan 13, 2018 at 11:33 AM, Linus Torvalds > [..] > > I'll respin this set along those lines, and drop the ifence bits. > > So now I'm not so sure. Yes,

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Tue, Jan 16, 2018 at 2:23 PM, Dan Williams wrote: > On Sat, Jan 13, 2018 at 11:33 AM, Linus Torvalds [..] > I'll respin this set along those lines, and drop the ifence bits. So now I'm not so sure. Yes, get_user_{1,2,4,8} can mask the pointer with the address limit

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Tue, Jan 16, 2018 at 2:23 PM, Dan Williams wrote: > On Sat, Jan 13, 2018 at 11:33 AM, Linus Torvalds [..] > I'll respin this set along those lines, and drop the ifence bits. So now I'm not so sure. Yes, get_user_{1,2,4,8} can mask the pointer with the address limit result, but this doesn't

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Sat, Jan 13, 2018 at 11:33 AM, Linus Torvalds wrote: > On Sat, Jan 13, 2018 at 11:05 AM, Linus Torvalds > wrote: >> >> I _know_ that lfence is expensive as hell on P4, for example. >> >> Yes, yes, "sbb" is often more expensive than

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Sat, Jan 13, 2018 at 11:33 AM, Linus Torvalds wrote: > On Sat, Jan 13, 2018 at 11:05 AM, Linus Torvalds > wrote: >> >> I _know_ that lfence is expensive as hell on P4, for example. >> >> Yes, yes, "sbb" is often more expensive than most ALU instructions, >> and Agner Fog says it has a

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

Linus Torvalds writes: > On Sat, Jan 13, 2018 at 11:05 AM, Linus Torvalds > wrote: >> >> I _know_ that lfence is expensive as hell on P4, for example. >> >> Yes, yes, "sbb" is often more expensive than most ALU instructions, >> and

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

Linus Torvalds writes: > On Sat, Jan 13, 2018 at 11:05 AM, Linus Torvalds > wrote: >> >> I _know_ that lfence is expensive as hell on P4, for example. >> >> Yes, yes, "sbb" is often more expensive than most ALU instructions, >> and Agner Fog says it has a 10-cycle latency on Prescott (which is

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Sat, Jan 13, 2018 at 11:05 AM, Linus Torvalds wrote: > > I _know_ that lfence is expensive as hell on P4, for example. > > Yes, yes, "sbb" is often more expensive than most ALU instructions, > and Agner Fog says it has a 10-cycle latency on Prescott (which is >

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Sat, Jan 13, 2018 at 11:05 AM, Linus Torvalds wrote: > > I _know_ that lfence is expensive as hell on P4, for example. > > Yes, yes, "sbb" is often more expensive than most ALU instructions, > and Agner Fog says it has a 10-cycle latency on Prescott (which is > outrageous, but being one or two

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Sat, Jan 13, 2018 at 10:18 AM, Dan Williams wrote: > diff --git a/arch/x86/lib/getuser.S b/arch/x86/lib/getuser.S > index c97d935a29e8..85f400b8ee7c 100644 > --- a/arch/x86/lib/getuser.S > +++ b/arch/x86/lib/getuser.S > @@ -41,6 +41,7 @@ ENTRY(__get_user_1) >

Re: [PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

On Sat, Jan 13, 2018 at 10:18 AM, Dan Williams wrote: > diff --git a/arch/x86/lib/getuser.S b/arch/x86/lib/getuser.S > index c97d935a29e8..85f400b8ee7c 100644 > --- a/arch/x86/lib/getuser.S > +++ b/arch/x86/lib/getuser.S > @@ -41,6 +41,7 @@ ENTRY(__get_user_1) > cmp

[PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

Quoting Linus: I do think that it would be a good idea to very expressly document the fact that it's not that the user access itself is unsafe. I do agree that things like "get_user()" want to be protected, but not because of any direct bugs or problems with get_user() and

[PATCH v3 8/9] x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths

Quoting Linus: I do think that it would be a good idea to very expressly document the fact that it's not that the user access itself is unsafe. I do agree that things like "get_user()" want to be protected, but not because of any direct bugs or problems with get_user() and