Re: Smatch check for Spectre stuff

On Wed, Jun 13, 2018 at 04:10:49PM +0300, Dan Carpenter wrote: > Also it turns out that mem_section[] is declared in mm/sparse.c and > Smatch is supposed to be able to figure out the size of it but > apparently there is a bug... :( I'll take a look at that. Oh. I have CONFIG_SPARSEMEM_EXTREME

Re: Smatch check for Spectre stuff

On Wed, Jun 13, 2018 at 04:10:49PM +0300, Dan Carpenter wrote: > Also it turns out that mem_section[] is declared in mm/sparse.c and > Smatch is supposed to be able to figure out the size of it but > apparently there is a bug... :( I'll take a look at that. Oh. I have CONFIG_SPARSEMEM_EXTREME

Re: Smatch check for Spectre stuff

On Fri, Jun 08, 2018 at 11:12:19AM -0500, Josh Poimboeuf wrote: > I have a few questions/comments. > > 1) I've noticed a common pattern for many of the false positives. >Smatch doesn't seem to detect when the code masks off the array index >to ensure that it's safe. > >For example: >

Re: Smatch check for Spectre stuff

On Fri, Jun 08, 2018 at 11:12:19AM -0500, Josh Poimboeuf wrote: > I have a few questions/comments. > > 1) I've noticed a common pattern for many of the false positives. >Smatch doesn't seem to detect when the code masks off the array index >to ensure that it's safe. > >For example: >

Re: Smatch check for Spectre stuff

On Fri, Jun 08, 2018 at 11:12:19AM -0500, Josh Poimboeuf wrote: > 1) I've noticed a common pattern for many of the false positives. >Smatch doesn't seem to detect when the code masks off the array index >to ensure that it's safe. > >For example: > >> ./include/linux/mmzone.h:1161

Re: Smatch check for Spectre stuff

On Fri, Jun 08, 2018 at 11:12:19AM -0500, Josh Poimboeuf wrote: > 1) I've noticed a common pattern for many of the false positives. >Smatch doesn't seem to detect when the code masks off the array index >to ensure that it's safe. > >For example: > >> ./include/linux/mmzone.h:1161

Re: Smatch check for Spectre stuff

On Thu, Apr 19, 2018 at 08:15:10AM +0300, Dan Carpenter wrote: > Several people have asked me to write this and I think one person was > maybe working on writing it themselves... > > The point of this check is to find place which might be vulnerable to > the Spectre vulnerability. In the kernel

Re: Smatch check for Spectre stuff

On Thu, Apr 19, 2018 at 08:15:10AM +0300, Dan Carpenter wrote: > Several people have asked me to write this and I think one person was > maybe working on writing it themselves... > > The point of this check is to find place which might be vulnerable to > the Spectre vulnerability. In the kernel

Re: Smatch check for Spectre stuff

On Wed, Apr 25, 2018 at 03:48:52PM +0100, Alan Cox wrote: > > 2) Compiler transformations can elide binary operations, so we cannot > >rely on source level AND (&) or MOD (%) operations to narrow the > >range of an expression, regardless of the types of either operand. > > > >This

Re: Smatch check for Spectre stuff

On Wed, Apr 25, 2018 at 03:48:52PM +0100, Alan Cox wrote: > > 2) Compiler transformations can elide binary operations, so we cannot > >rely on source level AND (&) or MOD (%) operations to narrow the > >range of an expression, regardless of the types of either operand. > > > >This

Re: Smatch check for Spectre stuff

> 2) Compiler transformations can elide binary operations, so we cannot >rely on source level AND (&) or MOD (%) operations to narrow the >range of an expression, regardless of the types of either operand. > >This means that source-level AND and MOD operations cannot be relied >

Re: Smatch check for Spectre stuff

> 2) Compiler transformations can elide binary operations, so we cannot >rely on source level AND (&) or MOD (%) operations to narrow the >range of an expression, regardless of the types of either operand. > >This means that source-level AND and MOD operations cannot be relied >

Re: Smatch check for Spectre stuff

Hi Dan, On Thu, Apr 19, 2018 at 08:15:10AM +0300, Dan Carpenter wrote: > Several people have asked me to write this and I think one person was > maybe working on writing it themselves... > > The point of this check is to find place which might be vulnerable to > the Spectre vulnerability. In

Re: Smatch check for Spectre stuff

Hi Dan, On Thu, Apr 19, 2018 at 08:15:10AM +0300, Dan Carpenter wrote: > Several people have asked me to write this and I think one person was > maybe working on writing it themselves... > > The point of this check is to find place which might be vulnerable to > the Spectre vulnerability. In

Re: Smatch check for Spectre stuff

