diff options
author | Joe Lawrence <joe.lawrence@stratus.com> | 2014-06-25 17:06:54 -0400 |
---|---|---|
committer | Christoph Hellwig <hch@lst.de> | 2014-07-25 17:16:58 -0400 |
commit | 9f21316fc2f297efd32b40a57083d5cecb4bda26 (patch) | |
tree | 1174b29f2f5d6907ede37373227683ff6876c66c | |
parent | 32696198745a74b37bc27498a5ade169d2c04c5e (diff) | |
download | op-kernel-dev-9f21316fc2f297efd32b40a57083d5cecb4bda26.zip op-kernel-dev-9f21316fc2f297efd32b40a57083d5cecb4bda26.tar.gz |
mptfusion: tweak null pointer checks
Fixes the following smatch warnings:
drivers/message/fusion/mptbase.c:652 mptbase_reply() warn: variable
dereferenced before check 'reply' (see line 639)
[JL: No-brainer, the enclosing switch statement dereferences
reply, so we can't get here unless reply is valid.]
drivers/message/fusion/mptsas.c:1255 mptsas_taskmgmt_complete() error:
we previously assumed 'pScsiTmReply' could be null (see line 1227)
[HCH: Reading the code in mptsas_taskmgmt_complete it's pretty
obvious that it can't do anything useful if mr/pScsiTmReply are
NULL, so I suspect it would be best to just return at the
beginning of the function.
I'd love to understand if it actually could ever be zero, which I
doubt. Maybe the LSI people can shed some light on that?]
drivers/message/fusion/mptsas.c:3888 mptsas_not_responding_devices()
error: we previously assumed 'port_info->phy_info' could be null
(see line 3875)
[HCH: It's pretty obvious from reading mptsas_sas_io_unit_pg0 that
we never register a port_info with a NULL phy_info in the lists,
so all NULL checks on it could be deleted.]
drivers/message/fusion/mptscsih.c:1284 mptscsih_info() error:
we previously assumed 'h' could be null (see line 1274)
[HCH: shost_priv can't return NULL, so the if (h) should be
removed.]
drivers/message/fusion/mptscsih.c:1388 mptscsih_qcmd() error: we
previously assumed 'vdevice' could be null (see line 1373)
[HCH: vdevice can't ever be NULL here, it's allocated in
->slave_alloc and thus guaranteed to be around when
->queuecommand is called.]
Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Acked-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
-rw-r--r-- | drivers/message/fusion/mptbase.c | 10 | ||||
-rw-r--r-- | drivers/message/fusion/mptsas.c | 52 | ||||
-rw-r--r-- | drivers/message/fusion/mptscsih.c | 19 |
3 files changed, 37 insertions, 44 deletions
diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c index 9d4c782..a896d94 100644 --- a/drivers/message/fusion/mptbase.c +++ b/drivers/message/fusion/mptbase.c @@ -649,12 +649,10 @@ mptbase_reply(MPT_ADAPTER *ioc, MPT_FRAME_HDR *req, MPT_FRAME_HDR *reply) case MPI_FUNCTION_CONFIG: case MPI_FUNCTION_SAS_IO_UNIT_CONTROL: ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_COMMAND_GOOD; - if (reply) { - ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_RF_VALID; - memcpy(ioc->mptbase_cmds.reply, reply, - min(MPT_DEFAULT_FRAME_SIZE, - 4 * reply->u.reply.MsgLength)); - } + ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_RF_VALID; + memcpy(ioc->mptbase_cmds.reply, reply, + min(MPT_DEFAULT_FRAME_SIZE, + 4 * reply->u.reply.MsgLength)); if (ioc->mptbase_cmds.status & MPT_MGMT_STATUS_PENDING) { ioc->mptbase_cmds.status &= ~MPT_MGMT_STATUS_PENDING; complete(&ioc->mptbase_cmds.done); diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c index b0b74c4..0707fa2 100644 --- a/drivers/message/fusion/mptsas.c +++ b/drivers/message/fusion/mptsas.c @@ -1203,27 +1203,28 @@ mptsas_taskmgmt_complete(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf, MPT_FRAME_HDR *mr) "(mf = %p, mr = %p)\n", ioc->name, mf, mr)); pScsiTmReply = (SCSITaskMgmtReply_t *)mr; - if (pScsiTmReply) { - dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT - "\tTaskMgmt completed: fw_channel = %d, fw_id = %d,\n" - "\ttask_type = 0x%02X, iocstatus = 0x%04X " - "loginfo = 0x%08X,\n\tresponse_code = 0x%02X, " - "term_cmnds = %d\n", ioc->name, - pScsiTmReply->Bus, pScsiTmReply->TargetID, - pScsiTmReply->TaskType, - le16_to_cpu(pScsiTmReply->IOCStatus), - le32_to_cpu(pScsiTmReply->IOCLogInfo), - pScsiTmReply->ResponseCode, - le32_to_cpu(pScsiTmReply->TerminationCount))); - - if (pScsiTmReply->ResponseCode) - mptscsih_taskmgmt_response_code(ioc, - pScsiTmReply->ResponseCode); - } - - if (pScsiTmReply && (pScsiTmReply->TaskType == + if (!pScsiTmReply) + return 0; + + dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT + "\tTaskMgmt completed: fw_channel = %d, fw_id = %d,\n" + "\ttask_type = 0x%02X, iocstatus = 0x%04X " + "loginfo = 0x%08X,\n\tresponse_code = 0x%02X, " + "term_cmnds = %d\n", ioc->name, + pScsiTmReply->Bus, pScsiTmReply->TargetID, + pScsiTmReply->TaskType, + le16_to_cpu(pScsiTmReply->IOCStatus), + le32_to_cpu(pScsiTmReply->IOCLogInfo), + pScsiTmReply->ResponseCode, + le32_to_cpu(pScsiTmReply->TerminationCount))); + + if (pScsiTmReply->ResponseCode) + mptscsih_taskmgmt_response_code(ioc, + pScsiTmReply->ResponseCode); + + if (pScsiTmReply->TaskType == MPI_SCSITASKMGMT_TASKTYPE_QUERY_TASK || pScsiTmReply->TaskType == - MPI_SCSITASKMGMT_TASKTYPE_ABRT_TASK_SET)) { + MPI_SCSITASKMGMT_TASKTYPE_ABRT_TASK_SET) { ioc->taskmgmt_cmds.status |= MPT_MGMT_STATUS_COMMAND_GOOD; ioc->taskmgmt_cmds.status |= MPT_MGMT_STATUS_RF_VALID; memcpy(ioc->taskmgmt_cmds.reply, mr, @@ -3853,10 +3854,8 @@ retry_page: phy_info = mptsas_find_phyinfo_by_sas_address(ioc, sas_info->sas_address); - if (phy_info) { - mptsas_del_end_device(ioc, phy_info); - goto redo_device_scan; - } + mptsas_del_end_device(ioc, phy_info); + goto redo_device_scan; } else mptsas_volume_delete(ioc, sas_info->fw.id); } @@ -3867,9 +3866,8 @@ retry_page: redo_expander_scan: list_for_each_entry(port_info, &ioc->sas_topology, list) { - if (port_info->phy_info && - (!(port_info->phy_info[0].identify.device_info & - MPI_SAS_DEVICE_INFO_SMP_TARGET))) + if (!(port_info->phy_info[0].identify.device_info & + MPI_SAS_DEVICE_INFO_SMP_TARGET)) continue; found_expander = 0; handle = 0xFFFF; diff --git a/drivers/message/fusion/mptscsih.c b/drivers/message/fusion/mptscsih.c index 39e5668..e7dcb25 100644 --- a/drivers/message/fusion/mptscsih.c +++ b/drivers/message/fusion/mptscsih.c @@ -1271,15 +1271,13 @@ mptscsih_info(struct Scsi_Host *SChost) h = shost_priv(SChost); - if (h) { - if (h->info_kbuf == NULL) - if ((h->info_kbuf = kmalloc(0x1000 /* 4Kb */, GFP_KERNEL)) == NULL) - return h->info_kbuf; - h->info_kbuf[0] = '\0'; - - mpt_print_ioc_summary(h->ioc, h->info_kbuf, &size, 0, 0); - h->info_kbuf[size-1] = '\0'; - } + if (h->info_kbuf == NULL) + if ((h->info_kbuf = kmalloc(0x1000 /* 4Kb */, GFP_KERNEL)) == NULL) + return h->info_kbuf; + h->info_kbuf[0] = '\0'; + + mpt_print_ioc_summary(h->ioc, h->info_kbuf, &size, 0, 0); + h->info_kbuf[size-1] = '\0'; return h->info_kbuf; } @@ -1368,8 +1366,7 @@ mptscsih_qcmd(struct scsi_cmnd *SCpnt) /* Default to untagged. Once a target structure has been allocated, * use the Inquiry data to determine if device supports tagged. */ - if (vdevice - && (vdevice->vtarget->tflags & MPT_TARGET_FLAGS_Q_YES) + if ((vdevice->vtarget->tflags & MPT_TARGET_FLAGS_Q_YES) && (SCpnt->device->tagged_supported)) { scsictl = scsidir | MPI_SCSIIO_CONTROL_SIMPLEQ; if (SCpnt->request && SCpnt->request->ioprio) { |