On Mon, Feb 26, 2018 at 12:10:02PM -0500, Joe Moriarty wrote: > The Parfait (version 2.1.0) static code analysis tool found the > following NULL pointer dereference problem.
What is the "CWE 476" thing in the subject line for? > > dev_to_shost() in include/scsi/scsi_host.h has the ability to return > NULL if the scsi host device does not have the Scsi_host->parent > field set. With the possibilty of a NULL pointer being set for > the Scsi_Host->parent field, calls to host_to_us() have to make > sure the return pointer is not null. Changes were made to check > for a return value of NULL on calls to host_to_us(). > > Signed-off-by: Joe Moriarty <[email protected]> > Reviewed-by: Steven Sistare <[email protected]> > Acked-by: Hakon Bugge <[email protected]> > --- > drivers/usb/storage/scsiglue.c | 53 > ++++++++++++++++++++++++++++++++++++------ > 1 file changed, 46 insertions(+), 7 deletions(-) > > diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c > index c267f2812a04..94af609d49bf 100644 > --- a/drivers/usb/storage/scsiglue.c > +++ b/drivers/usb/storage/scsiglue.c > @@ -66,6 +66,9 @@ static int slave_alloc (struct scsi_device *sdev) > { > struct us_data *us = host_to_us(sdev->host); > > + if (!us) > + pr_warn("Error in %s: us = NULL\n", __func__); It is a driver, you should never use pr_* calls, but rather dev_* calls. Also, you don't exit, are you sure the code keeps working after this happens? And what is a user supposed to do with this warning message? > + > /* > * Set the INQUIRY transfer length to 36. We don't use any of > * the extra data and many devices choke if asked for more or > @@ -102,6 +105,11 @@ static int slave_configure(struct scsi_device *sdev) > { > struct us_data *us = host_to_us(sdev->host); > > + if (!us) { > + pr_warn("Error in %s: us = NULL\n", __func__); > + return 0; Why are you returning a success? > + } > + > /* > * Many devices have trouble transferring more than 32KB at a time, > * while others have trouble with more than 64K. At this time we > @@ -331,6 +339,11 @@ static int target_alloc(struct scsi_target *starget) > { > struct us_data *us = host_to_us(dev_to_shost(starget->dev.parent)); Can host_to_us() handle NULL? Nope, just looked at it, this will never cause the return value to be NULL, your checker needs some more work :( > > + if (!us) { > + pr_warn("Error in %s: us = NULL\n", __func__); > + return 0; > + } > + > /* > * Some USB drives don't support REPORT LUNS, even though they > * report a SCSI revision level above 2. Tell the SCSI layer > @@ -361,6 +374,11 @@ static int queuecommand_lck(struct scsi_cmnd *srb, > { > struct us_data *us = host_to_us(srb->device->host); > > + if (!us) { How is that possible? This doesn't have anything to do with your subject line, right? > + pr_warn("Error in %s: us = NULL\n", __func__); > + return 0; success? I stopped reviewing here, sorry :( greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
