mke2fs: check that auto-detected blocksize <= sys_page_size

PATCH RESEND Superseded Yury V. Zaytsev 2 participants 2 messages Aug 10, 2011 – Sep 16, 2011

Share & tools

Yury V. Zaytsev

author

2011-08-10 09:16 UTC #0 lore raw

PATCH RESEND v2 1/1 mke2fs: check that auto-detected blocksize <= sys_page_size

Block size can be specified manually via the -b option or deduced automatically. Unfortunately, the check that it is still smaller than the system page size is only performed right after the command line options are parsed.

Therefore, if buggy or inappropriately installed/configured hardware hints that larger block sizes have to be used, mkfs will silently create a file system which can not be mounted on the system in question.

By moving the check beyond the last assignment to blocksize it is now ensured, that mkfs will issue a warning even if inappropriate blocksize was auto-detected.

The new behavior can be easily tested, by exporting the following variables before running mkfs:

export MKE2FS_DEVICE_SECTSIZE=8192
export MKE2FS_DEVICE_PHYS_SECTSIZE=8192

Signed-off-by Yury V. Zaytsev <[email protected]>

Reviewed-by Lukas Czerner <[email protected]>

---
 misc/mke2fs.c |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

misc/mke2fs.c +13 ~~-11~~

index e062bda..3f1b642 100644
@@ -1514,17 +1514,6 @@ profile_error:
 		ext2fs_close(jfs);
 	}
 
-	if (blocksize > sys_page_size) {
-		if (!force) {
-			com_err(program_name, 0,
-				_("%d-byte blocks too big for system (max %d)"),
-				blocksize, sys_page_size);
-			proceed_question();
-		}
-		fprintf(stderr, _("Warning: %d-byte blocks too big for system "
-				  "(max %d), forced to continue\n"),
-			blocksize, sys_page_size);
-	}
 	if (optind < argc) {
 		fs_blocks_count = parse_num_blocks2(argv[optind++],
 						   fs_param.s_log_block_size);

@@ -1833,6 +1822,19 @@ profile_error:
 
 	blocksize = EXT2_BLOCK_SIZE(&fs_param);
 
+	/* This check should happen beyond the last assignment to blocksize */
+	if (blocksize > sys_page_size) {
+		if (!force) {
+			com_err(program_name, 0,
+				_("%d-byte blocks too big for system (max %d)"),
+				blocksize, sys_page_size);
+			proceed_question();
+		}
+		fprintf(stderr, _("Warning: %d-byte blocks too big for system "
+				  "(max %d), forced to continue\n"),
+			blocksize, sys_page_size);
+	}
+
 	lazy_itable_init = 0;
 	if (access("/sys/fs/ext4/features/lazy_itable_init", R_OK) == 0)
 		lazy_itable_init = 1;

Signature · 1 line

1.7.6

Ted Ts'o

2011-09-16 03:11 UTC #1 lore raw

Quoted 13 lines · Yury V. Zaytsev

Block size can be specified manually via the -b option or deduced automatically. Unfortunately, the check that it is still smaller than the system page size is only performed right after the command line options are parsed.
Therefore, if buggy or inappropriately installed/configured hardware hints that larger block sizes have to be used, mkfs will silently create a file system which can not be mounted on the system in question.
By moving the check beyond the last assignment to blocksize it is now ensured, that mkfs will issue a warning even if inappropriate blocksize was auto-detected.

Quoted 8 lines

The new behavior can be easily tested, by exporting the following variables before running mkfs:
export MKE2FS_DEVICE_SECTSIZE=8192
export MKE2FS_DEVICE_PHYS_SECTSIZE=8192
Signed-off-by Yury V. Zaytsev <[email protected]>
Reviewed-by Lukas Czerner <[email protected]>

Thanks, I've merged this into the next branch. Apologies for the delay.

- Ted