So both smatch and coverity are complaining about sysvipc sems. They look legit (if the policy is, as peterz describes: "kill any speculation on the first load and not worry if it can be completed with a dependent load/store"). On Thu, 19 Apr 2018, Dan Carpenter wrote: ipc/sem.c:1359

Re: Smatch check for Spectre stuff

So both smatch and coverity are complaining about sysvipc sems. They look legit (if the policy is, as peterz describes: "kill any speculation on the first load and not worry if it can be completed with a dependent load/store"). On Thu, 19 Apr 2018, Dan Carpenter wrote: ipc/sem.c:1359

Re: Smatch check for Spectre stuff

On Mon, Apr 23, 2018 at 5:45 AM, Peter Zijlstra wrote: > On Mon, Apr 23, 2018 at 07:31:03AM -0500, Gustavo A. R. Silva wrote: >> Hi Peter, >> >> On 04/20/2018 07:00 AM, Peter Zijlstra wrote: >> > >> > Hi Dan, >> > >> > awesome stuff... >> > >> > So I fear that many are

Re: Smatch check for Spectre stuff

On Mon, Apr 23, 2018 at 5:45 AM, Peter Zijlstra wrote: > On Mon, Apr 23, 2018 at 07:31:03AM -0500, Gustavo A. R. Silva wrote: >> Hi Peter, >> >> On 04/20/2018 07:00 AM, Peter Zijlstra wrote: >> > >> > Hi Dan, >> > >> > awesome stuff... >> > >> > So I fear that many are actually things we want to

Re: Smatch check for Spectre stuff

On Mon, Apr 23, 2018 at 02:22:43PM +0100, Mark Rutland wrote: > On Mon, Apr 23, 2018 at 03:53:07PM +0300, Dan Carpenter wrote: > > On Fri, Apr 20, 2018 at 01:47:51PM +0100, Mark Rutland wrote: > > > > What the test does is it looks at array accesses where the user controls > > > > the offset. It

Re: Smatch check for Spectre stuff

On Mon, Apr 23, 2018 at 02:22:43PM +0100, Mark Rutland wrote: > On Mon, Apr 23, 2018 at 03:53:07PM +0300, Dan Carpenter wrote: > > On Fri, Apr 20, 2018 at 01:47:51PM +0100, Mark Rutland wrote: > > > > What the test does is it looks at array accesses where the user controls > > > > the offset. It

Re: Smatch check for Spectre stuff

On Mon, Apr 23, 2018 at 03:53:07PM +0300, Dan Carpenter wrote: > On Fri, Apr 20, 2018 at 01:47:51PM +0100, Mark Rutland wrote: > > > What the test does is it looks at array accesses where the user controls > > > the offset. It asks "is this a read?" and have we used the > > > array_index_nospec()

Re: Smatch check for Spectre stuff

On Mon, Apr 23, 2018 at 03:53:07PM +0300, Dan Carpenter wrote: > On Fri, Apr 20, 2018 at 01:47:51PM +0100, Mark Rutland wrote: > > > What the test does is it looks at array accesses where the user controls > > > the offset. It asks "is this a read?" and have we used the > > > array_index_nospec()

Re: Smatch check for Spectre stuff

On 04/23/2018 07:45 AM, Peter Zijlstra wrote: On Mon, Apr 23, 2018 at 07:31:03AM -0500, Gustavo A. R. Silva wrote: Hi Peter, On 04/20/2018 07:00 AM, Peter Zijlstra wrote: Hi Dan, awesome stuff... So I fear that many are actually things we want to fix. Our policy was to kill the

Re: Smatch check for Spectre stuff

On 04/23/2018 07:45 AM, Peter Zijlstra wrote: On Mon, Apr 23, 2018 at 07:31:03AM -0500, Gustavo A. R. Silva wrote: Hi Peter, On 04/20/2018 07:00 AM, Peter Zijlstra wrote: Hi Dan, awesome stuff... So I fear that many are actually things we want to fix. Our policy was to kill the

Re: Smatch check for Spectre stuff

On Fri, Apr 20, 2018 at 01:47:51PM +0100, Mark Rutland wrote: > > What the test does is it looks at array accesses where the user controls > > the offset. It asks "is this a read?" and have we used the > > array_index_nospec() macro? If the answers are yes, and no respectively > > then print a

Re: Smatch check for Spectre stuff

On Fri, Apr 20, 2018 at 01:47:51PM +0100, Mark Rutland wrote: > > What the test does is it looks at array accesses where the user controls > > the offset. It asks "is this a read?" and have we used the > > array_index_nospec() macro? If the answers are yes, and no respectively > > then print a

Re: Smatch check for Spectre stuff

On Mon, Apr 23, 2018 at 07:31:03AM -0500, Gustavo A. R. Silva wrote: > Hi Peter, > > On 04/20/2018 07:00 AM, Peter Zijlstra wrote: > > > > Hi Dan, > > > > awesome stuff... > > > > So I fear that many are actually things we want to fix. Our policy was > > to kill the speculation on the first

