ext3: ext4: Using uninitialized value

PATCH In review Roman Borisov 6 participants 13 messages Oct 13, 2010 – Oct 19, 2010 ext4 file system

Share & tools

Roman Borisov

author

2010-10-13 14:36 UTC #0 lore raw

Hello,

Could you clarify is there a bug in fs/ext4/namei.c, ext4_dx_find_entry() and fs/ext4/namei.c, ext3_dx_find_entry()?

<code>
static struct buffer_head * ext3_dx_find_entry(struct inode *dir,
...
struct dx_hash_info hinfo;
...
if (namelen > 2 || name[0] != '.'|| (namelen == 2 && name[1] != '.')) { if (!(frame = dx_probe(entry, dir, &hinfo, frames, err))) return NULL;
} else {
frame = frames;
frame->bh = NULL; /* for dx_release() */
frame->at = (struct dx_entry *)frames; /* hack for zero entry*/ dx_set_block(frame->at, 0); /* dx_root block is 0 */
}
hash = hinfo.hash;
...
retval = ext3_htree_next_block(dir, hash, frame,
...
</code>

In the code above: hinfo.hash is not initialized in "else" case. Should it be initialized as NULL?
Or maybe implementation doesn't assume to call ext3_htree_next_block() in such case?

Thanks,
Roman

Eric Sandeen

2010-10-13 16:14 UTC #1 lore raw

Quoted 4 lines · Roman Borisov

Hello,
Could you clarify is there a bug in fs/ext4/namei.c, ext4_dx_find_entry() and fs/ext4/namei.c, ext3_dx_find_entry()?

that was introduced with:

commit acfa1823d33859b0db77701726c9ca5ccc6e6f25

Author: Andreas Dilger <[email protected]>
Date:   Thu Jun 23 00:09:45 2005 -0700

[PATCH] Support for dx directories in ext3_get_parent (NFSD)

so maybe Andreas knows offhand ;) but I think:

Quoted 4 lines

<code>
static struct buffer_head * ext3_dx_find_entry(struct inode *dir,
...
        if (namelen > 2 || name[0] != '.'|| (namelen == 2 && name[1] != '.')) {

This is a fancy way of saying name is not "." or ".."

Quoted 3 lines

                if (!(frame = dx_probe(entry, dir, &hinfo, frames, err)))
                        return NULL;
        } else {

so here it -is- "." or ".." -

Quoted 4 lines

                frame = frames;
                frame->bh = NULL;                       /* for dx_release() */
                frame->at = (struct dx_entry *)frames;  /* hack for zero entry*/
                dx_set_block(frame->at, 0);             /* dx_root block is 0 */

now frame->at.block = 0

Quoted 4 lines

        }
        hash = hinfo.hash;
        do {
                block = dx_get_block(frame->at);

block = 0 (we just put it there)

Quoted 2 lines

                if (!(bh = ext3_bread (NULL,dir, block, 0, err)))
                        goto errout;

so we look up block 0 in the dir inode

Quoted 3 lines

                de = (struct ext3_dir_entry_2 *) bh->b_data;
                top = (struct ext3_dir_entry_2 *) ((char *) de + sb->s_blocksize -
                                       EXT3_DIR_REC_LEN(0));

and get to the dir entry, and search through them for our name

Quoted 11 lines

                for (; de < top; de = ext3_next_entry(de)) {
                        int off = (block << EXT3_BLOCK_SIZE_BITS(sb))
                                  + ((char *) de - bh->b_data);

                        if (!ext3_check_dir_entry(__func__, dir, de, bh, off)) {
                                brelse(bh);
                                *err = ERR_BAD_DX_DIR;
                                goto errout;
                        }

                        if (ext3_match(namelen, name, de)) {

here we should find the . or .. (it's always going to be there, right?)

Quoted 3 lines

                                *res_dir = de;
                                dx_release(frames);
                                return bh;

so we return

Quoted 2 lines

                        }
                }

before we get here:

Quoted 5 lines

...
retval = ext3_htree_next_block(dir, hash, frame,
frames, NULL);
...
</code>

-Eric

Quoted 12 lines

In the code above: hinfo.hash is not initialized in "else" case. Should it be initialized as NULL?
Or maybe implementation doesn't assume to call ext3_htree_next_block() in such case?
Thanks,
Roman
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Andreas Dilger

2010-10-13 18:56 UTC #2 lore raw

Quoted 8 lines · Eric Sandeen

Quoted 4 lines · Roman Borisov
Hello,
Could you clarify is there a bug in fs/ext4/namei.c, ext4_dx_find_entry() and fs/ext4/namei.c, ext3_dx_find_entry()?
that was introduced with:

Quoted 7 lines

commit acfa1823d33859b0db77701726c9ca5ccc6e6f25
Author: Andreas Dilger <[email protected]>
Date:   Thu Jun 23 00:09:45 2005 -0700
[PATCH] Support for dx directories in ext3_get_parent (NFSD)
so maybe Andreas knows offhand ;) but I think:

Your analysis is correct. I agree it's a bit convoluted, but it avoids replicating a bunch of code.

Quoted 14 lines

Quoted 4 lines

static struct buffer_head * ext3_dx_find_entry(struct inode *dir,
...
      if (namelen > 2 || name[0] != '.'|| (namelen == 2 && name[1] != '.'))
       } else {

so here it -is- "." or ".." -

Quoted 6 lines

              frame = frames;
              frame->bh = NULL;                       /* for dx_release() */
              frame->at = (struct dx_entry *)frames;  /* hack for zero entry*/
              dx_set_block(frame->at, 0);             /* dx_root block is 0 */
              if (!(bh = ext3_bread (NULL,dir, block, 0, err)))
                      goto errout;

Quoted 5 lines

so we look up block 0 in the dir inode
Quoted 1 line
                      if (ext3_match(namelen, name, de)) {
here we should find the . or .. (it's always going to be there, right?)

Right - it is important to note that the index root block is a "fake" directory block which has just the "." and ".." entries at the beginning (with the ".." spanning the rest of the block), and the rest of the block is holding the index entries. For a directory index to even exist, it HAS to have the "." and ".." entries in the first block, or there is no place to put the index.

Cheers, Andreas

Roman Borisov

author

2010-10-14 10:06 UTC #3 lore raw

Quoted 11 lines · ext Andreas Dilger

Quoted 9 lines
that was introduced with:
Quoted 8 lines
commit acfa1823d33859b0db77701726c9ca5ccc6e6f25
Author: Andreas Dilger<[email protected]>
Date: Thu Jun 23 00:09:45 2005 -0700
[PATCH] Support for dx directories in ext3_get_parent (NFSD)
so maybe Andreas knows offhand;) but I think:
Your analysis is correct. I agree it's a bit convoluted, but it avoids replicating a bunch of code.

Thanks a lot! it make sence.

But I just wondering why hash = hinfo->hash is located in separate scope where it looks like unitialized. The same situation in namei.c/dx_probe():

	if (entry)
		ext3fs_dirhash(entry->name, entry->len, hinfo);
	hash = hinfo->hash;
I believe that the implementation doesn't allow to use hash value when !entry but why it wasn't designed like:
	if (entry)
	{
		ext3fs_dirhash(entry->name, entry->len, hinfo);
		hash = hinfo->hash;
	}

for example?

Roman

Eric Sandeen

2010-10-14 13:42 UTC #4 lore raw

Quoted 29 lines · Roman Borisov

Quoted 6 lines · ext Andreas Dilger
Quoted 6 lines
that was introduced with:
Quoted 5 lines
commit acfa1823d33859b0db77701726c9ca5ccc6e6f25
Author: Andreas Dilger<[email protected]>
Date: Thu Jun 23 00:09:45 2005 -0700
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
[PATCH] Support for dx directories in ext3_get_parent (NFSD)
so maybe Andreas knows offhand;) but I think:
Your analysis is correct. I agree it's a bit convoluted, but it avoids replicating a bunch of code.
Thanks a lot! it make sence.
But I just wondering why hash = hinfo->hash is located in separate scope where it looks like unitialized.
The same situation in namei.c/dx_probe():
    if (entry)
        ext3fs_dirhash(entry->name, entry->len, hinfo);
    hash = hinfo->hash;
I believe that the implementation doesn't allow to use hash value when
!entry but why it wasn't designed like:
    if (entry)
    {
        ext3fs_dirhash(entry->name, entry->len, hinfo);
        hash = hinfo->hash;
    }

Quoted 1 line

for example?

Just a guess, but gcc might start complaining then ;) (It wasn't smart enough to see the potential problem with the other way...)

-Eric

Quoted 1 line

Roman

Ted Ts'o

maintainer

2010-10-16 23:35 UTC #5 lore raw

In reply to Eric Sandeen

Quoted 5 lines · Eric Sandeen

Quoted 4 lines · Roman Borisov
Hello,
Could you clarify is there a bug in fs/ext4/namei.c, ext4_dx_find_entry() and fs/ext4/namei.c, ext3_dx_find_entry()?

Let me guess, you compiled the kernel using Clang, right? I recently had a similar discussion with Brad about this issue.

Quoted 1 line

here we should find the . or .. (it's always going to be there, right?)

The problem is if '.' or '..' is not present, then when we call ext3_htree_next_block(), it's not the fact that hash is uninitialized which is the problem. The bigger problem is that the frame structure is not fully initialized; and we could end up dereference a NULL pointer (on a 32-bit system) or a pointer filled with stack garbage (on a 64-bit system).

Fortunately most of the time we'll never hit this case, since '.' and '..' handling is dealt with in fs/namei.c, and is never passed down to the fs-specific layer. The one exception to this is if the file system is being used as an NFS file server, in which case it is possible that the fs layer will be asked to look up "..".

So in the case where the directory is corrupted, and the file system is being exported via NFS, it's possible that we could get a kernel Oops. Since we're only reading from the garbaged pointer, I'm pretty sure this can't be leveraged into a security exposure, but still, it would be good to fix this.

I have patches chained to this e-mail that should be applied for ext3, and which I've queued for ext4.

- Ted

Theodore Ts'o

maintainer

2010-10-16 23:37 UTC #6 lore raw

PATCH 1/2 ext3: Avoid uninitialized memory references with a corrupted htree directory

If the first htree directory is missing '.' or '..' but is otherwise a valid directory, and we do a lookup for '.' or '..', it's possible to dereference an uninitialized memory pointer in ext3_htree_next_block(). Avoid this.

We avoid this by moving the special case from ext3_dx_find_entry() to ext3_find_entry(); this also means we can optimize ext3_find_entry() slightly when NFS looks up "..".

Thanks to Brad Spengler for pointing a Clang warning that led me to look more closely at this code. The warning was harmless, but it was useful in pointing out code that was too ugly to live. This warning was also reported by Roman Borisov.

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

Cc Jan Kara <[email protected]>

Cc Brad Spengler <[email protected]>

---
 fs/ext3/namei.c |   30 +++++++++++++++---------------
 1 files changed, 15 insertions(+), 15 deletions(-)

fs/ext3/namei.c +15 ~~-15~~

index 2b35ddb..01f12cc 100644
@@ -858,6 +858,7 @@ static struct buffer_head *ext3_find_entry(struct inode *dir,
 	struct buffer_head * bh_use[NAMEI_RA_SIZE];
 	struct buffer_head * bh, *ret = NULL;
 	unsigned long start, block, b;
+	const u8 *name = entry->name;
 	int ra_max = 0;		/* Number of bh's in the readahead
 				   buffer, bh_use[] */
 	int ra_ptr = 0;		/* Current index into readahead

@@ -871,6 +872,16 @@ static struct buffer_head *ext3_find_entry(struct inode *dir,
 	namelen = entry->len;
 	if (namelen > EXT3_NAME_LEN)
 		return NULL;
+	if ((namelen < 2) && (name[0] == '.') &&
+	    (name[1] == '.' || name[1] == '0')) {
+		/*
+		 * "." or ".." will only be in the first block
+		 * NFS may look up ".."; "." should be handled by the VFS
+		 */
+		block = start = 0;
+		nblocks = 1;
+		goto restart;
+	}
 	if (is_dx(dir)) {
 		bh = ext3_dx_find_entry(dir, entry, res_dir, &err);
 		/*

@@ -961,9 +972,8 @@ static struct buffer_head * ext3_dx_find_entry(struct inode *dir,
 			struct qstr *entry, struct ext3_dir_entry_2 **res_dir,
 			int *err)
 {
-	struct super_block * sb;
+	struct super_block *sb = dir->i_sb;
 	struct dx_hash_info	hinfo;
-	u32 hash;
 	struct dx_frame frames[2], *frame;
 	struct ext3_dir_entry_2 *de, *top;
 	struct buffer_head *bh;

@@ -972,18 +982,8 @@ static struct buffer_head * ext3_dx_find_entry(struct inode *dir,
 	int namelen = entry->len;
 	const u8 *name = entry->name;
 
-	sb = dir->i_sb;
-	/* NFS may look up ".." - look at dx_root directory block */
-	if (namelen > 2 || name[0] != '.'|| (namelen == 2 && name[1] != '.')) {
-		if (!(frame = dx_probe(entry, dir, &hinfo, frames, err)))
-			return NULL;
-	} else {
-		frame = frames;
-		frame->bh = NULL;			/* for dx_release() */
-		frame->at = (struct dx_entry *)frames;	/* hack for zero entry*/
-		dx_set_block(frame->at, 0);		/* dx_root block is 0 */
-	}
-	hash = hinfo.hash;
+	if (!(frame = dx_probe(entry, dir, &hinfo, frames, err)))
+		return NULL;
 	do {
 		block = dx_get_block(frame->at);
 		if (!(bh = ext3_bread (NULL,dir, block, 0, err)))

@@ -1009,7 +1009,7 @@ static struct buffer_head * ext3_dx_find_entry(struct inode *dir,
 		}
 		brelse (bh);
 		/* Check to see if we should continue to search */
-		retval = ext3_htree_next_block(dir, hash, frame,
+		retval = ext3_htree_next_block(dir, hinfo.hash, frame,
 					       frames, NULL);
 		if (retval < 0) {
 			ext3_warning(sb, __func__,

Signature · 1 line

1.7.1

Jan Kara

reviewer

2010-10-18 10:06 UTC #7 lore raw

Re: [PATCH 1/2] ext3: Avoid uninitialized memory references with a corrupted htree directory

Hi Ted,

thanks for the patch.

fs/ext3/namei.c static struct buffer_head *ext3_find_entry(struct inode *dir, · line 876

Quoted 6 lines · Theodore Ts'o

fs/ext3/namei.c +2 -0

@@ -871,6 +872,16 @@ static struct buffer_head *ext3_find_entry(struct inode *dir,
 	namelen = entry->len;
 	if (namelen > EXT3_NAME_LEN)
 		return NULL;
+	if ((namelen < 2) && (name[0] == '.') &&
+	    (name[1] == '.' || name[1] == '0')) {

This condition looks wrong... I suspect it should rather be:
(namelen <= 2) && (name[0] == '.') && (name[1] == '.' || name[1] == 0)
^^^ change here and here ^^^

fs/ext3/namei.c static struct buffer_head *ext3_find_entry(struct inode *dir, · line 884

Quoted 8 lines

fs/ext3/namei.c +8 -0

+		/*
+		 * "." or ".." will only be in the first block
+		 * NFS may look up ".."; "." should be handled by the VFS
+		 */
+		block = start = 0;
+		nblocks = 1;
+		goto restart;
+	}

Honza

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

Andreas Dilger

reviewer

2010-10-19 07:12 UTC #8 lore raw

Re: [PATCH 1/2] ext3: Avoid uninitialized memory references with a corrupted htree directory

Quoted 11 lines · Jan Kara

fs/ext3/namei.c static struct buffer_head *ext3_find_entry(struct inode *dir, · line 876
Quoted 6 lines · Theodore Ts'o
fs/ext3/namei.c +2 -0
@@ -871,6 +872,16 @@ static struct buffer_head *ext3_find_entry(struct inode *dir,
 	namelen = entry->len;
 	if (namelen > EXT3_NAME_LEN)
 		return NULL;
+	if ((namelen < 2) && (name[0] == '.') &&
+	    (name[1] == '.' || name[1] == '0')) {
This condition looks wrong... I suspect it should rather be:
(namelen <= 2) && (name[0] == '.') && (name[1] == '.' || name[1] == 0)
^^^ change here and here ^^^

I think it is preferable to use '\0' for the trailing NUL.

Cheers, Andreas

Theodore Ts'o

maintainer

2010-10-16 23:37 UTC #9 lore raw

PATCH 1/2 ext4: Avoid uninitialized memory references in ext3_htree_next_block()

In reply to Ted Ts'o

If the first block of htree directory is missing '.' or '..' but is otherwise a valid directory, and we do a lookup for '.' or '..', it's possible to dereference an uninitialized memory pointer in ext4_htree_next_block().

We avoid this by moving the special case from ext4_dx_find_entry() to ext4_find_entry(); this also means we can optimize ext4_find_entry() slightly when NFS looks up "..".

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

Cc Brad Spengler <[email protected]>

---
 fs/ext4/namei.c |   30 +++++++++++++++---------------
 1 files changed, 15 insertions(+), 15 deletions(-)

fs/ext4/namei.c +15 ~~-15~~

index 314c0d3..ff89449 100644
@@ -856,6 +856,7 @@ static struct buffer_head * ext4_find_entry (struct inode *dir,
 	struct buffer_head *bh_use[NAMEI_RA_SIZE];
 	struct buffer_head *bh, *ret = NULL;
 	ext4_lblk_t start, block, b;
+	const u8 *name = d_name->name;
 	int ra_max = 0;		/* Number of bh's in the readahead
 				   buffer, bh_use[] */
 	int ra_ptr = 0;		/* Current index into readahead

@@ -870,6 +871,16 @@ static struct buffer_head * ext4_find_entry (struct inode *dir,
 	namelen = d_name->len;
 	if (namelen > EXT4_NAME_LEN)
 		return NULL;
+	if ((namelen < 2) && (name[0] == '.') &&
+	    (name[1] == '.' || name[1] == '0')) {
+		/*
+		 * "." or ".." will only be in the first block
+		 * NFS may look up ".."; "." should be handled by the VFS
+		 */
+		block = start = 0;
+		nblocks = 1;
+		goto restart;
+	}
 	if (is_dx(dir)) {
 		bh = ext4_dx_find_entry(dir, d_name, res_dir, &err);
 		/*

@@ -960,9 +971,8 @@ cleanup_and_exit:
 static struct buffer_head * ext4_dx_find_entry(struct inode *dir, const struct qstr *d_name,
 		       struct ext4_dir_entry_2 **res_dir, int *err)
 {
-	struct super_block * sb;
+	struct super_block * sb = dir->i_sb;
 	struct dx_hash_info	hinfo;
-	u32 hash;
 	struct dx_frame frames[2], *frame;
 	struct ext4_dir_entry_2 *de, *top;
 	struct buffer_head *bh;

@@ -971,18 +981,8 @@ static struct buffer_head * ext4_dx_find_entry(struct inode *dir, const struct q
 	int namelen = d_name->len;
 	const u8 *name = d_name->name;
 
-	sb = dir->i_sb;
-	/* NFS may look up ".." - look at dx_root directory block */
-	if (namelen > 2 || name[0] != '.'||(name[1] != '.' && name[1] != '\0')){
-		if (!(frame = dx_probe(d_name, dir, &hinfo, frames, err)))
-			return NULL;
-	} else {
-		frame = frames;
-		frame->bh = NULL;			/* for dx_release() */
-		frame->at = (struct dx_entry *)frames;	/* hack for zero entry*/
-		dx_set_block(frame->at, 0);		/* dx_root block is 0 */
-	}
-	hash = hinfo.hash;
+	if (!(frame = dx_probe(d_name, dir, &hinfo, frames, err)))
+		return NULL;
 	do {
 		block = dx_get_block(frame->at);
 		if (!(bh = ext4_bread (NULL,dir, block, 0, err)))

@@ -1008,7 +1008,7 @@ static struct buffer_head * ext4_dx_find_entry(struct inode *dir, const struct q
 		}
 		brelse(bh);
 		/* Check to see if we should continue to search */
-		retval = ext4_htree_next_block(dir, hash, frame,
+		retval = ext4_htree_next_block(dir, hinfo.hash, frame,
 					       frames, NULL);
 		if (retval < 0) {
 			ext4_warning(sb,

Signature · 1 line

1.7.1

Andreas Dilger

2010-10-18 04:27 UTC #10 lore raw

Re: [PATCH 1/2] ext4: Avoid uninitialized memory references in ext3_htree_next_block()

fs/ext4/namei.c static struct buffer_head * ext4_find_entry (struct · line 872

Quoted 3 lines · Theodore Ts'o

fs/ext4/namei.c +2 -0

@@ -870,6 +871,16 @@ static struct buffer_head * ext4_find_entry (struct 
+	if ((namelen < 2) && (name[0] == '.') &&
+	    (name[1] == '.' || name[1] == '0')) {

Shouldn't this be "namelen <= 2"? Otherwise it won't actually match "..".

Cheers, Andreas

Theodore Ts'o

maintainer

2010-10-16 23:37 UTC #11 lore raw

PATCH 2/2 ext3: Use search_dirblock() in ext3_dx_find_entry()

In reply to Ted Ts'o

Use the search_dirblock() in ext3_dx_find_entry(). It makes the code easier to read, and it takes advantage of common code. It also saves 100 bytes or so of text space.

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

Cc Jan Kara <[email protected]>

Cc Brad Spengler <[email protected]>

---
 fs/ext3/namei.c |   33 ++++++++++++---------------------
 1 files changed, 12 insertions(+), 21 deletions(-)

fs/ext3/namei.c +12 ~~-21~~

index 01f12cc..24e15dd 100644
@@ -975,12 +975,9 @@ static struct buffer_head * ext3_dx_find_entry(struct inode *dir,
 	struct super_block *sb = dir->i_sb;
 	struct dx_hash_info	hinfo;
 	struct dx_frame frames[2], *frame;
-	struct ext3_dir_entry_2 *de, *top;
 	struct buffer_head *bh;
 	unsigned long block;
 	int retval;
-	int namelen = entry->len;
-	const u8 *name = entry->name;
 
 	if (!(frame = dx_probe(entry, dir, &hinfo, frames, err)))
 		return NULL;

@@ -988,26 +985,20 @@ static struct buffer_head * ext3_dx_find_entry(struct inode *dir,
 		block = dx_get_block(frame->at);
 		if (!(bh = ext3_bread (NULL,dir, block, 0, err)))
 			goto errout;
-		de = (struct ext3_dir_entry_2 *) bh->b_data;
-		top = (struct ext3_dir_entry_2 *) ((char *) de + sb->s_blocksize -
-				       EXT3_DIR_REC_LEN(0));
-		for (; de < top; de = ext3_next_entry(de)) {
-			int off = (block << EXT3_BLOCK_SIZE_BITS(sb))
-				  + ((char *) de - bh->b_data);
-
-			if (!ext3_check_dir_entry(__func__, dir, de, bh, off)) {
-				brelse(bh);
-				*err = ERR_BAD_DX_DIR;
-				goto errout;
-			}
 
-			if (ext3_match(namelen, name, de)) {
-				*res_dir = de;
-				dx_release(frames);
-				return bh;
-			}
+		retval = search_dirblock(bh, dir, entry,
+					 block << EXT3_BLOCK_SIZE_BITS(sb),
+					 res_dir);
+		if (retval == 1) {
+			dx_release(frames);
+			return bh;
 		}
-		brelse (bh);
+		brelse(bh);
+		if (retval == -1) {
+			*err = ERR_BAD_DX_DIR;
+			goto errout;
+		}
+			
 		/* Check to see if we should continue to search */
 		retval = ext3_htree_next_block(dir, hinfo.hash, frame,
 					       frames, NULL);

Signature · 1 line

1.7.1

Theodore Ts'o

maintainer

2010-10-16 23:37 UTC #12 lore raw

PATCH 2/2 ext4: Use search_dirblock() in ext4_dx_find_entry()

In reply to Ted Ts'o

Use the search_dirblock() in ext4_dx_find_entry(). It makes the code easier to read, and it takes advantage of common code. It also saves 100 bytes or so of text space.

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

Cc Brad Spengler <[email protected]>

---
 fs/ext4/namei.c |   33 ++++++++++++---------------------
 1 files changed, 12 insertions(+), 21 deletions(-)

fs/ext4/namei.c +12 ~~-21~~

index ff89449..eb8b794 100644
@@ -974,39 +974,30 @@ static struct buffer_head * ext4_dx_find_entry(struct inode *dir, const struct q
 	struct super_block * sb = dir->i_sb;
 	struct dx_hash_info	hinfo;
 	struct dx_frame frames[2], *frame;
-	struct ext4_dir_entry_2 *de, *top;
 	struct buffer_head *bh;
 	ext4_lblk_t block;
 	int retval;
-	int namelen = d_name->len;
-	const u8 *name = d_name->name;
 
 	if (!(frame = dx_probe(d_name, dir, &hinfo, frames, err)))
 		return NULL;
 	do {
 		block = dx_get_block(frame->at);
-		if (!(bh = ext4_bread (NULL,dir, block, 0, err)))
+		if (!(bh = ext4_bread(NULL, dir, block, 0, err)))
 			goto errout;
-		de = (struct ext4_dir_entry_2 *) bh->b_data;
-		top = (struct ext4_dir_entry_2 *) ((char *) de + sb->s_blocksize -
-				       EXT4_DIR_REC_LEN(0));
-		for (; de < top; de = ext4_next_entry(de, sb->s_blocksize)) {
-			int off = (block << EXT4_BLOCK_SIZE_BITS(sb))
-				  + ((char *) de - bh->b_data);
-
-			if (!ext4_check_dir_entry(dir, de, bh, off)) {
-				brelse(bh);
-				*err = ERR_BAD_DX_DIR;
-				goto errout;
-			}
 
-			if (ext4_match(namelen, name, de)) {
-				*res_dir = de;
-				dx_release(frames);
-				return bh;
-			}
+		retval = search_dirblock(bh, dir, d_name,
+					 block << EXT4_BLOCK_SIZE_BITS(sb),
+					 res_dir);
+		if (retval == 1) { 	/* Success! */
+			dx_release(frames);
+			return bh;
 		}
 		brelse(bh);
+		if (retval == -1) {
+			*err = ERR_BAD_DX_DIR;
+			goto errout;
+		}
+
 		/* Check to see if we should continue to search */
 		retval = ext4_htree_next_block(dir, hinfo.hash, frame,
 					       frames, NULL);

Signature · 1 line

1.7.1