directory offset overflows in 2.6.28

The patch below is a dumb version of just putting back the masking, to make sure we have the same behavior as in 2.6.27 and earlier. I think we should at least hide it in a macro that is well-commented, but I suspect we also need to make sure that we never ever get bigger offsets in directories in some way.

Index: xfs/fs/xfs/xfs_dir2_block.c
===================================================================

fs/xfs/xfs_dir2_block.c +0 -0

@@ -517,9 +517,9 @@ xfs_dir2_block_getdents(
 		/*

Quoted 6 lines · Christoph Hellwig

The patch below is a dumb version of just putting back the masking, to make sure we have the same behavior as in 2.6.27 and earlier. I think we should at least hide it in a macro that is well-commented, but I suspect we also need to make sure that we never ever get bigger offsets in directories in some way.

I think we need that macro sooner rather than later ;)

Quoted 7 lines

Index: xfs/fs/xfs/xfs_dir2_block.c
===================================================================

fs/xfs/xfs_dir2_block.c +0 -0

@@ -517,9 +517,9 @@ xfs_dir2_block_getdents(
 		/*

fs/xfs/xfs_dir2_block.c xfs_dir2_block_getdents( · line 525

Quoted 10 lines

fs/xfs/xfs_dir2_block.c +2 -2

 		 * If it didn't fit, set the final offset to here & return.
 		 */
-		if (filldir(dirent, dep->name, dep->namelen, cook,
+		if (filldir(dirent, dep->name, dep->namelen, cook & 0x7fffffff,
 			    ino, DT_UNKNOWN)) {
-			*offset = cook;
+			*offset = cook & 0x7fffffff;
 			xfs_da_brelse(NULL, bp);
 			return 0;
 		}

In this case, you can do the masking at the time cook is assigned. I haven't checked, but I suspect the rest will be the same. That will make the patch less invasive and with a macro somewhat cleaner...

Cheers,

Dave.

Quoted 5 lines · Christoph Hellwig

The patch below is a dumb version of just putting back the masking, to make sure we have the same behavior as in 2.6.27 and earlier. I think we should at least hide it in a macro that is well-commented, but I suspect we also need to make sure that we never ever get bigger offsets in directories in some way.

I think we need that macro sooner rather than later ;)

In this case, you can do the masking at the time cook is assigned. I haven't checked, but I suspect the rest will be the same. That will make the patch less invasive and with a macro somewhat cleaner...

Quoted 7 lines · Dave Chinner

Quoted 5 lines · Christoph Hellwig

The patch below is a dumb version of just putting back the masking, to make sure we have the same behavior as in 2.6.27 and earlier. I think we should at least hide it in a macro that is well-commented, but I suspect we also need to make sure that we never ever get bigger offsets in directories in some way.

Quoted 6 lines

I think we need that macro sooner rather than later ;)

In this case, you can do the masking at the time cook is assigned. I haven't checked, but I suspect the rest will be the same. That will make the patch less invasive and with a macro somewhat cleaner...

That way we could replace two assignment by one one time each in xfs_dir2_leaf.c and xfs_dir2_block.c.

I started working on the macro, but it seems even more hacky.

When looking at the big picture we have two problems:

- the end of directory marker which always seems to be always too large for 32bit values for 32 bit indices
- directories that actually are too large to be represented using 32 bit signed offsets. I guess we just can't support those for apps using the 32bit readdir interface and the EOVERFLOW is valid for those. The current approach as in < 2.6.28 and with this patch breaks that second case in subtile ways.

So, I'd just push the first hacky opencoded patch into 2.6.29 and -stable now to revert to the old behaviour with all it's faults, and in the meantime I'll look into a proper way finding a better end of directory indicator. That should also help Russell's BSD concerns.