On Thu, 12 May 2016, Sumit Saxena wrote:
> > From: Dan Carpenter [mailto:[email protected]]
> >
> > 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
