* [PATCH] kernel: sys: fix potential Spectre v1 @ 2018-05-15 3:00 Gustavo A. R. Silva 2018-05-15 22:08 ` Andrew Morton 0 siblings, 1 reply; 26+ messages in thread From: Gustavo A. R. Silva @ 2018-05-15 3:00 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, Gustavo A. R. Silva resource can be controlled by user-space, hence leading to a potential exploitation of the Spectre variant 1 vulnerability. This issue was detected with the help of Smatch: kernel/sys.c:1474 __do_compat_sys_old_getrlimit() warn: potential spectre issue 'get_current()->signal->rlim' (local cap) kernel/sys.c:1455 __do_sys_old_getrlimit() warn: potential spectre issue 'get_current()->signal->rlim' (local cap) Fix this by sanitizing *resource* before using it to index current->signal->rlim Notice that given that speculation windows are large, the policy is to kill the speculation on the first load and not worry if it can be completed with a dependent load/store [1]. [1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2 Cc: stable@vger.kernel.org Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> --- kernel/sys.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/kernel/sys.c b/kernel/sys.c index 63ef036..78646e6 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -69,6 +69,9 @@ #include <asm/io.h> #include <asm/unistd.h> +/* Hardening for Spectre-v1 */ +#include <linux/nospec.h> + #include "uid16.h" #ifndef SET_UNALIGN_CTL @@ -1451,6 +1454,7 @@ SYSCALL_DEFINE2(old_getrlimit, unsigned int, resource, if (resource >= RLIM_NLIMITS) return -EINVAL; + resource = array_index_nospec(resource, RLIM_NLIMITS); task_lock(current->group_leader); x = current->signal->rlim[resource]; task_unlock(current->group_leader); @@ -1470,6 +1474,7 @@ COMPAT_SYSCALL_DEFINE2(old_getrlimit, unsigned int, resource, if (resource >= RLIM_NLIMITS) return -EINVAL; + resource = array_index_nospec(resource, RLIM_NLIMITS); task_lock(current->group_leader); r = current->signal->rlim[resource]; task_unlock(current->group_leader); -- 2.7.4 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-15 3:00 [PATCH] kernel: sys: fix potential Spectre v1 Gustavo A. R. Silva @ 2018-05-15 22:08 ` Andrew Morton 2018-05-15 22:29 ` Thomas Gleixner 0 siblings, 1 reply; 26+ messages in thread From: Andrew Morton @ 2018-05-15 22:08 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: linux-kernel, Alexei Starovoitov, Dan Williams, Thomas Gleixner, Peter Zijlstra On Mon, 14 May 2018 22:00:38 -0500 "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote: > resource can be controlled by user-space, hence leading to a > potential exploitation of the Spectre variant 1 vulnerability. > > This issue was detected with the help of Smatch: > > kernel/sys.c:1474 __do_compat_sys_old_getrlimit() warn: potential > spectre issue 'get_current()->signal->rlim' (local cap) > kernel/sys.c:1455 __do_sys_old_getrlimit() warn: potential spectre issue > 'get_current()->signal->rlim' (local cap) > > Fix this by sanitizing *resource* before using it to index > current->signal->rlim > > Notice that given that speculation windows are large, the policy is > to kill the speculation on the first load and not worry if it can be > completed with a dependent load/store [1]. hm. Not my area, but I'm always willing to learn ;) > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -69,6 +69,9 @@ > #include <asm/io.h> > #include <asm/unistd.h> > > +/* Hardening for Spectre-v1 */ > +#include <linux/nospec.h> > + > #include "uid16.h" > > #ifndef SET_UNALIGN_CTL > @@ -1451,6 +1454,7 @@ SYSCALL_DEFINE2(old_getrlimit, unsigned int, resource, > if (resource >= RLIM_NLIMITS) > return -EINVAL; > > + resource = array_index_nospec(resource, RLIM_NLIMITS); > task_lock(current->group_leader); > x = current->signal->rlim[resource]; Can the speculation proceed past the task_lock()? Or is the policy to ignore such happy happenstances even if they are available? > task_unlock(current->group_leader); > @@ -1470,6 +1474,7 @@ COMPAT_SYSCALL_DEFINE2(old_getrlimit, unsigned int, resource, > if (resource >= RLIM_NLIMITS) > return -EINVAL; > > + resource = array_index_nospec(resource, RLIM_NLIMITS); > task_lock(current->group_leader); > r = current->signal->rlim[resource]; > task_unlock(current->group_leader); ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-15 22:08 ` Andrew Morton @ 2018-05-15 22:29 ` Thomas Gleixner 2018-05-15 22:57 ` Dan Williams 0 siblings, 1 reply; 26+ messages in thread From: Thomas Gleixner @ 2018-05-15 22:29 UTC (permalink / raw) To: Andrew Morton Cc: Gustavo A. R. Silva, linux-kernel, Alexei Starovoitov, Dan Williams, Peter Zijlstra On Tue, 15 May 2018, Andrew Morton wrote: > On Mon, 14 May 2018 22:00:38 -0500 "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote: > > > resource can be controlled by user-space, hence leading to a > > potential exploitation of the Spectre variant 1 vulnerability. > > > > This issue was detected with the help of Smatch: > > > > kernel/sys.c:1474 __do_compat_sys_old_getrlimit() warn: potential > > spectre issue 'get_current()->signal->rlim' (local cap) > > kernel/sys.c:1455 __do_sys_old_getrlimit() warn: potential spectre issue > > 'get_current()->signal->rlim' (local cap) > > > > Fix this by sanitizing *resource* before using it to index > > current->signal->rlim > > > > Notice that given that speculation windows are large, the policy is > > to kill the speculation on the first load and not worry if it can be > > completed with a dependent load/store [1]. > > hm. Not my area, but I'm always willing to learn ;) > > > --- a/kernel/sys.c > > +++ b/kernel/sys.c > > @@ -69,6 +69,9 @@ > > #include <asm/io.h> > > #include <asm/unistd.h> > > > > +/* Hardening for Spectre-v1 */ > > +#include <linux/nospec.h> > > + > > #include "uid16.h" > > > > #ifndef SET_UNALIGN_CTL > > @@ -1451,6 +1454,7 @@ SYSCALL_DEFINE2(old_getrlimit, unsigned int, resource, > > if (resource >= RLIM_NLIMITS) > > return -EINVAL; > > > > + resource = array_index_nospec(resource, RLIM_NLIMITS); > > task_lock(current->group_leader); > > x = current->signal->rlim[resource]; > > Can the speculation proceed past the task_lock()? Or is the policy to > ignore such happy happenstances even if they are available? Locks are not in the way of speculation. Speculation has almost no limits except serializing instructions. At least they respect the magic AND limitation in array_index_nospec(). Thanks, tglx ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-15 22:29 ` Thomas Gleixner @ 2018-05-15 22:57 ` Dan Williams 2018-05-18 19:04 ` Gustavo A. R. Silva 0 siblings, 1 reply; 26+ messages in thread From: Dan Williams @ 2018-05-15 22:57 UTC (permalink / raw) To: Thomas Gleixner Cc: Andrew Morton, Gustavo A. R. Silva, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On Tue, May 15, 2018 at 3:29 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Tue, 15 May 2018, Andrew Morton wrote: >> On Mon, 14 May 2018 22:00:38 -0500 "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote: >> >> > resource can be controlled by user-space, hence leading to a >> > potential exploitation of the Spectre variant 1 vulnerability. >> > >> > This issue was detected with the help of Smatch: >> > >> > kernel/sys.c:1474 __do_compat_sys_old_getrlimit() warn: potential >> > spectre issue 'get_current()->signal->rlim' (local cap) >> > kernel/sys.c:1455 __do_sys_old_getrlimit() warn: potential spectre issue >> > 'get_current()->signal->rlim' (local cap) >> > >> > Fix this by sanitizing *resource* before using it to index >> > current->signal->rlim >> > >> > Notice that given that speculation windows are large, the policy is >> > to kill the speculation on the first load and not worry if it can be >> > completed with a dependent load/store [1]. >> >> hm. Not my area, but I'm always willing to learn ;) >> >> > --- a/kernel/sys.c >> > +++ b/kernel/sys.c >> > @@ -69,6 +69,9 @@ >> > #include <asm/io.h> >> > #include <asm/unistd.h> >> > >> > +/* Hardening for Spectre-v1 */ >> > +#include <linux/nospec.h> >> > + >> > #include "uid16.h" >> > >> > #ifndef SET_UNALIGN_CTL >> > @@ -1451,6 +1454,7 @@ SYSCALL_DEFINE2(old_getrlimit, unsigned int, resource, >> > if (resource >= RLIM_NLIMITS) >> > return -EINVAL; >> > >> > + resource = array_index_nospec(resource, RLIM_NLIMITS); >> > task_lock(current->group_leader); >> > x = current->signal->rlim[resource]; >> >> Can the speculation proceed past the task_lock()? Or is the policy to >> ignore such happy happenstances even if they are available? > > Locks are not in the way of speculation. Speculation has almost no limits > except serializing instructions. At least they respect the magic AND > limitation in array_index_nospec(). I'd say it another way, because they don't respect the magic AND, we just make the result in the speculation path safe. So, it's controlled speculation. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-15 22:57 ` Dan Williams @ 2018-05-18 19:04 ` Gustavo A. R. Silva 2018-05-18 19:21 ` Gustavo A. R. Silva 0 siblings, 1 reply; 26+ messages in thread From: Gustavo A. R. Silva @ 2018-05-18 19:04 UTC (permalink / raw) To: Dan Williams, Thomas Gleixner Cc: Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On 05/15/2018 05:57 PM, Dan Williams wrote: > On Tue, May 15, 2018 at 3:29 PM, Thomas Gleixner <tglx@linutronix.de> wrote: >> On Tue, 15 May 2018, Andrew Morton wrote: >>> On Mon, 14 May 2018 22:00:38 -0500 "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote: >>> >>>> resource can be controlled by user-space, hence leading to a >>>> potential exploitation of the Spectre variant 1 vulnerability. >>>> >>>> This issue was detected with the help of Smatch: >>>> >>>> kernel/sys.c:1474 __do_compat_sys_old_getrlimit() warn: potential >>>> spectre issue 'get_current()->signal->rlim' (local cap) >>>> kernel/sys.c:1455 __do_sys_old_getrlimit() warn: potential spectre issue >>>> 'get_current()->signal->rlim' (local cap) >>>> >>>> Fix this by sanitizing *resource* before using it to index >>>> current->signal->rlim >>>> >>>> Notice that given that speculation windows are large, the policy is >>>> to kill the speculation on the first load and not worry if it can be >>>> completed with a dependent load/store [1]. >>> >>> hm. Not my area, but I'm always willing to learn ;) >>> >>>> --- a/kernel/sys.c >>>> +++ b/kernel/sys.c >>>> @@ -69,6 +69,9 @@ >>>> #include <asm/io.h> >>>> #include <asm/unistd.h> >>>> >>>> +/* Hardening for Spectre-v1 */ >>>> +#include <linux/nospec.h> >>>> + >>>> #include "uid16.h" >>>> >>>> #ifndef SET_UNALIGN_CTL >>>> @@ -1451,6 +1454,7 @@ SYSCALL_DEFINE2(old_getrlimit, unsigned int, resource, >>>> if (resource >= RLIM_NLIMITS) >>>> return -EINVAL; >>>> >>>> + resource = array_index_nospec(resource, RLIM_NLIMITS); >>>> task_lock(current->group_leader); >>>> x = current->signal->rlim[resource]; >>> >>> Can the speculation proceed past the task_lock()? Or is the policy to >>> ignore such happy happenstances even if they are available? >> >> Locks are not in the way of speculation. Speculation has almost no limits >> except serializing instructions. At least they respect the magic AND >> limitation in array_index_nospec(). > > I'd say it another way, because they don't respect the magic AND, we > just make the result in the speculation path safe. So, it's controlled > speculation. > Dan, What do you think about adding the following function to the nospec API: diff --git a/include/linux/nospec.h b/include/linux/nospec.h index e791ebc..81e9a77 100644 --- a/include/linux/nospec.h +++ b/include/linux/nospec.h @@ -55,4 +55,17 @@ static inline unsigned long array_index_mask_nospec(unsigned long index, \ (typeof(_i)) (_i & _mask); \ }) + + +#ifndef sanitize_index_nospec +inline bool sanitize_index_nospec(unsigned long index, + unsigned long size) +{ + if (index >= size) + return false; + index = array_index_nospec(index, size); + + return true; +} +#endif #endif /* _LINUX_NOSPEC_H */ And here is an example of its use: diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c index 4599b7e..27b39c0 100644 --- a/drivers/media/i2c/tvp7002.c +++ b/drivers/media/i2c/tvp7002.c @@ -35,6 +35,8 @@ #include <media/v4l2-ctrls.h> #include <media/v4l2-fwnode.h> +#include <linux/nospec.h> + #include "tvp7002_reg.h" MODULE_DESCRIPTION("TI TVP7002 Video and Graphics Digitizer driver"); @@ -784,7 +786,7 @@ static int tvp7002_enum_dv_timings(struct v4l2_subdev *sd, return -EINVAL; /* Check requested format index is within range */ - if (timings->index >= NUM_TIMINGS) + if (!sanitize_index_nospec(timings->index, NUM_TIMINGS)) return -EINVAL; timings->timings = tvp7002_timings[timings->index].timings; This patter is very common. So, it may be a good idea to unify both bounds checking and the serialization of instructions into a single function. What do you think? Thanks -- Gustavo ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-18 19:04 ` Gustavo A. R. Silva @ 2018-05-18 19:21 ` Gustavo A. R. Silva 2018-05-18 20:38 ` Dan Williams 0 siblings, 1 reply; 26+ messages in thread From: Gustavo A. R. Silva @ 2018-05-18 19:21 UTC (permalink / raw) To: Dan Williams, Thomas Gleixner Cc: Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On 05/18/2018 02:04 PM, Gustavo A. R. Silva wrote: > > > On 05/15/2018 05:57 PM, Dan Williams wrote: >> On Tue, May 15, 2018 at 3:29 PM, Thomas Gleixner <tglx@linutronix.de> >> wrote: >>> On Tue, 15 May 2018, Andrew Morton wrote: >>>> On Mon, 14 May 2018 22:00:38 -0500 "Gustavo A. R. Silva" >>>> <gustavo@embeddedor.com> wrote: >>>> >>>>> resource can be controlled by user-space, hence leading to a >>>>> potential exploitation of the Spectre variant 1 vulnerability. >>>>> >>>>> This issue was detected with the help of Smatch: >>>>> >>>>> kernel/sys.c:1474 __do_compat_sys_old_getrlimit() warn: potential >>>>> spectre issue 'get_current()->signal->rlim' (local cap) >>>>> kernel/sys.c:1455 __do_sys_old_getrlimit() warn: potential spectre >>>>> issue >>>>> 'get_current()->signal->rlim' (local cap) >>>>> >>>>> Fix this by sanitizing *resource* before using it to index >>>>> current->signal->rlim >>>>> >>>>> Notice that given that speculation windows are large, the policy is >>>>> to kill the speculation on the first load and not worry if it can be >>>>> completed with a dependent load/store [1]. >>>> >>>> hm. Not my area, but I'm always willing to learn ;) >>>> >>>>> --- a/kernel/sys.c >>>>> +++ b/kernel/sys.c >>>>> @@ -69,6 +69,9 @@ >>>>> #include <asm/io.h> >>>>> #include <asm/unistd.h> >>>>> >>>>> +/* Hardening for Spectre-v1 */ >>>>> +#include <linux/nospec.h> >>>>> + >>>>> #include "uid16.h" >>>>> >>>>> #ifndef SET_UNALIGN_CTL >>>>> @@ -1451,6 +1454,7 @@ SYSCALL_DEFINE2(old_getrlimit, unsigned int, >>>>> resource, >>>>> if (resource >= RLIM_NLIMITS) >>>>> return -EINVAL; >>>>> >>>>> + resource = array_index_nospec(resource, RLIM_NLIMITS); >>>>> task_lock(current->group_leader); >>>>> x = current->signal->rlim[resource]; >>>> >>>> Can the speculation proceed past the task_lock()? Or is the policy to >>>> ignore such happy happenstances even if they are available? >>> >>> Locks are not in the way of speculation. Speculation has almost no >>> limits >>> except serializing instructions. At least they respect the magic AND >>> limitation in array_index_nospec(). >> >> I'd say it another way, because they don't respect the magic AND, we >> just make the result in the speculation path safe. So, it's controlled >> speculation. >> > > Dan, > > What do you think about adding the following function to the nospec API: > > diff --git a/include/linux/nospec.h b/include/linux/nospec.h > index e791ebc..81e9a77 100644 > --- a/include/linux/nospec.h > +++ b/include/linux/nospec.h > @@ -55,4 +55,17 @@ static inline unsigned long > array_index_mask_nospec(unsigned long index, > \ > (typeof(_i)) (_i & _mask); \ > }) > + > + > +#ifndef sanitize_index_nospec > +inline bool sanitize_index_nospec(unsigned long index, > + unsigned long size) > +{ > + if (index >= size) > + return false; > + index = array_index_nospec(index, size); > + > + return true; > +} > +#endif > #endif /* _LINUX_NOSPEC_H */ > Oops, it seems I sent the wrong patch. The function would look like this: #ifndef sanitize_index_nospec inline bool sanitize_index_nospec(unsigned long *index, unsigned long size) { if (*index >= size) return false; *index = array_index_nospec(*index, size); return true; } #endif Thanks -- Gustavo ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-18 19:21 ` Gustavo A. R. Silva @ 2018-05-18 20:38 ` Dan Williams 2018-05-18 20:44 ` Gustavo A. R. Silva 0 siblings, 1 reply; 26+ messages in thread From: Dan Williams @ 2018-05-18 20:38 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On Fri, May 18, 2018 at 12:21 PM, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: > > > On 05/18/2018 02:04 PM, Gustavo A. R. Silva wrote: >> >> >> >> On 05/15/2018 05:57 PM, Dan Williams wrote: >>> >>> On Tue, May 15, 2018 at 3:29 PM, Thomas Gleixner <tglx@linutronix.de> >>> wrote: >>>> >>>> On Tue, 15 May 2018, Andrew Morton wrote: >>>>> >>>>> On Mon, 14 May 2018 22:00:38 -0500 "Gustavo A. R. Silva" >>>>> <gustavo@embeddedor.com> wrote: >>>>> >>>>>> resource can be controlled by user-space, hence leading to a >>>>>> potential exploitation of the Spectre variant 1 vulnerability. >>>>>> >>>>>> This issue was detected with the help of Smatch: >>>>>> >>>>>> kernel/sys.c:1474 __do_compat_sys_old_getrlimit() warn: potential >>>>>> spectre issue 'get_current()->signal->rlim' (local cap) >>>>>> kernel/sys.c:1455 __do_sys_old_getrlimit() warn: potential spectre >>>>>> issue >>>>>> 'get_current()->signal->rlim' (local cap) >>>>>> >>>>>> Fix this by sanitizing *resource* before using it to index >>>>>> current->signal->rlim >>>>>> >>>>>> Notice that given that speculation windows are large, the policy is >>>>>> to kill the speculation on the first load and not worry if it can be >>>>>> completed with a dependent load/store [1]. >>>>> >>>>> >>>>> hm. Not my area, but I'm always willing to learn ;) >>>>> >>>>>> --- a/kernel/sys.c >>>>>> +++ b/kernel/sys.c >>>>>> @@ -69,6 +69,9 @@ >>>>>> #include <asm/io.h> >>>>>> #include <asm/unistd.h> >>>>>> >>>>>> +/* Hardening for Spectre-v1 */ >>>>>> +#include <linux/nospec.h> >>>>>> + >>>>>> #include "uid16.h" >>>>>> >>>>>> #ifndef SET_UNALIGN_CTL >>>>>> @@ -1451,6 +1454,7 @@ SYSCALL_DEFINE2(old_getrlimit, unsigned int, >>>>>> resource, >>>>>> if (resource >= RLIM_NLIMITS) >>>>>> return -EINVAL; >>>>>> >>>>>> + resource = array_index_nospec(resource, RLIM_NLIMITS); >>>>>> task_lock(current->group_leader); >>>>>> x = current->signal->rlim[resource]; >>>>> >>>>> >>>>> Can the speculation proceed past the task_lock()? Or is the policy to >>>>> ignore such happy happenstances even if they are available? >>>> >>>> >>>> Locks are not in the way of speculation. Speculation has almost no >>>> limits >>>> except serializing instructions. At least they respect the magic AND >>>> limitation in array_index_nospec(). >>> >>> >>> I'd say it another way, because they don't respect the magic AND, we >>> just make the result in the speculation path safe. So, it's controlled >>> speculation. >>> >> >> Dan, >> >> What do you think about adding the following function to the nospec API: >> >> diff --git a/include/linux/nospec.h b/include/linux/nospec.h >> index e791ebc..81e9a77 100644 >> --- a/include/linux/nospec.h >> +++ b/include/linux/nospec.h >> @@ -55,4 +55,17 @@ static inline unsigned long >> array_index_mask_nospec(unsigned long index, >> \ >> (typeof(_i)) (_i & _mask); \ >> }) >> + >> + >> +#ifndef sanitize_index_nospec >> +inline bool sanitize_index_nospec(unsigned long index, >> + unsigned long size) >> +{ >> + if (index >= size) >> + return false; >> + index = array_index_nospec(index, size); >> + >> + return true; >> +} >> +#endif >> #endif /* _LINUX_NOSPEC_H */ >> > > Oops, it seems I sent the wrong patch. The function would look like this: > > #ifndef sanitize_index_nospec > inline bool sanitize_index_nospec(unsigned long *index, > unsigned long size) > { > if (*index >= size) > return false; > *index = array_index_nospec(*index, size); > > return true; > } > #endif I think this is fine in concept, we already do something similar in mpls_label_ok(). Perhaps call it validate_index_nospec() since validation is something that can fail, but sanitization in theory is something that can always succeed. However, the problem is the data type of the index. I expect you would need to do this in a macro and use typeof() if you wanted this to be generally useful, and also watch out for multiple usage of a macro argument. Is it still worth it at that point? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-18 20:38 ` Dan Williams @ 2018-05-18 20:44 ` Gustavo A. R. Silva 2018-05-18 21:27 ` Gustavo A. R. Silva 2018-05-21 0:50 ` Gustavo A. R. Silva 0 siblings, 2 replies; 26+ messages in thread From: Gustavo A. R. Silva @ 2018-05-18 20:44 UTC (permalink / raw) To: Dan Williams Cc: Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On 05/18/2018 03:38 PM, Dan Williams wrote: > On Fri, May 18, 2018 at 12:21 PM, Gustavo A. R. Silva > <gustavo@embeddedor.com> wrote: >> >> >> On 05/18/2018 02:04 PM, Gustavo A. R. Silva wrote: >>> >>> >>> >>> On 05/15/2018 05:57 PM, Dan Williams wrote: >>>> >>>> On Tue, May 15, 2018 at 3:29 PM, Thomas Gleixner <tglx@linutronix.de> >>>> wrote: >>>>> >>>>> On Tue, 15 May 2018, Andrew Morton wrote: >>>>>> >>>>>> On Mon, 14 May 2018 22:00:38 -0500 "Gustavo A. R. Silva" >>>>>> <gustavo@embeddedor.com> wrote: >>>>>> >>>>>>> resource can be controlled by user-space, hence leading to a >>>>>>> potential exploitation of the Spectre variant 1 vulnerability. >>>>>>> >>>>>>> This issue was detected with the help of Smatch: >>>>>>> >>>>>>> kernel/sys.c:1474 __do_compat_sys_old_getrlimit() warn: potential >>>>>>> spectre issue 'get_current()->signal->rlim' (local cap) >>>>>>> kernel/sys.c:1455 __do_sys_old_getrlimit() warn: potential spectre >>>>>>> issue >>>>>>> 'get_current()->signal->rlim' (local cap) >>>>>>> >>>>>>> Fix this by sanitizing *resource* before using it to index >>>>>>> current->signal->rlim >>>>>>> >>>>>>> Notice that given that speculation windows are large, the policy is >>>>>>> to kill the speculation on the first load and not worry if it can be >>>>>>> completed with a dependent load/store [1]. >>>>>> >>>>>> >>>>>> hm. Not my area, but I'm always willing to learn ;) >>>>>> >>>>>>> --- a/kernel/sys.c >>>>>>> +++ b/kernel/sys.c >>>>>>> @@ -69,6 +69,9 @@ >>>>>>> #include <asm/io.h> >>>>>>> #include <asm/unistd.h> >>>>>>> >>>>>>> +/* Hardening for Spectre-v1 */ >>>>>>> +#include <linux/nospec.h> >>>>>>> + >>>>>>> #include "uid16.h" >>>>>>> >>>>>>> #ifndef SET_UNALIGN_CTL >>>>>>> @@ -1451,6 +1454,7 @@ SYSCALL_DEFINE2(old_getrlimit, unsigned int, >>>>>>> resource, >>>>>>> if (resource >= RLIM_NLIMITS) >>>>>>> return -EINVAL; >>>>>>> >>>>>>> + resource = array_index_nospec(resource, RLIM_NLIMITS); >>>>>>> task_lock(current->group_leader); >>>>>>> x = current->signal->rlim[resource]; >>>>>> >>>>>> >>>>>> Can the speculation proceed past the task_lock()? Or is the policy to >>>>>> ignore such happy happenstances even if they are available? >>>>> >>>>> >>>>> Locks are not in the way of speculation. Speculation has almost no >>>>> limits >>>>> except serializing instructions. At least they respect the magic AND >>>>> limitation in array_index_nospec(). >>>> >>>> >>>> I'd say it another way, because they don't respect the magic AND, we >>>> just make the result in the speculation path safe. So, it's controlled >>>> speculation. >>>> >>> >>> Dan, >>> >>> What do you think about adding the following function to the nospec API: >>> >>> diff --git a/include/linux/nospec.h b/include/linux/nospec.h >>> index e791ebc..81e9a77 100644 >>> --- a/include/linux/nospec.h >>> +++ b/include/linux/nospec.h >>> @@ -55,4 +55,17 @@ static inline unsigned long >>> array_index_mask_nospec(unsigned long index, >>> \ >>> (typeof(_i)) (_i & _mask); \ >>> }) >>> + >>> + >>> +#ifndef sanitize_index_nospec >>> +inline bool sanitize_index_nospec(unsigned long index, >>> + unsigned long size) >>> +{ >>> + if (index >= size) >>> + return false; >>> + index = array_index_nospec(index, size); >>> + >>> + return true; >>> +} >>> +#endif >>> #endif /* _LINUX_NOSPEC_H */ >>> >> >> Oops, it seems I sent the wrong patch. The function would look like this: >> >> #ifndef sanitize_index_nospec >> inline bool sanitize_index_nospec(unsigned long *index, >> unsigned long size) >> { >> if (*index >= size) >> return false; >> *index = array_index_nospec(*index, size); >> >> return true; >> } >> #endif > > I think this is fine in concept, we already do something similar in > mpls_label_ok(). Perhaps call it validate_index_nospec() since > validation is something that can fail, but sanitization in theory is > something that can always succeed. > OK. I got it. > However, the problem is the data type of the index. I expect you would > need to do this in a macro and use typeof() if you wanted this to be > generally useful, and also watch out for multiple usage of a macro > argument. Is it still worth it at that point? > Yeah. I think it is worth it. I'll work on this during the weekend and send a proper patch for review. Thanks for the feedback. -- Gustavo ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-18 20:44 ` Gustavo A. R. Silva @ 2018-05-18 21:27 ` Gustavo A. R. Silva 2018-05-18 21:45 ` Dan Williams 2018-05-21 0:50 ` Gustavo A. R. Silva 1 sibling, 1 reply; 26+ messages in thread From: Gustavo A. R. Silva @ 2018-05-18 21:27 UTC (permalink / raw) To: Dan Williams Cc: Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On 05/18/2018 03:44 PM, Gustavo A. R. Silva wrote: >>>> >>> >>> Oops, it seems I sent the wrong patch. The function would look like >>> this: >>> >>> #ifndef sanitize_index_nospec >>> inline bool sanitize_index_nospec(unsigned long *index, >>> unsigned long size) >>> { >>> if (*index >= size) >>> return false; >>> *index = array_index_nospec(*index, size); >>> >>> return true; >>> } >>> #endif >> >> I think this is fine in concept, we already do something similar in >> mpls_label_ok(). Perhaps call it validate_index_nospec() since >> validation is something that can fail, but sanitization in theory is >> something that can always succeed. >> > > OK. I got it. > >> However, the problem is the data type of the index. I expect you would >> need to do this in a macro and use typeof() if you wanted this to be >> generally useful, and also watch out for multiple usage of a macro >> argument. Is it still worth it at that point? >> > > Yeah. I think it is worth it. I'll work on this during the weekend and > send a proper patch for review. > > Thanks for the feedback. BTW, I'm analyzing other cases, like the following: bool foo(int x) { if(!validate_index_nospec(&x)) return false; [...] return true; } int vulnerable(int x) { if (!foo(x)) return -1; temp = array[x]; [...] } Basically my doubt is how deep this barrier can be placed into the call chain in order to continue working. Thanks -- Gustavo ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-18 21:27 ` Gustavo A. R. Silva @ 2018-05-18 21:45 ` Dan Williams 2018-05-18 22:01 ` Gustavo A. R. Silva 0 siblings, 1 reply; 26+ messages in thread From: Dan Williams @ 2018-05-18 21:45 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On Fri, May 18, 2018 at 2:27 PM, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: > > > On 05/18/2018 03:44 PM, Gustavo A. R. Silva wrote: >>>>> >>>>> >>>> >>>> Oops, it seems I sent the wrong patch. The function would look like >>>> this: >>>> >>>> #ifndef sanitize_index_nospec >>>> inline bool sanitize_index_nospec(unsigned long *index, >>>> unsigned long size) >>>> { >>>> if (*index >= size) >>>> return false; >>>> *index = array_index_nospec(*index, size); >>>> >>>> return true; >>>> } >>>> #endif >>> >>> >>> I think this is fine in concept, we already do something similar in >>> mpls_label_ok(). Perhaps call it validate_index_nospec() since >>> validation is something that can fail, but sanitization in theory is >>> something that can always succeed. >>> >> >> OK. I got it. >> >>> However, the problem is the data type of the index. I expect you would >>> need to do this in a macro and use typeof() if you wanted this to be >>> generally useful, and also watch out for multiple usage of a macro >>> argument. Is it still worth it at that point? >>> >> >> Yeah. I think it is worth it. I'll work on this during the weekend and >> send a proper patch for review. >> >> Thanks for the feedback. > > > BTW, I'm analyzing other cases, like the following: > > bool foo(int x) > { > if(!validate_index_nospec(&x)) > return false; > > [...] > > return true; > } > > int vulnerable(int x) > { > if (!foo(x)) > return -1; > > temp = array[x]; > > [...] > > } > > Basically my doubt is how deep this barrier can be placed into the call > chain in order to continue working. This is broken you would need to pass the address of x into foo() otherwise there may be speculation on the return value of foo. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-18 21:45 ` Dan Williams @ 2018-05-18 22:01 ` Gustavo A. R. Silva 2018-05-18 22:08 ` Dan Williams 0 siblings, 1 reply; 26+ messages in thread From: Gustavo A. R. Silva @ 2018-05-18 22:01 UTC (permalink / raw) To: Dan Williams Cc: Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On 05/18/2018 04:45 PM, Dan Williams wrote: > On Fri, May 18, 2018 at 2:27 PM, Gustavo A. R. Silva > <gustavo@embeddedor.com> wrote: >> >> >> On 05/18/2018 03:44 PM, Gustavo A. R. Silva wrote: >>>>>> >>>>>> >>>>> >>>>> Oops, it seems I sent the wrong patch. The function would look like >>>>> this: >>>>> >>>>> #ifndef sanitize_index_nospec >>>>> inline bool sanitize_index_nospec(unsigned long *index, >>>>> unsigned long size) >>>>> { >>>>> if (*index >= size) >>>>> return false; >>>>> *index = array_index_nospec(*index, size); >>>>> >>>>> return true; >>>>> } >>>>> #endif >>>> >>>> >>>> I think this is fine in concept, we already do something similar in >>>> mpls_label_ok(). Perhaps call it validate_index_nospec() since >>>> validation is something that can fail, but sanitization in theory is >>>> something that can always succeed. >>>> >>> >>> OK. I got it. >>> >>>> However, the problem is the data type of the index. I expect you would >>>> need to do this in a macro and use typeof() if you wanted this to be >>>> generally useful, and also watch out for multiple usage of a macro >>>> argument. Is it still worth it at that point? >>>> >>> >>> Yeah. I think it is worth it. I'll work on this during the weekend and >>> send a proper patch for review. >>> >>> Thanks for the feedback. >> >> >> BTW, I'm analyzing other cases, like the following: >> >> bool foo(int x) >> { >> if(!validate_index_nospec(&x)) >> return false; >> >> [...] >> >> return true; >> } >> >> int vulnerable(int x) >> { >> if (!foo(x)) >> return -1; >> >> temp = array[x]; >> >> [...] >> >> }; >> >> Basically my doubt is how deep this barrier can be placed into the call >> chain in order to continue working. > > This is broken you would need to pass the address of x into foo() > otherwise there may be speculation on the return value of foo. > Oh I see now. Just to double check, then something like the following would be broken too, because is basically the same as the code above, and well, it doesn't make much sense to store the value returned by macro array_index_nospec into x, correct?: bool foo(int x) { if(x >= MAX) return false; x = array_index_nospec(x, MAX); return true; } int vulnerable(int x) { if(!foo(x)) return -1; temp = array[x]; [...] } Thanks -- Gustavo ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-18 22:01 ` Gustavo A. R. Silva @ 2018-05-18 22:08 ` Dan Williams 2018-05-18 22:11 ` Gustavo A. R. Silva 0 siblings, 1 reply; 26+ messages in thread From: Dan Williams @ 2018-05-18 22:08 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On Fri, May 18, 2018 at 3:01 PM, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: > > > On 05/18/2018 04:45 PM, Dan Williams wrote: >> >> On Fri, May 18, 2018 at 2:27 PM, Gustavo A. R. Silva >> <gustavo@embeddedor.com> wrote: >>> >>> >>> >>> On 05/18/2018 03:44 PM, Gustavo A. R. Silva wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>> Oops, it seems I sent the wrong patch. The function would look like >>>>>> this: >>>>>> >>>>>> #ifndef sanitize_index_nospec >>>>>> inline bool sanitize_index_nospec(unsigned long *index, >>>>>> unsigned long size) >>>>>> { >>>>>> if (*index >= size) >>>>>> return false; >>>>>> *index = array_index_nospec(*index, size); >>>>>> >>>>>> return true; >>>>>> } >>>>>> #endif >>>>> >>>>> >>>>> >>>>> I think this is fine in concept, we already do something similar in >>>>> mpls_label_ok(). Perhaps call it validate_index_nospec() since >>>>> validation is something that can fail, but sanitization in theory is >>>>> something that can always succeed. >>>>> >>>> >>>> OK. I got it. >>>> >>>>> However, the problem is the data type of the index. I expect you would >>>>> need to do this in a macro and use typeof() if you wanted this to be >>>>> generally useful, and also watch out for multiple usage of a macro >>>>> argument. Is it still worth it at that point? >>>>> >>>> >>>> Yeah. I think it is worth it. I'll work on this during the weekend and >>>> send a proper patch for review. >>>> >>>> Thanks for the feedback. >>> >>> >>> >>> BTW, I'm analyzing other cases, like the following: >>> >>> bool foo(int x) >>> { >>> if(!validate_index_nospec(&x)) >>> return false; >>> >>> [...] >>> >>> return true; >>> } >>> >>> int vulnerable(int x) >>> { >>> if (!foo(x)) >>> return -1; >>> >>> temp = array[x]; >>> >>> [...] >>> >>> }; >>> >>> Basically my doubt is how deep this barrier can be placed into the call >>> chain in order to continue working. >> >> >> This is broken you would need to pass the address of x into foo() >> otherwise there may be speculation on the return value of foo. >> > > Oh I see now. Just to double check, then something like the following would > be broken too, because is basically the same as the code above, and well, it > doesn't make much sense to store the value returned by macro > array_index_nospec into x, correct?: Correct, broken: > > bool foo(int x) > { > if(x >= MAX) > return false; Under speculation we may not return here when x is greater than max. > x = array_index_nospec(x, MAX); x is now sanitized under speculation to zero, but the compiler would likely just throw this away because nothing consumes it. > return true; > } > > int vulnerable(int x) > { > if(!foo(x)) > return -1; cpu might speculate that this branch is not taken... > > temp = array[x]; ...so x had better be bounded here, otherwise Spectre. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-18 22:08 ` Dan Williams @ 2018-05-18 22:11 ` Gustavo A. R. Silva 0 siblings, 0 replies; 26+ messages in thread From: Gustavo A. R. Silva @ 2018-05-18 22:11 UTC (permalink / raw) To: Dan Williams Cc: Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On 05/18/2018 05:08 PM, Dan Williams wrote: >> >> Oh I see now. Just to double check, then something like the following would >> be broken too, because is basically the same as the code above, and well, it >> doesn't make much sense to store the value returned by macro >> array_index_nospec into x, correct?: > > Correct, broken: > >> >> bool foo(int x) >> { >> if(x >= MAX) >> return false; > > Under speculation we may not return here when x is greater than max. > >> x = array_index_nospec(x, MAX); > > x is now sanitized under speculation to zero, but the compiler would > likely just throw this away because nothing consumes it. > >> return true; >> } >> >> int vulnerable(int x) >> { >> if(!foo(x)) >> return -1; > > cpu might speculate that this branch is not taken... > >> >> temp = array[x]; > > ...so x had better be bounded here, otherwise Spectre. > I got it. I appreciate the feedback. Thanks, Dan. -- Gustavo ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-18 20:44 ` Gustavo A. R. Silva 2018-05-18 21:27 ` Gustavo A. R. Silva @ 2018-05-21 0:50 ` Gustavo A. R. Silva 2018-05-21 2:00 ` Gustavo A. R. Silva 1 sibling, 1 reply; 26+ messages in thread From: Gustavo A. R. Silva @ 2018-05-21 0:50 UTC (permalink / raw) To: Dan Williams Cc: Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On 05/18/2018 03:44 PM, Gustavo A. R. Silva wrote: >>> >>> #ifndef sanitize_index_nospec >>> inline bool sanitize_index_nospec(unsigned long *index, >>> unsigned long size) >>> { >>> if (*index >= size) >>> return false; >>> *index = array_index_nospec(*index, size); >>> >>> return true; >>> } >>> #endif >> >> I think this is fine in concept, we already do something similar in >> mpls_label_ok(). Perhaps call it validate_index_nospec() since >> validation is something that can fail, but sanitization in theory is >> something that can always succeed. >> > > OK. I got it. > >> However, the problem is the data type of the index. I expect you would >> need to do this in a macro and use typeof() if you wanted this to be >> generally useful, and also watch out for multiple usage of a macro >> argument. Is it still worth it at that point? >> > > Yeah. I think it is worth it. I'll work on this during the weekend and > send a proper patch for review. > Dan, What do you think about this first draft: diff --git a/include/linux/nospec.h b/include/linux/nospec.h index e791ebc..6154183 100644 --- a/include/linux/nospec.h +++ b/include/linux/nospec.h @@ -55,4 +55,16 @@ static inline unsigned long array_index_mask_nospec(unsigned long index, \ (typeof(_i)) (_i & _mask); \ }) + +#define validate_index_nospec(index, size) \ +({ \ + typeof(index) *ptr = &(index); \ + typeof(size) _s = (size); \ + \ + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \ + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ + \ + *ptr >= _s ? false : \ + (*ptr = array_index_nospec(*ptr, _s) ? true : true); \ +}) #endif /* _LINUX_NOSPEC_H */ Thanks -- Gustavo ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-21 0:50 ` Gustavo A. R. Silva @ 2018-05-21 2:00 ` Gustavo A. R. Silva 2018-05-22 20:50 ` Dan Williams 0 siblings, 1 reply; 26+ messages in thread From: Gustavo A. R. Silva @ 2018-05-21 2:00 UTC (permalink / raw) To: Dan Williams Cc: Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On 05/20/2018 07:50 PM, Gustavo A. R. Silva wrote: > > > On 05/18/2018 03:44 PM, Gustavo A. R. Silva wrote: >>>> >>>> #ifndef sanitize_index_nospec >>>> inline bool sanitize_index_nospec(unsigned long *index, >>>> unsigned long size) >>>> { >>>> if (*index >= size) >>>> return false; >>>> *index = array_index_nospec(*index, size); >>>> >>>> return true; >>>> } >>>> #endif >>> >>> I think this is fine in concept, we already do something similar in >>> mpls_label_ok(). Perhaps call it validate_index_nospec() since >>> validation is something that can fail, but sanitization in theory is >>> something that can always succeed. >>> >> >> OK. I got it. >> >>> However, the problem is the data type of the index. I expect you would >>> need to do this in a macro and use typeof() if you wanted this to be >>> generally useful, and also watch out for multiple usage of a macro >>> argument. Is it still worth it at that point? >>> >> >> Yeah. I think it is worth it. I'll work on this during the weekend and >> send a proper patch for review. >> > > Dan, > > What do you think about this first draft: > > diff --git a/include/linux/nospec.h b/include/linux/nospec.h > index e791ebc..6154183 100644 > --- a/include/linux/nospec.h > +++ b/include/linux/nospec.h > @@ -55,4 +55,16 @@ static inline unsigned long > array_index_mask_nospec(unsigned long index, > \ > (typeof(_i)) (_i & _mask); \ > }) > + > +#define validate_index_nospec(index, size) \ > +({ \ > + typeof(index) *ptr = &(index); \ > + typeof(size) _s = (size); \ > + \ > + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \ > + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ > + \ > + *ptr >= _s ? false : \ > + (*ptr = array_index_nospec(*ptr, _s) ? true : true); \ This actually should be: ((*ptr = array_index_nospec(*ptr, _s)) ? true : true); > +}) > #endif /* _LINUX_NOSPEC_H */ > > Thanks > -- > Gustavo ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-21 2:00 ` Gustavo A. R. Silva @ 2018-05-22 20:50 ` Dan Williams 2018-05-23 5:03 ` Gustavo A. R. Silva 0 siblings, 1 reply; 26+ messages in thread From: Dan Williams @ 2018-05-22 20:50 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On Sun, May 20, 2018 at 7:00 PM, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: > > > On 05/20/2018 07:50 PM, Gustavo A. R. Silva wrote: >> >> >> >> On 05/18/2018 03:44 PM, Gustavo A. R. Silva wrote: >>>>> >>>>> >>>>> #ifndef sanitize_index_nospec >>>>> inline bool sanitize_index_nospec(unsigned long *index, >>>>> unsigned long size) >>>>> { >>>>> if (*index >= size) >>>>> return false; >>>>> *index = array_index_nospec(*index, size); >>>>> >>>>> return true; >>>>> } >>>>> #endif >>>> >>>> >>>> I think this is fine in concept, we already do something similar in >>>> mpls_label_ok(). Perhaps call it validate_index_nospec() since >>>> validation is something that can fail, but sanitization in theory is >>>> something that can always succeed. >>>> >>> >>> OK. I got it. >>> >>>> However, the problem is the data type of the index. I expect you would >>>> need to do this in a macro and use typeof() if you wanted this to be >>>> generally useful, and also watch out for multiple usage of a macro >>>> argument. Is it still worth it at that point? >>>> >>> >>> Yeah. I think it is worth it. I'll work on this during the weekend and >>> send a proper patch for review. >>> >> >> Dan, >> >> What do you think about this first draft: >> >> diff --git a/include/linux/nospec.h b/include/linux/nospec.h >> index e791ebc..6154183 100644 >> --- a/include/linux/nospec.h >> +++ b/include/linux/nospec.h >> @@ -55,4 +55,16 @@ static inline unsigned long >> array_index_mask_nospec(unsigned long index, >> \ >> (typeof(_i)) (_i & _mask); \ >> }) >> + >> +#define validate_index_nospec(index, size) \ >> +({ \ >> + typeof(index) *ptr = &(index); \ >> + typeof(size) _s = (size); \ >> + \ >> + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \ >> + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ >> + \ >> + *ptr >= _s ? false : \ >> + (*ptr = array_index_nospec(*ptr, _s) ? true : true); \ > > > This actually should be: > > ((*ptr = array_index_nospec(*ptr, _s)) ? true : true); > Let's not use ternary conditionals at all to make this more readable. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-22 20:50 ` Dan Williams @ 2018-05-23 5:03 ` Gustavo A. R. Silva 2018-05-23 5:15 ` Dan Williams 2018-05-23 9:08 ` Peter Zijlstra 0 siblings, 2 replies; 26+ messages in thread From: Gustavo A. R. Silva @ 2018-05-23 5:03 UTC (permalink / raw) To: Dan Williams Cc: Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On 05/22/2018 03:50 PM, Dan Williams wrote: >>> >>> Dan, >>> >>> What do you think about this first draft: >>> >>> diff --git a/include/linux/nospec.h b/include/linux/nospec.h >>> index e791ebc..6154183 100644 >>> --- a/include/linux/nospec.h >>> +++ b/include/linux/nospec.h >>> @@ -55,4 +55,16 @@ static inline unsigned long >>> array_index_mask_nospec(unsigned long index, >>> \ >>> (typeof(_i)) (_i & _mask); \ >>> }) >>> + >>> +#define validate_index_nospec(index, size) \ >>> +({ \ >>> + typeof(index) *ptr = &(index); \ >>> + typeof(size) _s = (size); \ >>> + \ >>> + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \ >>> + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ >>> + \ >>> + *ptr >= _s ? false : \ >>> + (*ptr = array_index_nospec(*ptr, _s) ? true : true); \ >> >> >> This actually should be: >> >> ((*ptr = array_index_nospec(*ptr, _s)) ? true : true); >> > > Let's not use ternary conditionals at all to make this more readable. > OK. How about this: diff --git a/include/linux/nospec.h b/include/linux/nospec.h index e791ebc..498995b 100644 --- a/include/linux/nospec.h +++ b/include/linux/nospec.h @@ -55,4 +55,21 @@ static inline unsigned long array_index_mask_nospec(unsigned long index, \ (typeof(_i)) (_i & _mask); \ }) + +#define validate_index_nospec(index, size) \ +({ \ + bool ret = true; \ + typeof(index) *ptr = &(index); \ + typeof(size) _s = (size); \ + \ + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \ + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ + \ + if (*ptr >= size) \ + ret = false; \ + \ + *ptr = array_index_nospec(*ptr, _s); \ + \ + ret; \ +}) #endif /* _LINUX_NOSPEC_H */ Thanks -- Gustavo ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-23 5:03 ` Gustavo A. R. Silva @ 2018-05-23 5:15 ` Dan Williams 2018-05-23 5:22 ` Gustavo A. R. Silva 2018-05-23 9:08 ` Peter Zijlstra 1 sibling, 1 reply; 26+ messages in thread From: Dan Williams @ 2018-05-23 5:15 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On Tue, May 22, 2018 at 10:03 PM, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: > > > On 05/22/2018 03:50 PM, Dan Williams wrote: >>>> >>>> >>>> Dan, >>>> >>>> What do you think about this first draft: >>>> >>>> diff --git a/include/linux/nospec.h b/include/linux/nospec.h >>>> index e791ebc..6154183 100644 >>>> --- a/include/linux/nospec.h >>>> +++ b/include/linux/nospec.h >>>> @@ -55,4 +55,16 @@ static inline unsigned long >>>> array_index_mask_nospec(unsigned long index, >>>> >>>> \ >>>> (typeof(_i)) (_i & _mask); >>>> \ >>>> }) >>>> + >>>> +#define validate_index_nospec(index, size) \ >>>> +({ \ >>>> + typeof(index) *ptr = &(index); \ >>>> + typeof(size) _s = (size); \ >>>> + \ >>>> + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \ >>>> + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ >>>> + \ >>>> + *ptr >= _s ? false : \ >>>> + (*ptr = array_index_nospec(*ptr, _s) ? true : true); \ >>> >>> >>> >>> This actually should be: >>> >>> ((*ptr = array_index_nospec(*ptr, _s)) ? true : true); >>> >> >> Let's not use ternary conditionals at all to make this more readable. >> > > OK. How about this: > > diff --git a/include/linux/nospec.h b/include/linux/nospec.h > index e791ebc..498995b 100644 > --- a/include/linux/nospec.h > +++ b/include/linux/nospec.h > @@ -55,4 +55,21 @@ static inline unsigned long > array_index_mask_nospec(unsigned long index, > \ > (typeof(_i)) (_i & _mask); \ > }) > + > +#define validate_index_nospec(index, size) \ > +({ \ > + bool ret = true; \ > + typeof(index) *ptr = &(index); \ > + typeof(size) _s = (size); \ > + \ > + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \ > + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ > + \ > + if (*ptr >= size) \ > + ret = false; \ > + \ > + *ptr = array_index_nospec(*ptr, _s); \ > + \ > + ret; \ > > +}) > #endif /* _LINUX_NOSPEC_H */ Assuming the assembly generation is comparable with the open coded version, this looks ok to me. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-23 5:15 ` Dan Williams @ 2018-05-23 5:22 ` Gustavo A. R. Silva 0 siblings, 0 replies; 26+ messages in thread From: Gustavo A. R. Silva @ 2018-05-23 5:22 UTC (permalink / raw) To: Dan Williams Cc: Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov, Peter Zijlstra On 05/23/2018 12:15 AM, Dan Williams wrote: >> >> OK. How about this: >> >> diff --git a/include/linux/nospec.h b/include/linux/nospec.h >> index e791ebc..498995b 100644 >> --- a/include/linux/nospec.h >> +++ b/include/linux/nospec.h >> @@ -55,4 +55,21 @@ static inline unsigned long >> array_index_mask_nospec(unsigned long index, >> \ >> (typeof(_i)) (_i & _mask); \ >> }) >> + >> +#define validate_index_nospec(index, size) \ >> +({ \ >> + bool ret = true; \ >> + typeof(index) *ptr = &(index); \ >> + typeof(size) _s = (size); \ >> + \ >> + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \ >> + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ >> + \ >> + if (*ptr >= size) \ I'll change the line above by this one: if (*ptr >= _s) >> + ret = false; \ >> + \ >> + *ptr = array_index_nospec(*ptr, _s); \ >> + \ >> + ret; \ >> >> +}) >> #endif /* _LINUX_NOSPEC_H */ > > Assuming the assembly generation is comparable with the open coded > version, this looks ok to me. > OK. I'll send a proper patch tomorrow morning. Thanks for the feedback, Dan. -- Gustavo ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-23 5:03 ` Gustavo A. R. Silva 2018-05-23 5:15 ` Dan Williams @ 2018-05-23 9:08 ` Peter Zijlstra 2018-05-23 13:55 ` Dan Williams 2018-05-23 15:07 ` Mark Rutland 1 sibling, 2 replies; 26+ messages in thread From: Peter Zijlstra @ 2018-05-23 9:08 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Dan Williams, Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov Sorry for being late to the party.. On Wed, May 23, 2018 at 12:03:57AM -0500, Gustavo A. R. Silva wrote: > +#define validate_index_nospec(index, size) \ > +({ \ > + bool ret = true; \ > + typeof(index) *ptr = &(index); \ > + typeof(size) _s = (size); \ > + \ > + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \ > + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ > + \ > + if (*ptr >= size) \ > + ret = false; \ > + \ > + *ptr = array_index_nospec(*ptr, _s); \ > + \ > + ret; \ > +}) Would not something like: bool ret = false; .... if (*ptr < _s) { *ptr = array_index_nospec(*ptr, _s); ret = true; } ret; be more obvious? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-23 9:08 ` Peter Zijlstra @ 2018-05-23 13:55 ` Dan Williams 2018-05-23 15:07 ` Mark Rutland 1 sibling, 0 replies; 26+ messages in thread From: Dan Williams @ 2018-05-23 13:55 UTC (permalink / raw) To: Peter Zijlstra Cc: Gustavo A. R. Silva, Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov On Wed, May 23, 2018 at 2:08 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > Sorry for being late to the party.. > > On Wed, May 23, 2018 at 12:03:57AM -0500, Gustavo A. R. Silva wrote: > >> +#define validate_index_nospec(index, size) \ >> +({ \ >> + bool ret = true; \ >> + typeof(index) *ptr = &(index); \ >> + typeof(size) _s = (size); \ >> + \ >> + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \ >> + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ >> + \ >> + if (*ptr >= size) \ >> + ret = false; \ >> + \ >> + *ptr = array_index_nospec(*ptr, _s); \ >> + \ >> + ret; \ >> +}) > > Would not something like: > > bool ret = false; > > .... > > if (*ptr < _s) { > *ptr = array_index_nospec(*ptr, _s); > ret = true; > } > > ret; > > be more obvious? Yes, that looks even better to me. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-23 9:08 ` Peter Zijlstra 2018-05-23 13:55 ` Dan Williams @ 2018-05-23 15:07 ` Mark Rutland 2018-05-23 15:57 ` Dan Williams ` (2 more replies) 1 sibling, 3 replies; 26+ messages in thread From: Mark Rutland @ 2018-05-23 15:07 UTC (permalink / raw) To: Peter Zijlstra Cc: Gustavo A. R. Silva, Dan Williams, Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov On Wed, May 23, 2018 at 11:08:40AM +0200, Peter Zijlstra wrote: > > Sorry for being late to the party.. Likewise! > On Wed, May 23, 2018 at 12:03:57AM -0500, Gustavo A. R. Silva wrote: > > +#define validate_index_nospec(index, size) \ > > +({ \ > > + bool ret = true; \ > > + typeof(index) *ptr = &(index); \ > > + typeof(size) _s = (size); \ > > + \ > > + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \ > > + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ > > + \ > > + if (*ptr >= size) \ > > + ret = false; \ > > + \ > > + *ptr = array_index_nospec(*ptr, _s); \ > > + \ > > + ret; \ > > +}) > > Would not something like: > > bool ret = false; > > .... > > if (*ptr < _s) { > *ptr = array_index_nospec(*ptr, _s); > ret = true; > } > > ret; > > be more obvious? I think that either way, we have a potential problem if the compiler generates a branch dependent on the result of validate_index_nospec(). In that case, we could end up with codegen approximating: bool safe = false; if (idx < bound) { idx = array_index_nospec(idx, bound); safe = true; } // this branch can be mispredicted if (safe) { foo = array[idx]; } ... and thus we lose the nospec protection. I also suspect that compiler transformations mean that this might already be the case for patterns like: if (idx < bound) { safe_idx = array_index_nospec(idx, bound)]; ... foo = array[safe_idx]; } ... if the compiler can transform that to something like: if (idx < bound) { idx = array_index_nospec(idx, bound); } // can be mispredicted if (idx < bound) { foo = array[idx]; } ... which I think a compiler might be capable of, depending on the rest of the function body (e.g. if there's a common portion shared with the else case). I'll see if I can trigger that in a test case. :/ Thanks, Mark. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-23 15:07 ` Mark Rutland @ 2018-05-23 15:57 ` Dan Williams 2018-05-23 16:27 ` Peter Zijlstra 2018-05-23 16:31 ` Mark Rutland 2 siblings, 0 replies; 26+ messages in thread From: Dan Williams @ 2018-05-23 15:57 UTC (permalink / raw) To: Mark Rutland Cc: Peter Zijlstra, Gustavo A. R. Silva, Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov On Wed, May 23, 2018 at 8:07 AM, Mark Rutland <mark.rutland@arm.com> wrote: > On Wed, May 23, 2018 at 11:08:40AM +0200, Peter Zijlstra wrote: >> >> Sorry for being late to the party.. > > Likewise! > >> On Wed, May 23, 2018 at 12:03:57AM -0500, Gustavo A. R. Silva wrote: >> > +#define validate_index_nospec(index, size) \ >> > +({ \ >> > + bool ret = true; \ >> > + typeof(index) *ptr = &(index); \ >> > + typeof(size) _s = (size); \ >> > + \ >> > + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \ >> > + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ >> > + \ >> > + if (*ptr >= size) \ >> > + ret = false; \ >> > + \ >> > + *ptr = array_index_nospec(*ptr, _s); \ >> > + \ >> > + ret; \ >> > +}) >> >> Would not something like: >> >> bool ret = false; >> >> .... >> >> if (*ptr < _s) { >> *ptr = array_index_nospec(*ptr, _s); >> ret = true; >> } >> >> ret; >> >> be more obvious? > > I think that either way, we have a potential problem if the compiler > generates a branch dependent on the result of validate_index_nospec(). > > In that case, we could end up with codegen approximating: > > bool safe = false; > > if (idx < bound) { > idx = array_index_nospec(idx, bound); > safe = true; > } > > // this branch can be mispredicted > if (safe) { > foo = array[idx]; > } > > ... and thus we lose the nospec protection. > > I also suspect that compiler transformations mean that this might > already be the case for patterns like: > > if (idx < bound) { > safe_idx = array_index_nospec(idx, bound)]; > ... > foo = array[safe_idx]; > } > > ... if the compiler can transform that to something like: > > if (idx < bound) { > idx = array_index_nospec(idx, bound); > } > > // can be mispredicted > if (idx < bound) { > foo = array[idx]; > } > > ... which I think a compiler might be capable of, depending on the rest > of the function body (e.g. if there's a common portion shared with the > else case). > > I'll see if I can trigger that in a test case. :/ This would be interesting, because my operating assumption is that the compiler will not play these games over inline asm, i.e. the index will always be modified before use in all cases. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-23 15:07 ` Mark Rutland 2018-05-23 15:57 ` Dan Williams @ 2018-05-23 16:27 ` Peter Zijlstra 2018-05-23 16:31 ` Mark Rutland 2 siblings, 0 replies; 26+ messages in thread From: Peter Zijlstra @ 2018-05-23 16:27 UTC (permalink / raw) To: Mark Rutland Cc: Gustavo A. R. Silva, Dan Williams, Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov On Wed, May 23, 2018 at 04:07:37PM +0100, Mark Rutland wrote: > I think that either way, we have a potential problem if the compiler > generates a branch dependent on the result of validate_index_nospec(). > > In that case, we could end up with codegen approximating: > > bool safe = false; > > if (idx < bound) { > idx = array_index_nospec(idx, bound); > safe = true; > } > > // this branch can be mispredicted > if (safe) { > foo = array[idx]; > } > > ... and thus we lose the nospec protection. I was assuming the compiler would not do that, that's pretty stupid code-gen. But you're right in calling that out, because I think it's entirely in it's right to do that :/ > I also suspect that compiler transformations mean that this might > already be the case for patterns like: > > if (idx < bound) { > safe_idx = array_index_nospec(idx, bound)]; > ... > foo = array[safe_idx]; > } > > ... if the compiler can transform that to something like: > > if (idx < bound) { > idx = array_index_nospec(idx, bound); > } > > // can be mispredicted > if (idx < bound) { > foo = array[idx]; > } > > ... which I think a compiler might be capable of, depending on the rest > of the function body (e.g. if there's a common portion shared with the > else case). > > I'll see if I can trigger that in a test case. :/ *groan*... ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-23 15:07 ` Mark Rutland 2018-05-23 15:57 ` Dan Williams 2018-05-23 16:27 ` Peter Zijlstra @ 2018-05-23 16:31 ` Mark Rutland 2018-05-25 18:11 ` Gustavo A. R. Silva 2 siblings, 1 reply; 26+ messages in thread From: Mark Rutland @ 2018-05-23 16:31 UTC (permalink / raw) To: Peter Zijlstra Cc: Gustavo A. R. Silva, Dan Williams, Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov On Wed, May 23, 2018 at 04:07:37PM +0100, Mark Rutland wrote: > I think that either way, we have a potential problem if the compiler > generates a branch dependent on the result of validate_index_nospec(). > > In that case, we could end up with codegen approximating: > > bool safe = false; > > if (idx < bound) { > idx = array_index_nospec(idx, bound); > safe = true; > } > > // this branch can be mispredicted > if (safe) { > foo = array[idx]; > } > > ... and thus we lose the nospec protection. I see GCC do this at -O0, but so far I haven't tricked it into doing this at -O1 or above. Regardless, I worry this is fragile -- GCC *can* generate code as per the above, even if it's unlikely to. > I also suspect that compiler transformations mean that this might > already be the case for patterns like: > > if (idx < bound) { > safe_idx = array_index_nospec(idx, bound)]; > ... > foo = array[safe_idx]; > } > > ... if the compiler can transform that to something like: > > if (idx < bound) { > idx = array_index_nospec(idx, bound); > } > > // can be mispredicted > if (idx < bound) { > foo = array[idx]; > } > > ... which I think a compiler might be capable of, depending on the rest > of the function body (e.g. if there's a common portion shared with the > else case). > > I'll see if I can trigger that in a test case. :/ No luck so far, but I'll keeep fighting... GCC will happily pull a common suffix after the branch, e.g. if (cond) { foo(); bar(); } else { bar(); } .. goes to: if (cond) foo() bar(); ... but I can't convince it to pull a common prefix before the branch. Mark. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] kernel: sys: fix potential Spectre v1 2018-05-23 16:31 ` Mark Rutland @ 2018-05-25 18:11 ` Gustavo A. R. Silva 0 siblings, 0 replies; 26+ messages in thread From: Gustavo A. R. Silva @ 2018-05-25 18:11 UTC (permalink / raw) To: Mark Rutland, Peter Zijlstra Cc: Dan Williams, Thomas Gleixner, Andrew Morton, Linux Kernel Mailing List, Alexei Starovoitov On 05/23/2018 11:31 AM, Mark Rutland wrote: > On Wed, May 23, 2018 at 04:07:37PM +0100, Mark Rutland wrote: >> I think that either way, we have a potential problem if the compiler >> generates a branch dependent on the result of validate_index_nospec(). >> >> In that case, we could end up with codegen approximating: >> >> bool safe = false; >> >> if (idx < bound) { >> idx = array_index_nospec(idx, bound); >> safe = true; >> } >> >> // this branch can be mispredicted >> if (safe) { >> foo = array[idx]; >> } >> >> ... and thus we lose the nospec protection. > > I see GCC do this at -O0, but so far I haven't tricked it into doing > this at -O1 or above. > > Regardless, I worry this is fragile -- GCC *can* generate code as per > the above, even if it's unlikely to. > >> I also suspect that compiler transformations mean that this might >> already be the case for patterns like: >> >> if (idx < bound) { >> safe_idx = array_index_nospec(idx, bound)]; >> ... >> foo = array[safe_idx]; >> } >> >> ... if the compiler can transform that to something like: >> >> if (idx < bound) { >> idx = array_index_nospec(idx, bound); >> } >> >> // can be mispredicted >> if (idx < bound) { >> foo = array[idx]; >> } >> >> ... which I think a compiler might be capable of, depending on the rest >> of the function body (e.g. if there's a common portion shared with the >> else case). >> >> I'll see if I can trigger that in a test case. :/ > > No luck so far, but I'll keeep fighting... > > GCC will happily pull a common suffix after the branch, e.g. > > if (cond) { > foo(); > bar(); > } else { > bar(); > } > > .. goes to: > > if (cond) > foo() > > bar(); > > ... but I can't convince it to pull a common prefix before the branch. > > Mark. > I will send the following patch once Dan's [1] has been applied upstream. diff --git a/include/linux/nospec.h b/include/linux/nospec.h index e791ebc..2a1ab2e 100644 --- a/include/linux/nospec.h +++ b/include/linux/nospec.h @@ -55,4 +55,21 @@ static inline unsigned long array_index_mask_nospec(unsigned long index, \ (typeof(_i)) (_i & _mask); \ }) + +#define validate_index_nospec(index, size) \ +({ \ + bool ret = false; \ + typeof(index) *ptr = &(index); \ + typeof(size) _s = (size); \ + \ + BUILD_BUG_ON(sizeof(*ptr) > sizeof(long)); \ + BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ + \ + if (*ptr < _s) { \ + *ptr = array_index_nospec(*ptr, _s); \ + ret = true; \ + } \ + \ + ret; \ +}) [1] https://marc.info/?l=linux-kernel&m=152726947109104&w=2 Thank you, Dan, Peter and Mark for your feedback. -- Gustavo ^ permalink raw reply related [flat|nested] 26+ messages in thread
end of thread, other threads:[~2018-05-25 18:11 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-15 3:00 [PATCH] kernel: sys: fix potential Spectre v1 Gustavo A. R. Silva 2018-05-15 22:08 ` Andrew Morton 2018-05-15 22:29 ` Thomas Gleixner 2018-05-15 22:57 ` Dan Williams 2018-05-18 19:04 ` Gustavo A. R. Silva 2018-05-18 19:21 ` Gustavo A. R. Silva 2018-05-18 20:38 ` Dan Williams 2018-05-18 20:44 ` Gustavo A. R. Silva 2018-05-18 21:27 ` Gustavo A. R. Silva 2018-05-18 21:45 ` Dan Williams 2018-05-18 22:01 ` Gustavo A. R. Silva 2018-05-18 22:08 ` Dan Williams 2018-05-18 22:11 ` Gustavo A. R. Silva 2018-05-21 0:50 ` Gustavo A. R. Silva 2018-05-21 2:00 ` Gustavo A. R. Silva 2018-05-22 20:50 ` Dan Williams 2018-05-23 5:03 ` Gustavo A. R. Silva 2018-05-23 5:15 ` Dan Williams 2018-05-23 5:22 ` Gustavo A. R. Silva 2018-05-23 9:08 ` Peter Zijlstra 2018-05-23 13:55 ` Dan Williams 2018-05-23 15:07 ` Mark Rutland 2018-05-23 15:57 ` Dan Williams 2018-05-23 16:27 ` Peter Zijlstra 2018-05-23 16:31 ` Mark Rutland 2018-05-25 18:11 ` Gustavo A. R. Silva
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).