Re: [PATCH] a100u2w: Added sanitization for pointer dereference using a value from hardware. Detected using Carburizer (http://lwn.net/Articles/479653/)

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Thu, 2012-12-27 at 08:12 -0600, Asim Kadav wrote:
> > If your theory is that the hardware just returned a bogus value, this
> > isn't the right way to sanitise it because the chances are you'll
> > complete the wrong command and cause corruption: you'd have to halt the
> > entire system at that point.  Also, I don't understand why you think the
> > value should only be 0-31?  The size of variable allocated there is for
> > SCBs up to 243, no idea why, since some of the allocation routines will
> > search up to 256.  However, safety from overrun should be guaranteed at
> > least at the system level by the can_queue value.
> 
> I agree. If the hardware returns a bogus value, it immediately would crash.
> Would a more appropriate patch be checking within the ORC_MAXQUEUE 
> range and flagging an error?

It's probably really not worth it.  The u100w2 is a pretty old SCSI
driver.  I can't imagine there's more than a handful of them left.  As I
said, there's no evidence of a problem.

> > Double checking hardware values isn't something we habitually do unless
> > there's a known reason for it (like the state machine does throw bogus
> > values with a defined recovery procedure).  We definitely don't run in
> > the mode where you can't trust your hardware.
> > 
> 
> Hardware can fail for multiple reasons. Furthermore, such bugs are security
> vulnerabilities too in the case of virtual hardware or USB drivers where
> such values can be faked. The article in my patch gives more details.
> http://lwn.net/Articles/479653/

USB possibly, but this isn't a USB driver.  Virtual hardware is a bit
unlikely, since it's created by the host for the guest.  I don't believe
there's any hypervisor system that can survive attempted subversion by
the *host* (host protecting against guest, yes, but guest trying to
protect against host is a very different ball game).

James