FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> writes:
> The patch looks fine to me. dma_alloc_coherent() doesn't take zero for
> the size argument (causes a kernel crash). The driver can't assume
> that an applications is sane so it needs to check the size that an
> application passed on.
>
> Unfortunately, your patch can't be applied to the latest git. I think
> that you need to submit the updated patch first. After it's merged,
> you can send stable maintainers the modified patch that can be applied
> to stable kernels.
>
> Btw, about your patch, it's better to use "if (!hoge)" instead of "if
> (hoge == 0)"
I believe that is a matter of taste, although I tend to agree that it
looks better. I used the "(hoge == 0)" syntax to try to keep in line
with the style already used in this driver, like e.g.
static int
megasas_queue_command_lck(struct scsi_cmnd *scmd, void (*done) (struct scsi_cmnd *))
{
struct megasas_instance *instance;
unsigned long flags;
instance = (struct megasas_instance *)
scmd->device->host->hostdata;
if (instance->issuepend_done == 0)
return SCSI_MLQUEUE_HOST_BUSY;
but I see now that there are quite a few "if (!hoge)" as well, so I will
update as you suggest.
> and kbuff_arr[] is initialized so seems that you don't
> need to reset it again.
Thanks. Don't understand how I could have missed that.
> The updated patch would be something like:
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> index 5d6d07b..cee1d3b 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -4611,6 +4611,9 @@ megasas_mgmt_fw_ioctl(struct megasas_instance *instance,
> * For each user buffer, create a mirror buffer and copy in
> */
> for (i = 0; i < ioc->sge_count; i++) {
> + if (!ioc->sgl[i].iov_len)
> + continue;
> +
> kbuff_arr[i] = dma_alloc_coherent(&instance->pdev->dev,
> ioc->sgl[i].iov_len,
> &buf_handle, GFP_KERNEL);
Yes, I'll followup with that in a separate mail. Thanks a lot for your
thorough review.
BjÃrn
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
