Re: [PATCH 10/18] qla2xxx: prevent bounds-check bypass via speculative execution

[James Bottomley <jejb@linux.vnet.ibm.com>]

On Fri, 2018-01-12 at 08:27 +0100, Greg KH wrote:
> On Thu, Jan 11, 2018 at 02:15:12PM -0800, Dan Williams wrote:
> > 
> > On Sat, Jan 6, 2018 at 1:03 AM, Greg KH <gregkh@linuxfoundation.org
> > > wrote:
> > > 
> > > On Fri, Jan 05, 2018 at 05:10:48PM -0800, Dan Williams wrote:
> > > > 
> > > > Static analysis reports that 'handle' may be a user controlled
> > > > value that is used as a data dependency to read 'sp' from the
> > > > 'req->outstanding_cmds' array.  In order to avoid potential
> > > > leaks of kernel memory values, block speculative execution of
> > > > the instruction stream that could issue reads based on an
> > > > invalid value of 'sp'. In this case 'sp' is directly
> > > > dereferenced later in the function.
> > > 
> > > I'm pretty sure that 'handle' comes from the hardware, not from
> > > userspace, from what I can tell here.  If we want to start
> > > auditing __iomem data sources, great!  But that's a bigger task,
> > > and one I don't think we are ready to tackle...
> > 
> > I think it falls in the hygiene bucket of shutting off an array
> > index from a source that could be under attacker control. Should we
> > leave this one un-patched while we decide if we generally have a
> > problem with trusting completion 'tags' from hardware? My vote is
> > patch it for now.
> 
> Hah, if you are worried about "tags" from hardware, we have a lot
> more auditing to do, right? 

We'd also have a lot more to do: the assumption would have to be
malicious hardware and most hardware has access to fairly vital stuff
directly.  I really don't think we have to worry about side channel
attacks from hardware until the direct attack vector is closed.

James