aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJoe Lawrence <joe.lawrence@stratus.com>2014-06-25 17:06:54 -0400
committerChristoph Hellwig <hch@lst.de>2014-07-25 17:16:58 -0400
commit9f21316fc2f297efd32b40a57083d5cecb4bda26 (patch)
tree1174b29f2f5d6907ede37373227683ff6876c66c
parent32696198745a74b37bc27498a5ade169d2c04c5e (diff)
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.c10
-rw-r--r--drivers/message/fusion/mptsas.c52
-rw-r--r--drivers/message/fusion/mptscsih.c19
3 files changed, 37 insertions, 44 deletions
diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 9d4c7825a5f2..a896d948b79e 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)
649 case MPI_FUNCTION_CONFIG: 649 case MPI_FUNCTION_CONFIG:
650 case MPI_FUNCTION_SAS_IO_UNIT_CONTROL: 650 case MPI_FUNCTION_SAS_IO_UNIT_CONTROL:
651 ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_COMMAND_GOOD; 651 ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_COMMAND_GOOD;
652 if (reply) { 652 ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_RF_VALID;
653 ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_RF_VALID; 653 memcpy(ioc->mptbase_cmds.reply, reply,
654 memcpy(ioc->mptbase_cmds.reply, reply, 654 min(MPT_DEFAULT_FRAME_SIZE,
655 min(MPT_DEFAULT_FRAME_SIZE, 655 4 * reply->u.reply.MsgLength));
656 4 * reply->u.reply.MsgLength));
657 }
658 if (ioc->mptbase_cmds.status & MPT_MGMT_STATUS_PENDING) { 656 if (ioc->mptbase_cmds.status & MPT_MGMT_STATUS_PENDING) {
659 ioc->mptbase_cmds.status &= ~MPT_MGMT_STATUS_PENDING; 657 ioc->mptbase_cmds.status &= ~MPT_MGMT_STATUS_PENDING;
660 complete(&ioc->mptbase_cmds.done); 658 complete(&ioc->mptbase_cmds.done);
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index b0b74c4d2645..0707fa2c701b 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)
1203 "(mf = %p, mr = %p)\n", ioc->name, mf, mr)); 1203 "(mf = %p, mr = %p)\n", ioc->name, mf, mr));
1204 1204
1205 pScsiTmReply = (SCSITaskMgmtReply_t *)mr; 1205 pScsiTmReply = (SCSITaskMgmtReply_t *)mr;
1206 if (pScsiTmReply) { 1206 if (!pScsiTmReply)
1207 dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT 1207 return 0;
1208 "\tTaskMgmt completed: fw_channel = %d, fw_id = %d,\n" 1208
1209 "\ttask_type = 0x%02X, iocstatus = 0x%04X " 1209 dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT
1210 "loginfo = 0x%08X,\n\tresponse_code = 0x%02X, " 1210 "\tTaskMgmt completed: fw_channel = %d, fw_id = %d,\n"
1211 "term_cmnds = %d\n", ioc->name, 1211 "\ttask_type = 0x%02X, iocstatus = 0x%04X "
1212 pScsiTmReply->Bus, pScsiTmReply->TargetID, 1212 "loginfo = 0x%08X,\n\tresponse_code = 0x%02X, "
1213 pScsiTmReply->TaskType, 1213 "term_cmnds = %d\n", ioc->name,
1214 le16_to_cpu(pScsiTmReply->IOCStatus), 1214 pScsiTmReply->Bus, pScsiTmReply->TargetID,
1215 le32_to_cpu(pScsiTmReply->IOCLogInfo), 1215 pScsiTmReply->TaskType,
1216 pScsiTmReply->ResponseCode, 1216 le16_to_cpu(pScsiTmReply->IOCStatus),
1217 le32_to_cpu(pScsiTmReply->TerminationCount))); 1217 le32_to_cpu(pScsiTmReply->IOCLogInfo),
1218 1218 pScsiTmReply->ResponseCode,
1219 if (pScsiTmReply->ResponseCode) 1219 le32_to_cpu(pScsiTmReply->TerminationCount)));
1220 mptscsih_taskmgmt_response_code(ioc, 1220
1221 pScsiTmReply->ResponseCode); 1221 if (pScsiTmReply->ResponseCode)
1222 } 1222 mptscsih_taskmgmt_response_code(ioc,
1223 1223 pScsiTmReply->ResponseCode);
1224 if (pScsiTmReply && (pScsiTmReply->TaskType == 1224
1225 if (pScsiTmReply->TaskType ==
1225 MPI_SCSITASKMGMT_TASKTYPE_QUERY_TASK || pScsiTmReply->TaskType == 1226 MPI_SCSITASKMGMT_TASKTYPE_QUERY_TASK || pScsiTmReply->TaskType ==
1226 MPI_SCSITASKMGMT_TASKTYPE_ABRT_TASK_SET)) { 1227 MPI_SCSITASKMGMT_TASKTYPE_ABRT_TASK_SET) {
1227 ioc->taskmgmt_cmds.status |= MPT_MGMT_STATUS_COMMAND_GOOD; 1228 ioc->taskmgmt_cmds.status |= MPT_MGMT_STATUS_COMMAND_GOOD;
1228 ioc->taskmgmt_cmds.status |= MPT_MGMT_STATUS_RF_VALID; 1229 ioc->taskmgmt_cmds.status |= MPT_MGMT_STATUS_RF_VALID;
1229 memcpy(ioc->taskmgmt_cmds.reply, mr, 1230 memcpy(ioc->taskmgmt_cmds.reply, mr,
@@ -3853,10 +3854,8 @@ retry_page:
3853 phy_info = mptsas_find_phyinfo_by_sas_address(ioc, 3854 phy_info = mptsas_find_phyinfo_by_sas_address(ioc,
3854 sas_info->sas_address); 3855 sas_info->sas_address);
3855 3856
3856 if (phy_info) { 3857 mptsas_del_end_device(ioc, phy_info);
3857 mptsas_del_end_device(ioc, phy_info); 3858 goto redo_device_scan;
3858 goto redo_device_scan;
3859 }
3860 } else 3859 } else
3861 mptsas_volume_delete(ioc, sas_info->fw.id); 3860 mptsas_volume_delete(ioc, sas_info->fw.id);
3862 } 3861 }
@@ -3867,9 +3866,8 @@ retry_page:
3867 redo_expander_scan: 3866 redo_expander_scan:
3868 list_for_each_entry(port_info, &ioc->sas_topology, list) { 3867 list_for_each_entry(port_info, &ioc->sas_topology, list) {
3869 3868
3870 if (port_info->phy_info && 3869 if (!(port_info->phy_info[0].identify.device_info &
3871 (!(port_info->phy_info[0].identify.device_info & 3870 MPI_SAS_DEVICE_INFO_SMP_TARGET))
3872 MPI_SAS_DEVICE_INFO_SMP_TARGET)))
3873 continue; 3871 continue;
3874 found_expander = 0; 3872 found_expander = 0;
3875 handle = 0xFFFF; 3873 handle = 0xFFFF;
diff --git a/drivers/message/fusion/mptscsih.c b/drivers/message/fusion/mptscsih.c
index 39e566803089..e7dcb2583369 100644
--- a/drivers/message/fusion/mptscsih.c
+++ b/drivers/message/fusion/mptscsih.c
@@ -1271,15 +1271,13 @@ mptscsih_info(struct Scsi_Host *SChost)
1271 1271
1272 h = shost_priv(SChost); 1272 h = shost_priv(SChost);
1273 1273
1274 if (h) { 1274 if (h->info_kbuf == NULL)
1275 if (h->info_kbuf == NULL) 1275 if ((h->info_kbuf = kmalloc(0x1000 /* 4Kb */, GFP_KERNEL)) == NULL)
1276 if ((h->info_kbuf = kmalloc(0x1000 /* 4Kb */, GFP_KERNEL)) == NULL) 1276 return h->info_kbuf;
1277 return h->info_kbuf; 1277 h->info_kbuf[0] = '\0';
1278 h->info_kbuf[0] = '\0'; 1278
1279 1279 mpt_print_ioc_summary(h->ioc, h->info_kbuf, &size, 0, 0);
1280 mpt_print_ioc_summary(h->ioc, h->info_kbuf, &size, 0, 0); 1280 h->info_kbuf[size-1] = '\0';
1281 h->info_kbuf[size-1] = '\0';
1282 }
1283 1281
1284 return h->info_kbuf; 1282 return h->info_kbuf;
1285} 1283}
@@ -1368,8 +1366,7 @@ mptscsih_qcmd(struct scsi_cmnd *SCpnt)
1368 /* Default to untagged. Once a target structure has been allocated, 1366 /* Default to untagged. Once a target structure has been allocated,
1369 * use the Inquiry data to determine if device supports tagged. 1367 * use the Inquiry data to determine if device supports tagged.
1370 */ 1368 */
1371 if (vdevice 1369 if ((vdevice->vtarget->tflags & MPT_TARGET_FLAGS_Q_YES)
1372 && (vdevice->vtarget->tflags & MPT_TARGET_FLAGS_Q_YES)
1373 && (SCpnt->device->tagged_supported)) { 1370 && (SCpnt->device->tagged_supported)) {
1374 scsictl = scsidir | MPI_SCSIIO_CONTROL_SIMPLEQ; 1371 scsictl = scsidir | MPI_SCSIIO_CONTROL_SIMPLEQ;
1375 if (SCpnt->request && SCpnt->request->ioprio) { 1372 if (SCpnt->request && SCpnt->request->ioprio) {