Re: Smatch check for Spectre stuff

On Mon, Apr 23, 2018 at 07:31:03AM -0500, Gustavo A. R. Silva wrote: > Hi Peter, > > On 04/20/2018 07:00 AM, Peter Zijlstra wrote: > > > > Hi Dan, > > > > awesome stuff... > > > > So I fear that many are actually things we want to fix. Our policy was > > to kill the speculation on the first

Re: Smatch check for Spectre stuff

Hi Peter, On 04/20/2018 07:00 AM, Peter Zijlstra wrote: Hi Dan, awesome stuff... So I fear that many are actually things we want to fix. Our policy was to kill the speculation on the first load and not worry if it can be completed with a dependent load/store. I wonder if there is any thread

Re: Smatch check for Spectre stuff

Hi Peter, On 04/20/2018 07:00 AM, Peter Zijlstra wrote: Hi Dan, awesome stuff... So I fear that many are actually things we want to fix. Our policy was to kill the speculation on the first load and not worry if it can be completed with a dependent load/store. I wonder if there is any thread

Re: Smatch check for Spectre stuff

On 04/20, Thomas Gleixner wrote: > > > kernel/signal.c:3457 do_sigaction() warn: potential spectre issue > > 'p->sighand->action' > > This one is correctly detected Not sure, k = >sighand->action[sig-1]; calculates the addr, although we do '*oact = *k' later. I dunno. > >

Re: Smatch check for Spectre stuff

On 04/20, Thomas Gleixner wrote: > > > kernel/signal.c:3457 do_sigaction() warn: potential spectre issue > > 'p->sighand->action' > > This one is correctly detected Not sure, k = >sighand->action[sig-1]; calculates the addr, although we do '*oact = *k' later. I dunno. > >

Re: Smatch check for Spectre stuff

Hi Dan, On Thu, Apr 19, 2018 at 08:15:10AM +0300, Dan Carpenter wrote: > Several people have asked me to write this and I think one person was > maybe working on writing it themselves... > > The point of this check is to find place which might be vulnerable to > the Spectre vulnerability. In

Re: Smatch check for Spectre stuff

Hi Dan, On Thu, Apr 19, 2018 at 08:15:10AM +0300, Dan Carpenter wrote: > Several people have asked me to write this and I think one person was > maybe working on writing it themselves... > > The point of this check is to find place which might be vulnerable to > the Spectre vulnerability. In

Re: Smatch check for Spectre stuff

On Thu, 19 Apr 2018, Dan Carpenter wrote: > Several people have asked me to write this and I think one person was > maybe working on writing it themselves... > > The point of this check is to find place which might be vulnerable to > the Spectre vulnerability. In the kernel we have the

Re: Smatch check for Spectre stuff

On Thu, 19 Apr 2018, Dan Carpenter wrote: > Several people have asked me to write this and I think one person was > maybe working on writing it themselves... > > The point of this check is to find place which might be vulnerable to > the Spectre vulnerability. In the kernel we have the

Re: Smatch check for Spectre stuff

Hi Dan, awesome stuff... So I fear that many are actually things we want to fix. Our policy was to kill the speculation on the first load and not worry if it can be completed with a dependent load/store. Also, many of the reported things (for the ones I looked at) are on slow paths and fixing

Re: Smatch check for Spectre stuff

Hi Dan, awesome stuff... So I fear that many are actually things we want to fix. Our policy was to kill the speculation on the first load and not worry if it can be completed with a dependent load/store. Also, many of the reported things (for the ones I looked at) are on slow paths and fixing

Re: Smatch check for Spectre stuff

Hi Dan, On 04/19/2018 12:15 AM, Dan Carpenter wrote: Several people have asked me to write this and I think one person was maybe working on writing it themselves... The point of this check is to find place which might be vulnerable to the Spectre vulnerability. In the kernel we have the

Re: Smatch check for Spectre stuff

Hi Dan, On 04/19/2018 12:15 AM, Dan Carpenter wrote: Several people have asked me to write this and I think one person was maybe working on writing it themselves... The point of this check is to find place which might be vulnerable to the Spectre vulnerability. In the kernel we have the

Smatch check for Spectre stuff

Several people have asked me to write this and I think one person was maybe working on writing it themselves... The point of this check is to find place which might be vulnerable to the Spectre vulnerability. In the kernel we have the array_index_nospec() macro which turns off speculation.

Smatch check for Spectre stuff

Several people have asked me to write this and I think one person was maybe working on writing it themselves... The point of this check is to find place which might be vulnerable to the Spectre vulnerability. In the kernel we have the array_index_nospec() macro which turns off speculation.