* [PATCH] scsi: libsas: fix length error in sas_smp_handler()
@ 2017-12-05 9:39 Jason Yan
2017-12-07 1:41 ` Jason Yan
0 siblings, 1 reply; 4+ messages in thread
From: Jason Yan @ 2017-12-05 9:39 UTC (permalink / raw)
To: martin.petersen, jejb; +Cc: linux-scsi, Jason Yan, Christoph Hellwig
The bsg_job_done() requires the length of payload received, but we give
it the untransferred residual.
Fixes: 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for SMP")
Reported-and-tested-by: chenqilin <chenqilin2@huawei.com>
Signed-off-by: Jason Yan <yanaijie@huawei.com>
CC: Christoph Hellwig <hch@lst.de>
---
drivers/scsi/libsas/sas_expander.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 50cb0f3..8323dc1 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -2177,9 +2177,9 @@ void sas_smp_handler(struct bsg_job *job, struct Scsi_Host *shost,
ret = smp_execute_task_sg(dev, job->request_payload.sg_list,
job->reply_payload.sg_list);
- if (ret > 0) {
+ if (ret >= 0) {
/* positive number is the untransferred residual */
- reslen = ret;
+ reslen = job->reply_payload.payload_len - ret;
ret = 0;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] scsi: libsas: fix length error in sas_smp_handler()
2017-12-05 9:39 [PATCH] scsi: libsas: fix length error in sas_smp_handler() Jason Yan
@ 2017-12-07 1:41 ` Jason Yan
2017-12-07 9:27 ` John Garry
0 siblings, 1 reply; 4+ messages in thread
From: Jason Yan @ 2017-12-07 1:41 UTC (permalink / raw)
To: martin.petersen, jejb; +Cc: linux-scsi, Christoph Hellwig
Can anybody review this patch? Our test of SG_IO all failed because of
this bug.
On 2017/12/5 17:39, Jason Yan wrote:
> The bsg_job_done() requires the length of payload received, but we give
> it the untransferred residual.
>
> Fixes: 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for SMP")
> Reported-and-tested-by: chenqilin <chenqilin2@huawei.com>
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> CC: Christoph Hellwig <hch@lst.de>
> ---
> drivers/scsi/libsas/sas_expander.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index 50cb0f3..8323dc1 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -2177,9 +2177,9 @@ void sas_smp_handler(struct bsg_job *job, struct Scsi_Host *shost,
>
> ret = smp_execute_task_sg(dev, job->request_payload.sg_list,
> job->reply_payload.sg_list);
> - if (ret > 0) {
> + if (ret >= 0) {
> /* positive number is the untransferred residual */
> - reslen = ret;
> + reslen = job->reply_payload.payload_len - ret;
> ret = 0;
> }
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] scsi: libsas: fix length error in sas_smp_handler()
2017-12-07 1:41 ` Jason Yan
@ 2017-12-07 9:27 ` John Garry
2017-12-07 10:26 ` Jason Yan
0 siblings, 1 reply; 4+ messages in thread
From: John Garry @ 2017-12-07 9:27 UTC (permalink / raw)
To: Jason Yan, martin.petersen, jejb; +Cc: linux-scsi, Christoph Hellwig, Linuxarm
On 07/12/2017 01:41, Jason Yan wrote:
> Can anybody review this patch? Our test of SG_IO all failed because of
> this bug.
>
> On 2017/12/5 17:39, Jason Yan wrote:
>> The bsg_job_done() requires the length of payload received, but we give
>> it the untransferred residual.
>>
>> Fixes: 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for
>> SMP")
>> Reported-and-tested-by: chenqilin <chenqilin2@huawei.com>
>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>> CC: Christoph Hellwig <hch@lst.de>
>> ---
>> drivers/scsi/libsas/sas_expander.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/libsas/sas_expander.c
>> b/drivers/scsi/libsas/sas_expander.c
>> index 50cb0f3..8323dc1 100644
>> --- a/drivers/scsi/libsas/sas_expander.c
>> +++ b/drivers/scsi/libsas/sas_expander.c
>> @@ -2177,9 +2177,9 @@ void sas_smp_handler(struct bsg_job *job, struct
>> Scsi_Host *shost,
>>
>> ret = smp_execute_task_sg(dev, job->request_payload.sg_list,
>> job->reply_payload.sg_list);
>> - if (ret > 0) {
>> + if (ret >= 0) {
>> /* positive number is the untransferred residual */
>> - reslen = ret;
>> + reslen = job->reply_payload.payload_len - ret;
Hi Jason,
If we really want the length of the payload received, then should you
change the reslen variable name? The name implies "residual length",
which is not really what it is holding according to your change.
Thanks,
John
>> ret = 0;
>> }
>>
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] scsi: libsas: fix length error in sas_smp_handler()
2017-12-07 9:27 ` John Garry
@ 2017-12-07 10:26 ` Jason Yan
0 siblings, 0 replies; 4+ messages in thread
From: Jason Yan @ 2017-12-07 10:26 UTC (permalink / raw)
To: John Garry, martin.petersen, jejb; +Cc: linux-scsi, Christoph Hellwig, Linuxarm
On 2017/12/7 17:27, John Garry wrote:
> On 07/12/2017 01:41, Jason Yan wrote:
>> Can anybody review this patch? Our test of SG_IO all failed because of
>> this bug.
>>
>> On 2017/12/5 17:39, Jason Yan wrote:
>>> The bsg_job_done() requires the length of payload received, but we give
>>> it the untransferred residual.
>>>
>>> Fixes: 651a01364994 ("scsi: scsi_transport_sas: switch to bsg-lib for
>>> SMP")
>>> Reported-and-tested-by: chenqilin <chenqilin2@huawei.com>
>>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>>> CC: Christoph Hellwig <hch@lst.de>
>>> ---
>>> drivers/scsi/libsas/sas_expander.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/scsi/libsas/sas_expander.c
>>> b/drivers/scsi/libsas/sas_expander.c
>>> index 50cb0f3..8323dc1 100644
>>> --- a/drivers/scsi/libsas/sas_expander.c
>>> +++ b/drivers/scsi/libsas/sas_expander.c
>>> @@ -2177,9 +2177,9 @@ void sas_smp_handler(struct bsg_job *job, struct
>>> Scsi_Host *shost,
>>>
>>> ret = smp_execute_task_sg(dev, job->request_payload.sg_list,
>>> job->reply_payload.sg_list);
>>> - if (ret > 0) {
>>> + if (ret >= 0) {
>>> /* positive number is the untransferred residual */
>>> - reslen = ret;
>>> + reslen = job->reply_payload.payload_len - ret;
>
> Hi Jason,
>
> If we really want the length of the payload received, then should you
> change the reslen variable name? The name implies "residual length",
> which is not really what it is holding according to your change.
>
> Thanks,
> John
>
Thanks a lot. I will correct it.
Jason
>>> ret = 0;
>>> }
>>>
>>>
>>
>>
>> .
>>
>
>
>
> .
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-12-07 10:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-05 9:39 [PATCH] scsi: libsas: fix length error in sas_smp_handler() Jason Yan
2017-12-07 1:41 ` Jason Yan
2017-12-07 9:27 ` John Garry
2017-12-07 10:26 ` Jason Yan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).