[PATCH] ext3: htree entry integrity checking

[PATCH] ext3: htree entry integrity checking

[Jeff Mahoney]

 This patch adds integrity checking to two htree paths that are missing it.

 Currently, if a corrupted directory entry with rec_len=0 is encountered,
 we still trust that the data is valid. This can cause an infinite loop
 in htree_dirblock_to_tree() since the iteration loop will never make any
 progress.

 I initially had written a ext3_check_htree_entry(), but saw that
 ext3_dir_entry_2 is used for both htree and regular directory entries. Since
 ext3_check_dir_entry() is used for checking ext3_dir_entry_2, I used that.
 Can someone confirm that this is correct? There are other places where
 de->rec_len == 0 is used by itself and I'd be fine doing that too, but I
 figure more integrity checking isn't a bad thing.

 This fixes the problem described at:
 http://projects.info-pull.com/mokb/MOKB-10-11-2006.html

Signed-off-by: Jeff Mahoney <jeffm@suse.com>

diff -ruNpX ../dontdiff linux-2.6.18.orig/fs/ext3/namei.c linux-2.6.18.orig.devel/fs/ext3/namei.c
--- linux-2.6.18.orig/fs/ext3/namei.c	2006-11-09 00:06:37.000000000 -0500
+++ linux-2.6.18.orig.devel/fs/ext3/namei.c	2006-11-12 20:15:19.000000000 -0500
@@ -551,6 +551,11 @@ static int htree_dirblock_to_tree(struct
 					   dir->i_sb->s_blocksize -
 					   EXT3_DIR_REC_LEN(0));
 	for (; de < top; de = ext3_next_entry(de)) {
+		if (!ext3_check_dir_entry(__FUNCTION__, dir, de, bh,
+		                            (char *)de - bh->b_data)) {
+			brelse(bh);
+			return ERR_BAD_DX_DIR;
+		}
 		ext3fs_dirhash(de->name, de->name_len, hinfo);
 		if ((hinfo->hash < start_hash) ||
 		    ((hinfo->hash == start_hash) &&
@@ -617,6 +622,11 @@ int ext3_htree_fill_tree(struct file *di
 	if (start_hash < 2 || (start_hash ==2 && start_minor_hash==0)) {
 		de = (struct ext3_dir_entry_2 *) frames[0].bh->b_data;
 		de = ext3_next_entry(de);
+		if (!ext3_check_dir_entry(__FUNCTION__, dir, de, frames[0].bh,
+		                          (char *)de - frames[0].bh->b_data)) {
+			err = ERR_BAD_DX_DIR;
+			goto errout;
+		}
 		if ((err = ext3_htree_store_dirent(dir_file, 2, 0, de)) != 0)
 			goto errout;
 		count++;

-- 
Jeff Mahoney

Re: [PATCH] ext3: htree entry integrity checking

[Andreas Dilger]

On Nov 16, 2006  11:50 -0500, Jeff Mahoney wrote:
>  Currently, if a corrupted directory entry with rec_len=0 is encountered,
>  we still trust that the data is valid. This can cause an infinite loop
>  in htree_dirblock_to_tree() since the iteration loop will never make any
>  progress.

Actually, I think Eric Sandeen was working on similar fixes already, and
instead of doing a per-item check each time we look at the entry it does
a full-block check the first time it is read (as ext2 does).

>  This fixes the problem described at:
>  http://projects.info-pull.com/mokb/MOKB-10-11-2006.html

Would also be good to CC linux-ext4, where the ext3 maintainers live.
Hmm, maybe we need to update MAINTAINERS with the new list address?

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

Re: [PATCH] ext3: htree entry integrity checking

[Eric Sandeen]

Andreas Dilger wrote:
> On Nov 16, 2006  11:50 -0500, Jeff Mahoney wrote:
>>  Currently, if a corrupted directory entry with rec_len=0 is encountered,
>>  we still trust that the data is valid. This can cause an infinite loop
>>  in htree_dirblock_to_tree() since the iteration loop will never make any
>>  progress.
> 
> Actually, I think Eric Sandeen was working on similar fixes already, and
> instead of doing a per-item check each time we look at the entry it does
> a full-block check the first time it is read (as ext2 does).
> 
>>  This fixes the problem described at:
>>  http://projects.info-pull.com/mokb/MOKB-10-11-2006.html
> 
> Would also be good to CC linux-ext4, where the ext3 maintainers live.
> Hmm, maybe we need to update MAINTAINERS with the new list address?

This should already be fixed, in some fashion, in -mm:

http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.19-rc5/2.6.19-rc5-mm2/broken-out/handle-ext3-directory-corruption-better.patch

I have been looking at doing a check only when the block is first read,
but other things have come up & taken some time, and that is a bit on
the back burner now...

-Eric

Re: [PATCH] ext3: htree entry integrity checking

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andreas Dilger wrote:
> On Nov 16, 2006  11:50 -0500, Jeff Mahoney wrote:
>>  Currently, if a corrupted directory entry with rec_len=0 is encountered,
>>  we still trust that the data is valid. This can cause an infinite loop
>>  in htree_dirblock_to_tree() since the iteration loop will never make any
>>  progress.
> 
> Actually, I think Eric Sandeen was working on similar fixes already, and
> instead of doing a per-item check each time we look at the entry it does
> a full-block check the first time it is read (as ext2 does).
> 
>>  This fixes the problem described at:
>>  http://projects.info-pull.com/mokb/MOKB-10-11-2006.html
> 
> Would also be good to CC linux-ext4, where the ext3 maintainers live.

Ok, thanks. If that's already in -mm, I'll use that one.

- -Jeff

- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

iD8DBQFFXQIwLPWxlyuTD7IRApH7AJ9+/SFmd9bf8E741wvxw/6vdrUrdwCeJNEG
eHZMo5RWUrLW5iDEqehjRlI=
=lGRM
-----END PGP SIGNATURE-----