On Thu, 12 May 2016, Sumit Saxena wrote: > > From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx] > > > > Also when I'm doing static analysis people always tell me that "that > > bug is impossible, trust me." and instead of trusting people I really > > wish they would just show me the relevant code that prevents it from > > happening. > > Inside megasas_build_io_fusion() function, driver sets "cmd->scmd" > pointer(SCSI command pointer received from SCSI mid layer). Functions > called inside megasas_build_io_fusion()(which actually builds frame to > be sent to firmware) are setting Function type- > MPI2_FUNCTION_SCSI_IO_REQUEST (or) MEGASAS_MPI2_FUNCTION_LD_IO_REQUEST. > So in case Function type set to any one these two, there must be valid > "cmd->scmd". That doesn't show what prevents the bug. It merely shows that the bug does not always manifest. For example, you might check whether anything prevents megasas_build_io_fusion() from returning before assigning cmd->scmd, like so: 2112 if (sge_count > instance->max_num_sge) { 2113 dev_err(&instance->pdev->dev, "Error. sge_count (0x%x) exceeds " 2114 "max (0x%x) allowed\n", sge_count, 2115 instance->max_num_sge); 2116 return 1; 2117 } Another possibility: cmd->io_request->Function is valid yet cmd->scmd is NULL when seen from the interrupt handler if it intervenes between the two statements in megasas_return_cmd_fusion(): 180 inline void megasas_return_cmd_fusion(struct megasas_instance *instance, 181 struct megasas_cmd_fusion *cmd) 182 { 183 cmd->scmd = NULL; 184 memset(cmd->io_request, 0, sizeof(struct MPI2_RAID_SCSI_IO_REQUEST)); 185 } You might want to confirm that locking always prevents that. OTOH, without an actual backtrace, I too might be reluctant to pursue this kind of speculation. -- -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
