2008-12-17 08:59:50

by Miao Xie

Subject: [PATCH] [e2fsprogs] e2fsck: fix segmentation fault when block size is greater than 8192

When I did fsck a filesystem with large blocksize(greater than 8192),
segmentation fault occured. The cause is the size of b_data array that is
defined as a fixed size in buffer_head structure.
(File: e2fsck/jfs_user.h)
struct buffer_head {
char b_data[8192];
e2fsck_t b_ctx;
io_channel b_io;
int b_size;
blk_t b_blocknr;
int b_dirty;
int b_uptodate;
int b_err;
};

It is unreasonable, because if the blocksize is greater than 8192, b_data will
overflow and the other variable would be changed, then if we touch those
variable, segmentation fault occurs.

This patch fixes this bug.

Signed-off-by: Miao Xie <[email protected]>

---
e2fsck/jfs_user.h | 2 +-
e2fsck/journal.c | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/e2fsck/jfs_user.h b/e2fsck/jfs_user.h
index 0e4f951..f042218 100644
--- a/e2fsck/jfs_user.h
+++ b/e2fsck/jfs_user.h
@@ -15,7 +15,7 @@
#include "e2fsck.h"

struct buffer_head {
- char b_data[8192];
+ char * b_data;
e2fsck_t b_ctx;
io_channel b_io;
int b_size;
diff --git a/e2fsck/journal.c b/e2fsck/journal.c
index 10f5095..ca7a4c3 100644
--- a/e2fsck/journal.c
+++ b/e2fsck/journal.c
@@ -73,6 +73,12 @@ struct buffer_head *getblk(kdev_t kdev, blk_t blocknr, int blocksize)
if (!bh)
return NULL;

+ bh->b_data = e2fsck_allocate_memory(kdev->k_ctx, blocksize,
+ "block buffer b_data");
+ if (!bh->b_data) {
+ ext2fs_free_mem(&bh);
+ return NULL;
+ }
#ifdef CONFIG_JBD_DEBUG
if (journal_enable_debug >= 3)
bh_count++;
@@ -163,6 +169,8 @@ void brelse(struct buffer_head *bh)
ll_rw_block(WRITE, 1, &bh);
jfs_debug(3, "freeing block %lu/%p (total %d)\n",
(unsigned long) bh->b_blocknr, (void *) bh, --bh_count);
+ if (bh->b_data)
+ ext2fs_free_mem(&bh->b_data);
ext2fs_free_mem(&bh);
}

--
1.5.4.rc3

2009-01-02 23:30:54

by Theodore Ts'o

Subject: Re: [PATCH] [e2fsprogs] e2fsck: fix segmentation fault when block size is greater than 8192

On Wed, Dec 17, 2008 at 04:58:12PM +0800, Miao Xie wrote:
> When I did fsck a filesystem with large blocksize(greater than 8192),
> segmentation fault occured. The cause is the size of b_data array that is
> defined as a fixed size in buffer_head structure.

Here's a better patch which avoids the need for two calls to malloc
for each buffer_head strcture.

- Ted

commit e35d548b59e24af9f7fc18396e8880df96d1bc51
Author: Theodore Ts'o <[email protected]>
Date: Fri Jan 2 18:14:42 2009 -0500

e2fsck: Fix journal replay for block sizes greater than 8k

E2fsck was using a fixed-size 8k buffer for replaying blocks from the
journal. So attempts to replay a journal on filesystems greater than
8k would cause e2fsck to crash with a segfault.

Thanks to Miao Xie <[email protected]> for reporting this problem.

Signed-off-by: "Theodore Ts'o" <[email protected]>

diff --git a/e2fsck/jfs_user.h b/e2fsck/jfs_user.h
index 0e4f951..60cc682 100644
--- a/e2fsck/jfs_user.h
+++ b/e2fsck/jfs_user.h
@@ -15,7 +15,6 @@
#include "e2fsck.h"

struct buffer_head {
- char b_data[8192];
e2fsck_t b_ctx;
io_channel b_io;
int b_size;
@@ -23,6 +22,7 @@ struct buffer_head {
int b_dirty;
int b_uptodate;
int b_err;
+ char b_data[1024];
};

struct inode {
diff --git a/e2fsck/journal.c b/e2fsck/journal.c
index 10f5095..adbd3db 100644
--- a/e2fsck/journal.c
+++ b/e2fsck/journal.c
@@ -68,8 +68,10 @@ int journal_bmap(journal_t *journal, blk_t block, unsigned long *phys)
struct buffer_head *getblk(kdev_t kdev, blk_t blocknr, int blocksize)
{
struct buffer_head *bh;
+ int bufsize = sizeof(*bh) + kdev->k_ctx->fs->blocksize -
+ sizeof(bh->b_data);

- bh = e2fsck_allocate_memory(kdev->k_ctx, sizeof(*bh), "block buffer");
+ bh = e2fsck_allocate_memory(kdev->k_ctx, bufsize, "block buffer");
if (!bh)
return NULL;