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) |
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 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) { |