[PATCH 05/10] staging: unisys: visorbus: Clarify reason for pointer check in bus_destroy()

Sell, Timothy C Timothy.Sell at unisys.com
Thu Feb 2 12:30:36 UTC 2017


> -----Original Message-----
> From: Greg KH [mailto:gregkh at linuxfoundation.org]
> Sent: Thursday, February 02, 2017 7:14 AM
> To: Kershner, David A <David.Kershner at unisys.com>
> Cc: driverdev-devel at linuxdriverproject.org; *S-Par-Maintainer
> <SParMaintainer at unisys.com>; jes.sorensen at gmail.com; Binder, David
> Anthony <David.Binder at unisys.com>
> Subject: Re: [PATCH 05/10] staging: unisys: visorbus: Clarify reason for pointer
> check in bus_destroy()
> 
> On Wed, Feb 01, 2017 at 05:38:57PM -0500, David Kershner wrote:
> > From: David Binder <david.binder at unisys.com>
> >
> > Clarifies why the pointer returned from visorbus_get_device_by_id() in
> > bus_destroy() is validated. The check is performed in order to be extra
> > careful, for the sake of added security, that the s-Par backend is
> > providing us with a valid bus/device pair.
> >
> > Signed-off-by: David Binder <david.binder at unisys.com>
> > Signed-off-by: David Kershner <david.kershner at unisys.com>
> > Reported-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> > ---
> >  drivers/staging/unisys/visorbus/visorchipset.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/staging/unisys/visorbus/visorchipset.c
> b/drivers/staging/unisys/visorbus/visorchipset.c
> > index c90ea6a..2d1b226 100644
> > --- a/drivers/staging/unisys/visorbus/visorchipset.c
> > +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> > @@ -755,6 +755,7 @@ bus_destroy(struct controlvm_message *inmsg)
> >  	int err;
> >
> >  	bus_info = visorbus_get_device_by_id(bus_no, BUS_ROOT_DEVICE,
> NULL);
> > +	/* Validate that s-Par backend gave a good bus */
> 
> I don't remember what I said in my review, but this comment is pretty
> useless.
> 
> I guess my point is, how could BUS_ROOT_DEVICE ever NOT be a valid
> device on the bus?  What would have made it go away?

We are essentially validating "bus_no" here, whose value we just received
from the s-Par backend via a message.  If bus_no is invalid, i.e., we have
no record of a bus with that number ever being created, then
visorbus_get_device_by_id() will return NULL.
 
> 
> Comments like this don't make much sense, maybe my review didn't either
> :)
> 
> thanks,
> 
> greg k-h


More information about the devel mailing list