linux-kernel - Re: [PATCH] block: fix residual byte count handling

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 for Android: free password hash cracker in your pocket

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

Date:	Wed, 05 Mar 2008 03:33:38 +0900
From:	Tejun Heo <htejun@...il.com>
To:	James Bottomley <James.Bottomley@...senPartnership.com>
CC:	FUJITA Tomonori <tomof@....org>, efault@....de,
	jens.axboe@...cle.com, fujita.tomonori@....ntt.co.jp,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	linux-ide@...r.kernel.org, linux-scsi@...r.kernel.org,
	jgarzik@...ox.com, bzolnier@...il.com
Subject: Re: [PATCH] block: fix residual byte count handling

James Bottomley wrote:
> On Wed, 2008-03-05 at 01:42 +0900, Tejun Heo wrote:
>> Tejun Heo wrote:
>>> Tejun Heo wrote:
>>>> FUJITA Tomonori wrote:
>>>>>> Aiee... device going down after timing out on READ_DISC_INFO.  That's
>>>>>> gruesome.  Can you please try the other patches?
>>>>> Tejun, I thought that libata needs a fix for sum(sg) != rq->data_len. No?
>>>> The extra_len you added to qc->nbytes should be it.  The only other
>>>> place to pay attention is the ATAPI transfer chunk size and your patch
>>>> seems to get it right.
>>>>
>>>>> Now Jens' git tree should work with all the non libata stuff, ide,
>>>>> firewire, bsg, etc. But I'm not sure about libata.
>>>> With the second patch, all others should be fine no matter what.  I'll
>>>> go check libata part again.
>>> I can reproduce the problem here and it's very weird.  I'll report back
>>> when I know more.
>> Okay, I got it.  Heh, it turns out SCSI and/or block layer is not
>> ready for rq->data_len != sum(sg).  When adjusted command completes,
>> SCSI midlayer completes the command with rq->data_len for PC commands
>> which eventually ends up in __end_that_request_first().  As there are
>> extra sg area left after completing rq->data_len, blk layer says so to
>> SCSI layer and SCSI layer retries the command only with the appended
>> area.
>>
>> The following patch gets the writing going.  I really think it's a
>> serious mistake to break rq->data_len == sum(sg).  If we break
>> rq->data_len == requested size, the worst bugs are giving wrong size
>> when issuing commands to application layer of devices which is
>> relatively easy to spot and not all that command anyway.  Breaking
>> rq->data_len == sum(sg), bugs will be in internal mechanics, DMA
>> engine programming and transport layer.  Oh well...
>>
>> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
>> index fecba05..32439ac 100644
>> --- a/drivers/scsi/scsi.c
>> +++ b/drivers/scsi/scsi.c
>> @@ -757,7 +757,7 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
>>  				"Notifying upper driver of completion "
>>  				"(result %x)\n", cmd->result));
>>  
>> -	good_bytes = scsi_bufflen(cmd);
>> +	good_bytes = scsi_bufflen(cmd) + cmd->request->data_len;
> 
> This doesn't look right.  scsi_bufflen(cmd) is req->data_len for PC
> commands ... did you mean to add extra_len here?

Yeap, sorry about the confusion.  Adding two times data_len accidentally
worked tho.  :-)

-- 
tejun
--
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/