[PATCH] xfs_repair: check for null single-block dir buffer pointer in phase6

[PATCH] xfs_repair: check for null single-block dir buffer pointer in phase6

[Eric Sandeen]

This is for Bug 844 -  xfs_repair from git segfaults in stage 6
on oss.sgi.com's bugzilla:

Phase 6 - check inode connectivity...
        - resetting contents of realtime bitmap and summary inodes
        - traversing filesystem ...
entry "stdio-common" in dir ino 2858345118 doesn't have a .. entry, will set it
in ino 3503084373.
empty data block 0 in directory inode 3503084373: junking block
<segfault>

longform_dir2_entry_check() calls longform_dir2_entry_check_data()
which issues that "junking block" message, and it sets *bpp (which
is bplist[0] passed in) to NULL.

(minor note, I think this leaks a bit of memory).

In this case it's a single-block directory, the dir was found to 
have no valid data, and so it was junked.  So there is no point in
checking the integrity of this block, and in fact trying to do
so is what segfaults, thanks to the NULL-setting above.

So a simple patch like this avoids the segfault.

However, there is still an issue where the problematic directory
is set to link count 1 in Phase 7, and a subsequent repair run
bumps it back up to 2.  But in the spirit of fixing one thing
at a time, here's a patch.

Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---

diff --git a/repair/phase6.c b/repair/phase6.c
index becedbd..101df15 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -2516,6 +2516,7 @@ longform_dir2_entry_check(xfs_mount_t	*mp,
 			*num_illegal += 1;
 			continue;	/* try and read all "data" blocks */
 		}
+		/* Note, this may NULL out bplist[db] if it's junked */
 		longform_dir2_entry_check_data(mp, ip, num_illegal, need_dot,
 				irec, ino_offset, &bplist[db], hashtab,
 				&freetab, da_bno, isblock);
@@ -2524,7 +2525,7 @@ longform_dir2_entry_check(xfs_mount_t	*mp,
 
 	if (!dotdot_update) {
 		/* check btree and freespace */
-		if (isblock) {
+		if (isblock && bplist[0]) {
 			xfs_dir2_block_tail_t	*btp;
 			xfs_dir2_leaf_entry_t	*blp;
 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_repair: check for null single-block dir buffer pointer in phase6

[Christoph Hellwig]

On Fri, Jul 24, 2009 at 09:34:08AM -0500, Eric Sandeen wrote:
> @@ -2524,7 +2525,7 @@ longform_dir2_entry_check(xfs_mount_t	*mp,
>  
>  	if (!dotdot_update) {
>  		/* check btree and freespace */
> -		if (isblock) {
> +		if (isblock && bplist[0]) {
>  			xfs_dir2_block_tail_t	*btp;
>  			xfs_dir2_leaf_entry_t	*blp;

This doesn't look quite correct to me.  Now we falls through to the
final else statement when bplist[0] is zeroed.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_repair: check for null single-block dir buffer pointer in phase6

[Eric Sandeen]

Christoph Hellwig wrote:
> On Fri, Jul 24, 2009 at 09:34:08AM -0500, Eric Sandeen wrote:
>> @@ -2524,7 +2525,7 @@ longform_dir2_entry_check(xfs_mount_t	*mp,
>>  
>>  	if (!dotdot_update) {
>>  		/* check btree and freespace */
>> -		if (isblock) {
>> +		if (isblock && bplist[0]) {
>>  			xfs_dir2_block_tail_t	*btp;
>>  			xfs_dir2_leaf_entry_t	*blp;
> 
> This doesn't look quite correct to me.  Now we falls through to the
> final else statement when bplist[0] is zeroed.
> 

Oh, ugh, you're right.  Will fix that up, thanks.

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs