linux-ext4 - Re: [patch 07/12] rfc: 2fsprogs update

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: <20060927133642.GB25703@openx1.frec.bull.fr>
Date:	Wed, 27 Sep 2006 15:36:42 +0200
From:	Alexandre Ratchov <alexandre.ratchov@...l.net>
To:	Theodore Tso <tytso@....edu>
Cc:	linux-ext4@...r.kernel.org,
	Jean-Pierre Dion <jean-pierre.dion@...l.net>
Subject: Re: [patch 07/12] rfc: 2fsprogs update

On Tue, Sep 26, 2006 at 01:37:00PM -0400, Theodore Tso wrote:
> On Tue, Sep 26, 2006 at 04:48:32PM +0200, Alexandre Ratchov wrote:
> > convert all 32bit on-disk block number definitions (currently __u32,
> > blk_t, unsigned long, unsigned int...) to pblk_t that is defined as
> > __u32. In this way we are sure that blk_t is used only in memory.
> > Later, this would allow to make blk_t 64bit without disturbing any
> > programs (this would just eat more momory and use 64bit arithmetic).
> 
> I *really* dislike this approach, because it makes it very hard to
> prove that we got all of the conversions right --- any mistakes about
> which instances of blk_t need to become pblk_t, and which are supposed
> to stay blk_t, and we end up breaking our ABI compatibility.  And
> let's just say I have higher standards than Greg KH has shown with
> udev.  :-)    
> 
> I also don't like pblk_t, because it's not at all obvious what it
> means. 

It just means "__u32". It's used in very very few places. Mainly to access
on-disk indirect blocks, typically as follows:

		char *ind_block_buf;
		pblk_t *pblk;
		blk_t blk;

		...

		pblk = (pblk_t *)ind_block_buf;

		for (i = 0, ; i < N; i++) {
			blk = *pblk;
			...
			pblk++;
		}

So, the only purpose on the typedef is to "tag" 32bit variables that are
used for such purposes and that have to stay 32bit forever. I expect in the
long term to remove the typedef and to simply use __u32 in such code. For me
this patch is a "temporary stage" in order to avoid confusion between blk_t,
__u32, int, long, etc... and to do the 32bit -> 64bit convertion of block
numbers in a safe manner.

> And what if we want to support 64-bit logicial blocks someday?
> Then it's another painful exercise to figure out which blk_t get
> separated to lblk_t, etc.  
> 

i fully agree here; personnaly, i would like to see only one type for block
numbers: blk_t (or blk64_t if you prefer). And in some sense this patch
contributes to reduce the number of types we use for block numbers. Indeed,
it replaces most int, long, __u32 etc... by blk_t and pblk_t.

> 
> So my plan is to introduce a new type, blk64_t, and create new
> interfaces, such as ext2fs_extent_iterate(), which will use the new
> type --- and which will work with old-style indirect block inodes as
> well (it will translate indirect blocks into extents).

i think we want the same thing, except that i've called blk_t what you call
blk64_t.

In order to get the blk64_t interface working with old-style indirect block
inodes, we have to use __u32 pointers to access indirect blocks and blk64_t
for the rest. If we want to reuse the existing code, we have to walk through
it distinguishing between __u32, long, int, blk_t, and replacing some of them by
blk64_t. That's very close to what this patch does.

Perhaps it's not obvious, but thats my approach to get all of the
conversions right without rewriting everything from scratch ;-). I'm still
open to other approches.

cheers,

-- Alexandre
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html