Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

Joe Lawrence and committed by
Christoph Hellwig
9f21316f 32696198

+34 -41
+4 -6
drivers/message/fusion/mptbase.c
··· 649 649 case MPI_FUNCTION_CONFIG: 650 650 case MPI_FUNCTION_SAS_IO_UNIT_CONTROL: 651 651 ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_COMMAND_GOOD; 652 - if (reply) { 653 - ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_RF_VALID; 654 - memcpy(ioc->mptbase_cmds.reply, reply, 655 - min(MPT_DEFAULT_FRAME_SIZE, 656 - 4 * reply->u.reply.MsgLength)); 657 - } 652 + ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_RF_VALID; 653 + memcpy(ioc->mptbase_cmds.reply, reply, 654 + min(MPT_DEFAULT_FRAME_SIZE, 655 + 4 * reply->u.reply.MsgLength)); 658 656 if (ioc->mptbase_cmds.status & MPT_MGMT_STATUS_PENDING) { 659 657 ioc->mptbase_cmds.status &= ~MPT_MGMT_STATUS_PENDING; 660 658 complete(&ioc->mptbase_cmds.done);
+23 -25
drivers/message/fusion/mptsas.c
··· 1203 1203 "(mf = %p, mr = %p)\n", ioc->name, mf, mr)); 1204 1204 1205 1205 pScsiTmReply = (SCSITaskMgmtReply_t *)mr; 1206 - if (pScsiTmReply) { 1207 - dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT 1208 - "\tTaskMgmt completed: fw_channel = %d, fw_id = %d,\n" 1209 - "\ttask_type = 0x%02X, iocstatus = 0x%04X " 1210 - "loginfo = 0x%08X,\n\tresponse_code = 0x%02X, " 1211 - "term_cmnds = %d\n", ioc->name, 1212 - pScsiTmReply->Bus, pScsiTmReply->TargetID, 1213 - pScsiTmReply->TaskType, 1214 - le16_to_cpu(pScsiTmReply->IOCStatus), 1215 - le32_to_cpu(pScsiTmReply->IOCLogInfo), 1216 - pScsiTmReply->ResponseCode, 1217 - le32_to_cpu(pScsiTmReply->TerminationCount))); 1206 + if (!pScsiTmReply) 1207 + return 0; 1218 1208 1219 - if (pScsiTmReply->ResponseCode) 1220 - mptscsih_taskmgmt_response_code(ioc, 1221 - pScsiTmReply->ResponseCode); 1222 - } 1209 + dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT 1210 + "\tTaskMgmt completed: fw_channel = %d, fw_id = %d,\n" 1211 + "\ttask_type = 0x%02X, iocstatus = 0x%04X " 1212 + "loginfo = 0x%08X,\n\tresponse_code = 0x%02X, " 1213 + "term_cmnds = %d\n", ioc->name, 1214 + pScsiTmReply->Bus, pScsiTmReply->TargetID, 1215 + pScsiTmReply->TaskType, 1216 + le16_to_cpu(pScsiTmReply->IOCStatus), 1217 + le32_to_cpu(pScsiTmReply->IOCLogInfo), 1218 + pScsiTmReply->ResponseCode, 1219 + le32_to_cpu(pScsiTmReply->TerminationCount))); 1223 1220 1224 - if (pScsiTmReply && (pScsiTmReply->TaskType == 1221 + if (pScsiTmReply->ResponseCode) 1222 + mptscsih_taskmgmt_response_code(ioc, 1223 + pScsiTmReply->ResponseCode); 1224 + 1225 + if (pScsiTmReply->TaskType == 1225 1226 MPI_SCSITASKMGMT_TASKTYPE_QUERY_TASK || pScsiTmReply->TaskType == 1226 - MPI_SCSITASKMGMT_TASKTYPE_ABRT_TASK_SET)) { 1227 + MPI_SCSITASKMGMT_TASKTYPE_ABRT_TASK_SET) { 1227 1228 ioc->taskmgmt_cmds.status |= MPT_MGMT_STATUS_COMMAND_GOOD; 1228 1229 ioc->taskmgmt_cmds.status |= MPT_MGMT_STATUS_RF_VALID; 1229 1230 memcpy(ioc->taskmgmt_cmds.reply, mr, ··· 3854 3853 phy_info = mptsas_find_phyinfo_by_sas_address(ioc, 3855 3854 sas_info->sas_address); 3856 3855 3857 - if (phy_info) { 3858 - mptsas_del_end_device(ioc, phy_info); 3859 - goto redo_device_scan; 3860 - } 3856 + mptsas_del_end_device(ioc, phy_info); 3857 + goto redo_device_scan; 3861 3858 } else 3862 3859 mptsas_volume_delete(ioc, sas_info->fw.id); 3863 3860 } ··· 3866 3867 redo_expander_scan: 3867 3868 list_for_each_entry(port_info, &ioc->sas_topology, list) { 3868 3869 3869 - if (port_info->phy_info && 3870 - (!(port_info->phy_info[0].identify.device_info & 3871 - MPI_SAS_DEVICE_INFO_SMP_TARGET))) 3870 + if (!(port_info->phy_info[0].identify.device_info & 3871 + MPI_SAS_DEVICE_INFO_SMP_TARGET)) 3872 3872 continue; 3873 3873 found_expander = 0; 3874 3874 handle = 0xFFFF;
+7 -10
drivers/message/fusion/mptscsih.c
··· 1271 1271 1272 1272 h = shost_priv(SChost); 1273 1273 1274 - if (h) { 1275 - if (h->info_kbuf == NULL) 1276 - if ((h->info_kbuf = kmalloc(0x1000 /* 4Kb */, GFP_KERNEL)) == NULL) 1277 - return h->info_kbuf; 1278 - h->info_kbuf[0] = '\0'; 1274 + if (h->info_kbuf == NULL) 1275 + if ((h->info_kbuf = kmalloc(0x1000 /* 4Kb */, GFP_KERNEL)) == NULL) 1276 + return h->info_kbuf; 1277 + h->info_kbuf[0] = '\0'; 1279 1278 1280 - mpt_print_ioc_summary(h->ioc, h->info_kbuf, &size, 0, 0); 1281 - h->info_kbuf[size-1] = '\0'; 1282 - } 1279 + mpt_print_ioc_summary(h->ioc, h->info_kbuf, &size, 0, 0); 1280 + h->info_kbuf[size-1] = '\0'; 1283 1281 1284 1282 return h->info_kbuf; 1285 1283 } ··· 1366 1368 /* Default to untagged. Once a target structure has been allocated, 1367 1369 * use the Inquiry data to determine if device supports tagged. 1368 1370 */ 1369 - if (vdevice 1370 - && (vdevice->vtarget->tflags & MPT_TARGET_FLAGS_Q_YES) 1371 + if ((vdevice->vtarget->tflags & MPT_TARGET_FLAGS_Q_YES) 1371 1372 && (SCpnt->device->tagged_supported)) { 1372 1373 scsictl = scsidir | MPI_SCSIIO_CONTROL_SIMPLEQ; 1373 1374 if (SCpnt->request && SCpnt->request->ioprio) {