Re: [PATCHv3] mvsas:Fix possible NULL pointer deference in mvs_dev_found_notify

[James Bottomley]

On Sat, 2016-07-02 at 19:16 +0100, Luis de Bethencourt wrote:
> On 02/07/16 18:00, Nicholas Krause wrote:
> > This adds properly checking after the call to mvs_find_dev_mvi
> >  due to this function being able to return a NULL pointer and
> > if this does arise we will deference it in mvs_alloc_dev due
> > to this function never checking if a NULL pointer is given as
> > it's input argument. Signed-off-by: Nicholas Krause <
> > xerofoify@xxxxxxxxx>
> > ---
> > v3 - Make logic simpler on error path by returning -1 directly
> > if mvs_find_dev_mvi returns NULL.
> > v2 - Fix NULL pointer deferenece in error path by calling 
> > spin_unlock_irqrestore on the now NULL pointer, as returned
> > 
> > 
> >  drivers/scsi/mvsas/mv_sas.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/scsi/mvsas/mv_sas.c
> > b/drivers/scsi/mvsas/mv_sas.c
> > index 5b9fcff..dffab01 100644
> > --- a/drivers/scsi/mvsas/mv_sas.c
> > +++ b/drivers/scsi/mvsas/mv_sas.c
> > @@ -1194,6 +1194,8 @@ int mvs_dev_found_notify(struct domain_device
> > *dev, int lock)
> >  	struct mvs_device *mvi_device;
> >  
> >  	mvi = mvs_find_dev_mvi(dev);
> > +	if (!mvi)
> > +		return -1;
> >  
> >  	if (lock)
> >  		spin_lock_irqsave(&mvi->lock, flags);
> > 
> 
> This looks better :)
> 
> Checking the value of mvi makes sense if mvs_find_dev_mvi() can
> return NULL.

Which it can't, if you actually look at the function.  For this to
happen, we'd have to be receiving a discovery event for a non-existent
port on the adapter, meaning the system was so corrupted that operation
shouldn't be continuing.

Nick is a known bogus patch submitter.  If you want to review them,
that's your choice (and perhaps some might be useful), but it's not
unreasonable of me to expect the review will be thorough enough to turn
up issues like this.

James


> Reviewed-by: Luis de Bethencourt <luisbg@xxxxxxxxxxxxxxx>
> 
> Thanks,
> Luis
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi"
> in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>