Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <fb0efbd1-a54f-09d6-bd27-6f665b461e58@acm.org>
Date:   Thu, 18 May 2023 12:54:27 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH] scsi: Let scsi_execute_cmd() mark args->sshdr as invalid
Content-Language: en-US
To:     John Garry <john.g.garry@oracle.com>,
        Juergen Gross <jgross@suse.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>
Cc:     linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
        "James E.J. Bottomley" <jejb@linux.ibm.com>, stable@vger.kernel.org
References: <20230511123432.5793-1-jgross@suse.com>
 <yq1ttwbsoii.fsf@ca-mkp.ca.oracle.com>
 <6614f626-d174-03d0-0993-79e6f6169b71@suse.com>
 <9d356278-c826-dacf-cbe0-79f512b7970e@oracle.com>
 <60aeffe4-b31d-4ea3-d4ea-f50ae25e0316@suse.com>
 <74879c87-689f-6a8e-a177-8bde4c9c4e51@oracle.com>
From:   Bart Van Assche <bvanassche@acm.org>
In-Reply-To: <74879c87-689f-6a8e-a177-8bde4c9c4e51@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 5/18/23 03:57, John Garry wrote:
> I think it's better to fix up the callers.

+1

> 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.

sizeof(struct scsi_sense_hdr) = 8. Using kmalloc() to allocate an eight 
byte data structure sounds like overkill to me. Additionally, making 
scsi_execute_cmd() allocate struct scsi_sense_hdr and letting the 
callers free that data structure will make it harder to review whether 
or not any memory leaks are triggered. No such review is necessary if 
the scsi_execute_cmd() caller allocates that data structure on the stack.

Bart.