[Cluster-devel] [PATCH] fsck.gfs2: Fix block size validation

[Andrew Price]

The previous patch had an off-by-one error when looping over
sdp->sd_heightsize. This fixes that and adds an explicit sanity check
for sb_bsize. Some new tests are added to cover both valid and invalid
block size cases.

Signed-off-by: Andrew Price <anprice at redhat.com>
---
 gfs2/libgfs2/super.c |  5 ++++-
 tests/fsck.at        | 10 ++++++----
 tests/mkfs.at        |  7 +++++++
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/gfs2/libgfs2/super.c b/gfs2/libgfs2/super.c
index 8ffd144..d074236 100644
--- a/gfs2/libgfs2/super.c
+++ b/gfs2/libgfs2/super.c
@@ -67,6 +67,9 @@ int read_sb(struct gfs2_sbd *sdp)
 		sdp->gfs1 = 1;
 	sdp->sd_fsb2bb_shift = sdp->sd_sb.sb_bsize_shift - GFS2_BASIC_BLOCK_SHIFT;
 	sdp->bsize = sdp->sd_sb.sb_bsize;
+	if (sdp->bsize < 512 || sdp->bsize != (sdp->bsize & -sdp->bsize)) {
+		return -1;
+	}
 	if (sdp->gfs1) {
 		sdp->sd_diptrs = (sdp->sd_sb.sb_bsize -
 				  sizeof(struct gfs_dinode)) /
@@ -88,7 +91,7 @@ int read_sb(struct gfs2_sbd *sdp)
 	sdp->sd_hash_ptrs = sdp->sd_hash_bsize / sizeof(uint64_t);
 	sdp->sd_heightsize[0] = sdp->sd_sb.sb_bsize - sizeof(struct gfs2_dinode);
 	sdp->sd_heightsize[1] = sdp->sd_sb.sb_bsize * sdp->sd_diptrs;
-	for (x = 2; x <= GFS2_MAX_META_HEIGHT; x++){
+	for (x = 2; x < GFS2_MAX_META_HEIGHT; x++){
 		space = sdp->sd_heightsize[x - 1] * sdp->sd_inptrs;
 		/* FIXME: Do we really need this first check?? */
 		if (space / sdp->sd_inptrs != sdp->sd_heightsize[x - 1] ||
diff --git a/tests/fsck.at b/tests/fsck.at
index 34c5bd5..d7a8357 100644
--- a/tests/fsck.at
+++ b/tests/fsck.at
@@ -1,8 +1,10 @@
 AT_TESTED([fsck.gfs2])
 AT_BANNER([fsck.gfs2 tests])
 
-AT_SETUP([Zeroed block size])
-GFS_LANG_CHECK(
-	[mkfs.gfs2 -O -p lock_nolock $GFS_TGT],
-	[set sb { sb_bsize: 0 }])
+AT_SETUP([Fix invalid block sizes])
+GFS_LANG_CHECK([mkfs.gfs2 -O -p lock_nolock $GFS_TGT], [set sb { sb_bsize: 0 }])
+GFS_LANG_CHECK([mkfs.gfs2 -O -p lock_nolock $GFS_TGT], [set sb { sb_bsize: 1 }])
+GFS_LANG_CHECK([mkfs.gfs2 -O -p lock_nolock $GFS_TGT], [set sb { sb_bsize: 513 }])
+GFS_LANG_CHECK([mkfs.gfs2 -O -p lock_nolock $GFS_TGT], [set sb { sb_bsize: 4095 }])
+GFS_LANG_CHECK([mkfs.gfs2 -O -p lock_nolock $GFS_TGT], [set sb { sb_bsize: 4097 }])
 AT_CLEANUP
diff --git a/tests/mkfs.at b/tests/mkfs.at
index aff6a0d..a3973af 100644
--- a/tests/mkfs.at
+++ b/tests/mkfs.at
@@ -31,6 +31,13 @@ GFS_FSCK_CHECK([mkfs.gfs2 -O -p lock_nolock $GFS_TGT])
 GFS_FSCK_CHECK([mkfs.gfs2 -O -p lock_dlm -t foo:bar $GFS_TGT])
 AT_CLEANUP
 
+AT_SETUP([Valid block sizes 512-4096])
+GFS_FSCK_CHECK([mkfs.gfs2 -O -p lock_nolock -b 512 $GFS_TGT])
+GFS_FSCK_CHECK([mkfs.gfs2 -O -p lock_nolock -b 1024 $GFS_TGT])
+GFS_FSCK_CHECK([mkfs.gfs2 -O -p lock_nolock -b 2048 $GFS_TGT])
+GFS_FSCK_CHECK([mkfs.gfs2 -O -p lock_nolock -b 4096 $GFS_TGT])
+AT_CLEANUP
+
 AT_SETUP([Max. blocks, min. block size])
 GFS_FSCK_CHECK([mkfs.gfs2 -O -p lock_nolock -b 512 $GFS_TGT $(gfs_max_blocks 512)])
 AT_CLEANUP
-- 
1.8.3.1