linux-kernel - Re: [PATCH 03/11] block: add rq->resid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4A093A34.3020101@panasas.com>
Date:	Tue, 12 May 2009 11:58:28 +0300
From:	Boaz Harrosh <bharrosh@...asas.com>
To:	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
CC:	tj@...nel.org, axboe@...nel.dk, linux-kernel@...r.kernel.org,
	jeff@...zik.org, linux-ide@...r.kernel.org,
	James.Bottomley@...senPartnership.com, linux-scsi@...r.kernel.org,
	bzolnier@...il.com, petkovbb@...glemail.com,
	sshtylyov@...mvista.com, mike.miller@...com, Eric.Moore@....com,
	stern@...land.harvard.edu, zaitcev@...hat.com,
	Geert.Uytterhoeven@...ycom.com, sfr@...b.auug.org.au,
	grant.likely@...retlab.ca, paul.clements@...eleye.com,
	tim@...erelk.net, jeremy@...source.com, adrian@...en.demon.co.uk,
	oakad@...oo.com, dwmw2@...radead.org, schwidefsky@...ibm.com,
	ballabio_dario@....com, davem@...emloft.net, rusty@...tcorp.com.au,
	Markus.Lidel@...dowconnect.com, dgilbert@...erlog.com,
	djwong@...ibm.com
Subject: Re: [PATCH 03/11] block: add rq->resid_len

On 05/11/2009 05:59 PM, FUJITA Tomonori wrote:
> On Mon, 11 May 2009 14:31:41 +0300
> Boaz Harrosh <bharrosh@...asas.com> wrote:
> 
>>>>> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
>>>>> index 3da02e4..6605ec9 100644
>>>>> --- a/drivers/scsi/libsas/sas_expander.c
>>>>> +++ b/drivers/scsi/libsas/sas_expander.c
>>>>> @@ -1936,12 +1936,8 @@ int sas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
>>>>>  			       bio_data(rsp->bio), rsp->data_len);
>>>>>  	if (ret > 0) {
>>>>>  		/* positive number is the untransferred residual */
>>>>> -		rsp->data_len = ret;
>>>>> -		req->data_len = 0;
>>>>> +		rsp->resid_len = ret;
>>>>>  		ret = 0;
>>>>> -	} else if (ret == 0) {
>>>>> -		rsp->data_len = 0;
>>>>> -		req->data_len = 0;
>>>>>  	}
>>>>>  
>>>>>  	return ret;
>>>> This is actually a bug fix, as well as a strait conversion
>>> Can you elaborate a bit about the bug fix part?
>>>
>> Nothing big really, just that before (according to the comment), the theoretical
>> negative case would be full-residual. and now it is zero (untouched).
>>
>> I know that in iscsi a negative residual is possible which means over-flow. That is:
>> the target had more data to give then the buffer had space for. (which is not an error at all)
> 
> Hmm, iSCSI? This code is for SAS management Protocol.
> 

I gave that as an example of what the scsi standard says about negative
residual count return from the target. If SAS as sepecific and different
meaning to negative residual, it should be noted and handled.

> smp_execute_task returns a negative value for some errors (ENOMEM, the
> hardware doesn't respond, etc). It's not related with 'transferred
> buffer length' at all.

This is a serious problem, and a violation of the block/scsi stacks. If so, then
in that case error/status should be set properly. And residual cleared.

Failing to return an error might theoretically be catastrophic. If what
you say is certain then previous behaviour is better then after this patch.

I'm not at all familiar with this code. Could you send a patch to
fix these things?

Thanks
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/