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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <47C8F4FC.1040505@gmail.com>
Date:	Sat, 01 Mar 2008 15:17:32 +0900
From:	Tejun Heo <htejun@...il.com>
To:	Jens Axboe <jens.axboe@...cle.com>
CC:	James Bottomley <James.Bottomley@...senPartnership.com>,
	Mike Galbraith <efault@....de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>, linux-ide@...r.kernel.org,
	linux-scsi@...r.kernel.org, Jeff Garzik <jgarzik@...ox.com>
Subject: Re: [PATCH] block: fix residual byte count handling

Hello, Jens, James.

Jens Axboe wrote:
>> With the original patch, I have to run through the whole of libsas and
>> scsi_transport_sas doing
>>
>> s/data_len/raw_data_len/
>>
>> With your update it looks like I have to run through them all doing
>>
>> s/data_len/data_len - extra_len/

blk_rq_raw_data_len() should do.

>> which is even worse.  Can't we put things back to a point where data_len
>> means exactly that and extra_len means how much we have spare on the
>> end, so you know you can DMA up to data_len + extra_len if need be?
>>
>> That way we don't have to sweep through every block driver altering the
>> way it uses data_len.

If SMP is broken because it needs start address alignment but not
padding to align the size, what should be done is to make that exact
requirement visible to the block layer.  Say,
blk_queue_dma_start_alignment() or maybe change
blk_queue_dma_alignment() such that it only indicates start address
alignment and add blk_queue_dma_size_alignment() for drivers which
require size to be aligned too.  I think those are few.

I think the decision which value rq->data_len represents comes down to
which size is used more in low level drivers because no matter which way
we choose we'll have to update some of the drivers which expects the
other thing from rq->data_len.

blk_rq_raw_data_len() is needed iff a driver needs dummy buffers
attached at the end and still needs to know the original request size
which isn't the common case.

> Fully agree. The reason why I think it's so ugly is that you have to
> keep these two seperate variables in sync. The burning was just one bug,
> there will be others...

The posted modification isn't too bad as the maintenance of the two
variables is at places where the nasty things happen.  I think what
rq->data_len should represent when seen from LLDs is more important and
please note that if SMP is broken because it simply doesn't require
512byte size alignment, it's a different issue.

As long as both raw_data_len and data_len are accessible, I'm okay
either way.  My biggest reluctance is against breaking sum(sg) ==
rq->data_len.  I think this can lead to much more subtle problems such
as programming the controller w/ wrong bytes count and wrapped-around
resid calculation.

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/