linux-kernel - Re: [RFC PATCH] bio-integrity: Copy bip_buf and bip_size in bio_integrity

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:	Mon, 25 May 2009 23:07:04 -0300
From:	Alberto Bertogli <albertito@...tiri.com.ar>
To:	"Martin K. Petersen" <martin.petersen@...cle.com>
Cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	Jens Axboe <jens.axboe@...cle.com>
Subject: Re: [RFC PATCH] bio-integrity: Copy bip_buf and bip_size in
	bio_integrity_clone()

On Mon, May 25, 2009 at 01:04:55AM -0400, Martin K. Petersen wrote:
> >>>>> "Alberto" == Alberto Bertogli <albertito@...tiri.com.ar> writes:
> 
> Alberto> While at it, I found that bio_integrity_clone() does not clone
> Alberto> neither bip_buf nor bip_size, which already copies the bvec,
> Alberto> which should have the same data because it's allocated in
> Alberto> bio_integrity_prep().
> 
> Alberto> Is there any reason I'm missing why they shouldn't be copied in
> Alberto> bio_integrity_clone(), as illustrated in the following patch?
> 
> Yes.  The bip_buf is strictly an internal housekeeping construct.  It
> contains a pointer to the kernel buffer that contains the protection
> information if the protection was added by the block layer itself (via
> bio_integrity_prep()).  However, that's not always the case.  The
> protection information may be passed down from a filesystem or
> (eventually) a userland application.  So the bip_buf is for use by the
> top-level of the block layer exclusively.  The bip_vec is what you want
> to use for accessing the actual protection information.
>
> Also, the cloned bio may be truncated or split.  In that case the
> bip_buf isn't going to match the data bvec.  So in any case blindly
> cloning bip_buf isn't going to work.
>
> Right now the integrity vector itself is cloned together with the bio
> because of the way the block layer works (advancing bvec offset for
> partial completion).  However, I'm brewing on a patch that gets rid of
> that so we can avoid cloning the vector and thus cut down on memory
> allocations as the I/O goes through each remapping layer.  With my patch
> in place the bip_vec becomes immutable and bip_buf will go away.

That makes sense, thanks for the explanation.

The case I was thinking about was something like a filesystem calling
bio_integrity_get_tag() on a cloned bio, since it depends on having a bip_buf
available. But if you're going to remove it altogether then it's a moot
question.


> I took a quick look at your DM patch last week and I didn't see any
> reason why it couldn't hook into the block integrity infrastructure.
> Take a look at drivers/scsi/sd_dif.c for clues on how to do that.

Thanks, I've already implemented it, and will post an updated patch soon, after
I clean it up a little.


> Let me know if you have questions...

Actually, I have two minor questions, if you don't mind:

 - What would be a decent name to use in struct blk_integrity for a module
   such as mine? Is LINUX-DMCSUM-V0-CCITT reasonable?
 - How can I test the tag functions? From a quick grep I found no in-tree
   users of bio_integrity_get/set_tag().

Thanks a lot,
		Alberto

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