2006-12-21 18:58:35

by Eric Sandeen

Subject: [PATCH] ext2: skip pages past number of blocks in ext2_find_entry

This one was pointed out on the MOKB site:
http://kernelfun.blogspot.com/2006/11/mokb-09-11-2006-linux-26x-ext2checkpage.html

If a directory's i_size is corrupted, ext2_find_entry() will keep processing
pages until the i_size is reached, even if there are no more blocks associated
with the directory inode. This patch puts in some minimal sanity-checking
so that we don't keep checking pages (and issuing errors) if we know there
can be no more data to read, based on the block count of the directory inode.

This is somewhat similar in approach to the ext3 patch I sent earlier this
year.

Thanks,

-Eric

Signed-off-by: Eric Sandeen <[email protected]>

Index: linux-2.6.19/fs/ext2/dir.c
===================================================================
--- linux-2.6.19.orig/fs/ext2/dir.c
+++ linux-2.6.19/fs/ext2/dir.c
@@ -368,6 +368,14 @@ struct ext2_dir_entry_2 * ext2_find_entr
}
if (++n >= npages)
n = 0;
+ /* next page is past the blocks we've got */
+ if (unlikely(n > (dir->i_blocks >> (PAGE_CACHE_SHIFT - 9)))) {
+ ext2_error(dir->i_sb, __FUNCTION__,
+ "dir %lu size %lld exceeds block count %llu",
+ dir->i_ino, dir->i_size,
+ (unsigned long long)dir->i_blocks);
+ goto out;
+ }
} while (n != start);
out:
return NULL;

2006-12-21 19:05:49

by Randy Dunlap

Subject: Re: [PATCH] ext2: skip pages past number of blocks in ext2_find_entry

On Thu, 21 Dec 2006 12:58:28 -0600 Eric Sandeen wrote:

> This one was pointed out on the MOKB site:
> http://kernelfun.blogspot.com/2006/11/mokb-09-11-2006-linux-26x-ext2checkpage.html
>
> If a directory's i_size is corrupted, ext2_find_entry() will keep processing
> pages until the i_size is reached, even if there are no more blocks associated
> with the directory inode. This patch puts in some minimal sanity-checking
> so that we don't keep checking pages (and issuing errors) if we know there
> can be no more data to read, based on the block count of the directory inode.
>
> This is somewhat similar in approach to the ext3 patch I sent earlier this
> year.
>
> Thanks,
>
> -Eric
>
> Signed-off-by: Eric Sandeen <[email protected]>
>
> Index: linux-2.6.19/fs/ext2/dir.c
> ===================================================================
> --- linux-2.6.19.orig/fs/ext2/dir.c
> +++ linux-2.6.19/fs/ext2/dir.c
> @@ -368,6 +368,14 @@ struct ext2_dir_entry_2 * ext2_find_entr
> }
> if (++n >= npages)
> n = 0;
> + /* next page is past the blocks we've got */
> + if (unlikely(n > (dir->i_blocks >> (PAGE_CACHE_SHIFT - 9)))) {
> + ext2_error(dir->i_sb, __FUNCTION__,
> + "dir %lu size %lld exceeds block count %llu",
> + dir->i_ino, dir->i_size,
> + (unsigned long long)dir->i_blocks);
> + goto out;

Please don't hide the goto; un-indent 1 tab stop.

> + }
> } while (n != start);
> out:
> return NULL;


---
~Randy

2006-12-21 19:11:15

by Eric Sandeen

Subject: Re: [PATCH] ext2: skip pages past number of blocks in ext2_find_entry

Randy Dunlap wrote:

> Please don't hide the goto; un-indent 1 tab stop.
Whoops, thanks Randy - it wasn't intentional. :)

Signed-off-by: Eric Sandeen <[email protected]>

Index: linux-2.6.19/fs/ext2/dir.c
===================================================================
--- linux-2.6.19.orig/fs/ext2/dir.c
+++ linux-2.6.19/fs/ext2/dir.c
@@ -368,6 +368,14 @@ struct ext2_dir_entry_2 * ext2_find_entr
}
if (++n >= npages)
n = 0;
+ /* next page is past the blocks we've got */
+ if (unlikely(n > (dir->i_blocks >> (PAGE_CACHE_SHIFT - 9)))) {
+ ext2_error(dir->i_sb, __FUNCTION__,
+ "dir %lu size %lld exceeds block count %llu",
+ dir->i_ino, dir->i_size,
+ (unsigned long long)dir->i_blocks);
+ goto out;
+ }
} while (n != start);
out:
return NULL;

2006-12-21 19:14:45

by Randy Dunlap

Subject: Re: [PATCH] ext2: skip pages past number of blocks in ext2_find_entry

On Thu, 21 Dec 2006 13:11:01 -0600 Eric Sandeen wrote:

> Randy Dunlap wrote:
>
> > Please don't hide the goto; un-indent 1 tab stop.
> Whoops, thanks Randy - it wasn't intentional. :)

Yeah, I didn't think it was. Sorry if it sounded like that.

---
~Randy