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
