2007-09-16 01:22:43

by Theodore Ts'o

[permalink] [raw]
Subject: Potential pitfall in the clusterfs extent patches for e2fsprogs

I've been busy working on extents support in e2fsck, and so as part of
that I was looking at the clusterfs extent patches (mostly for
inspiration to see how they work). One of the things which I noticed is
the patches seem to check the inode direct blocks, and if they look like
the extent header, but the EXTENTS_FL flag isn't set, it flags this as
an error.

The problem with this is that it's fragile; you could potentially have
an inode that happens to have as its first block something which looks
like the extent magic number, and if the second block passes the extent
validity checks, e2fsck will flag an error --- and if e2fsck is run in
preen mode, it will just set the extent flag without prompting the user
or aborting the boot process.

Not a problem for people who are using it for testing, but if anyone is
using these patches in production (such as Gentoo, cough, who is
shipping the patches), I thought I should give a warning --- these
patches are not ready for prime-time, and anyone who wants to use it
production will probably want to take out that particular check.

- Ted


2007-09-22 00:11:54

by Andreas Dilger

[permalink] [raw]
Subject: Re: Potential pitfall in the clusterfs extent patches for e2fsprogs

On Sep 15, 2007 21:22 -0400, Theodore Ts'o wrote:
> The problem with this is that it's fragile; you could potentially have
> an inode that happens to have as its first block something which looks
> like the extent magic number, and if the second block passes the extent
> validity checks, e2fsck will flag an error --- and if e2fsck is run in
> preen mode, it will just set the extent flag without prompting the user
> or aborting the boot process.

Well, I agree it's possible, but given that it is checking 8 bytes for
validity (though only a 2-byte magic) it seems reasonably safe. There
are only 5 blocks in the filesystem that could correctly match in this
case (though I grant they are low-valued blocks due to little-endian
placement of the 16-bit magic). They would have to be allocated as the
first block in the file (0x0000f30a, 0x0001f30a, ..., 0x0004f30a) and
3 always-invalid blocks that would have to be allocated as the second
block (0x000N0002, 0x000N0003, 0x000N0004). The latter are used as
group descriptor and/or bitmap/inode table blocks, so the inode would
likely be corrupt as a non-extent inode as well.

I'm not terribly worried about it.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.