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]

Date:	Tue, 12 May 2009 19:06:09 +0900
From:	Tejun Heo <tj@...nel.org>
To:	Boaz Harrosh <bharrosh@...asas.com>
CC:	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,
	fujita.tomonori@....ntt.co.jp, 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,
	Doug Gilbert <dgilbert@...erlog.com>,
	"Darrick J. Wong" <djwong@...ibm.com>
Subject: Re: [PATCH 03/11] block: add rq->resid_len

Hello, Boaz.

Boaz Harrosh wrote:
>> Please consider write failing after successfully writing certain
>> number of bytes.  A failed write command MUST NOT cause any actual
>> write on the device.
>>
> 
> I know for sure that scsi has other cases. The above is a
> per-command per-sense-information. Some commands might write/read
> some bytes and still return CHECK-CONDITION with some
> sense-info+residual pertaining what was actually executed.

I see, so we're talking about two different layers here.  At least at
the block layer, there's nothing like partial failure.  I highly doubt
it would work with buffer bouncing and etc.  A failed request is a
failed request.  It may have sense request but doesn't have any
meaningful actual data.

>> Residual count is not about how many bytes have been transferred.  The
>> number of transferred bytes itself doesn't mean a thing because it
>> changes depending on which transfer protocol is used regardless of
>> where the actual failure is.  Residual count is about how many bytes
>> have been actually produced or consumed and when a command fails none
>> should have been.
>>
> See above. I know for a fact that this is not always the case, and
> that it is a per command. In fact I relay on such behaviour in the
> osd filesystems.  If the midlayer would filter such cases for me I
> would have had data corruptions.
> 
> [Read passed end of osd-object will read up to end, and return a
>  warning check condition. With a choice of returning zero-bytes at
>  invalid bytes or returning residual]

Umm... If it's going through block layer, it should be split in such a
way that the initial partial part is completed successuflly and then
the latter part fails when re-issued.  Not sure whether that would be
possible for your code tho.

>> Please note that in the above 'command' and 'residual count' are not
>> as defined in SCSI.  Those terms refer to how those concepts are used
>> in the kernel and via SG_IO interface.
> 
> I do have Kernel code that relays on this. I do use bsg's SG_IO
> successfully that has above behaviour.
> 
> That is: total transparency of residual count and sense information
> communicated between user-code and target. Then user-code can make
> intelligent decisions based on what he's done. And this is what the
> Kernel policy should be. No more, no less.

I'm not sure the said code is safe with regard to buffer bouncing and
all.

...
>> No it's not about being good or bad.  It's about being plain
>> undefined.  There is no value in setting an inherently undefined value
>> to certain values in certain corner cases.
>>
> 
> Again. With some commands/subsystems these are undefined, with some
> like SCSI, they are very well defined in all cases. (Even in the
> undefined cases). So please keep them intact, some code might want
> to relay on this.
> 
> (On the undefined cases Kernel should set a policy on what is
> returned)

IIUC, it's undefined at the block layer, well, at least till now and
it working for your code could just be dumb luck (bouncing not
happened or data transfer direction is to device or whatever).

So, basically, your code which depends on residual count on failure is
broken.  :-(

Thanks.

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