lkml.org 
[lkml]   [2023]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] scsi: Let scsi_execute_cmd() mark args->sshdr as invalid
From
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

\
 
 \ /
  Last update: 2023-05-18 12:58    [W:0.436 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site