On 18/05/2023 05:53, Juergen Gross wrote:
Is there a reason that callers of scsi_execute_cmd() are not always checking the result for a negative error code (before examining the buffer)?
I don't know.
I've stumbled over the problem while looking into the code due to analyzing a customer's problem. I'm no SCSI expert, but the customer was running Xen and there was the suspicion this could be an underlying Xen issue (which is my area of interest).
It became clear rather quickly that the uninitialized sshdr wasn't the root cause of the customer's problems, but I thought it should be fixed anyway. As there seem to be quite some problematic callers of scsi_execute_cmd(), I've chosen to add the minimal needed initialization of sshdr to scsi_execute_cmd() instead of trying to fix all callers.
ok, understood. I am looking through this thread again, and you seem to have to repeat yourself - sorry about that.
So I don't think that this code has changed from commit 3949e2, as you say.
I think it's better to fix up the callers. Further to that, I dislike how we pass a pointer to this local sshdr structure. I would prefer if scsi_execute_cmd() could kmalloc() the mem for these buffers and the callers could handle free'ing them - I can put together a patch for that, to see what people think.
@Martin, Do you have any preference for what we do now? This code which does not check for error and does not pre-zero sshdr is longstanding, so I am not sure if Juergen's change is required for for v6.4. I'm thinking to fix callers for v6.5 and also maybe change the API, as I described.
Thanks, John