Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

On Tue, Jan 9, 2018 at 4:48 PM, Dan Williams wrote: > > I looks like there is another problem, or I'm misreading the > cleverness. I think you were misreading it. I was basically saying that this: unsigned long _mask = ~(long)(_m - 1 - _i) >> BITS_PER_LONG -

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

On Tue, Jan 09, 2018 at 06:22:09PM -0800, Dan Williams wrote: > > When you came up with that tweak you noted: > > "The following: > [..] > is generic and no speculative flows." I meant 'no speculative control flow' load speculation still happens. > > > This macro doesn't prevent speculation.

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

On Tue, Jan 9, 2018 at 5:57 PM, Alexei Starovoitov wrote: > On Tue, Jan 09, 2018 at 04:48:24PM -0800, Dan Williams wrote: >> >> #define __nospec_array_ptr(base, idx, sz) \ >> ({

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

On Tue, Jan 09, 2018 at 04:48:24PM -0800, Dan Williams wrote: > > #define __nospec_array_ptr(base, idx, sz) \ > ({ \ > union { typeof([0]) _ptr; unsigned long _bit; } __u; \ >

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

On Tue, Jan 9, 2018 at 4:48 PM, Dan Williams wrote: > On Mon, Jan 8, 2018 at 8:13 PM, Linus Torvalds > wrote: >> >> On Mon, Jan 8, 2018 at 7:42 PM, Dan Williams >> wrote: >> > >> > originally from Linus and

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

On Tue, Jan 9, 2018 at 4:54 PM, Eric W. Biederman wrote: > Dan Williams writes: [..] > Sigh. > I am not worrying about what is in the speculation window. My > assumption is that the speculation window could be infinite, as we don't > know the

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

Dan Williams writes: > On Tue, Jan 9, 2018 at 8:17 AM, Eric W. Biederman > wrote: >> Dan Williams writes: > [..] >> The user controlled value condition of your patchset implies that you >> assume indirect branch

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

On Mon, Jan 8, 2018 at 8:13 PM, Linus Torvalds wrote: > > On Mon, Jan 8, 2018 at 7:42 PM, Dan Williams wrote: > > > > originally from Linus and tweaked by Alexei and I: > > Sadly, that tweak - while clever - is wrong. > > >

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

On Tue, Jan 9, 2018 at 8:17 AM, Eric W. Biederman wrote: > Dan Williams writes: [..] > The user controlled value condition of your patchset implies that you > assume indirect branch predictor poisoning is handled in other ways. > > Which means

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

Dan Williams writes: > On Mon, Jan 8, 2018 at 7:11 PM, Eric W. Biederman > wrote: >> Dan Williams writes: >> >>> Static analysis reports that 'index' may be a user controlled value that >>> is used as a data dependency

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

On Mon, Jan 8, 2018 at 8:13 PM, Linus Torvalds wrote: > > # carry will be clear if idx >= max > cmpq %idx,%max Bah. Other way around. cmpq %max,%idx I'm a moron. > # mask will be clear if carry was clear, ~0 otherwise > sbbq %mask,%mask >

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

On Mon, Jan 8, 2018 at 7:42 PM, Dan Williams wrote: > > originally from Linus and tweaked by Alexei and I: Sadly, that tweak - while clever - is wrong. > unsigned long _mask = ~(long)(_m - 1 - _i) >> BITS_PER_LONG - 1;\ Why? Because "(long)(_m-1-_i)" is not

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

On Mon, Jan 8, 2018 at 7:11 PM, Eric W. Biederman wrote: > Dan Williams writes: > >> Static analysis reports that 'index' may be a user controlled value that >> is used as a data dependency reading 'rt' from the 'platform_label' >> array. In

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

Dan Williams writes: > Static analysis reports that 'index' may be a user controlled value that > is used as a data dependency reading 'rt' from the 'platform_label' > array. In order to avoid potential leaks of kernel memory values, block > speculative execution of

Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

On 1/6/2018 4:11 AM, Dan Williams wrote: Static analysis reports that 'index' may be a user controlled value that is used as a data dependency reading 'rt' from the 'platform_label' array. In order to avoid potential leaks of kernel memory values, block speculative execution of the instruction

[PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution

Static analysis reports that 'index' may be a user controlled value that is used as a data dependency reading 'rt' from the 'platform_label' array. In order to avoid potential leaks of kernel memory values, block speculative execution of the instruction stream that could issue further reads based