linux-ext4 - RE: [PATCH] mke2fs: fix flexbg check to allow small filesystems

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <28989C0C9F1C0A428470D967B5FCED372B71A0@BPXM22GP.gisp.nec.co.jp>
Date:	Thu, 31 Jul 2014 00:12:11 +0000
From:	Akira Fujita <a-fujita@...jp.nec.com>
To:	Andreas Dilger <adilger@...ger.ca>, "tytso@....edu" <tytso@....edu>
CC:	"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
	Andreas Dilger <andreas.dilger@...el.com>
Subject: RE: [PATCH] mke2fs: fix flexbg check to allow small filesystems

Hi Andreas,

>From: linux-ext4-owner@...r.kernel.org [mailto:linux-ext4-owner@...r.kernel.org] On Behalf Of
>Andreas Dilger
>Sent: Wednesday, July 30, 2014 5:30 PM
>To: tytso@....edu
>Cc: linux-ext4@...r.kernel.org; Andreas Dilger; Andreas Dilger
>Subject: [PATCH] mke2fs: fix flexbg check to allow small filesystems
>
>Fix the flexbg check added in d988201ef9c so that it allows common
>small filesystem configurations to succeed instead of reporting
>a cryptic error message and failing due to a bad flex_bg layout:
>
>    mke2fs -O flex_bg -b 4096 -I 1024 -F /tmp/tt 79106
>    mke2fs 1.42.11 (09-Jul-2014)
>    /tmp/tt: Invalid argument passed to ext2 library while setting
>             up superblock
>
>This check in ext2fs_initialize() was added to prevent the metadata
>from being allocated beyond the end of the filesystem, but it is
>also causing a wide range of failures for small filesystems.
>
>Fix this by shrinking the flex_bg factor by half repeatedly until it
>would be expected to fit into the current filesystem size, rather
>than return an error as previously done.  Ideally it would only try
>a smaller flex_bg factor if the default value was being used, and had
>not been specified on the command line, but it isn't possible to see
>if this is the case in ext2fs_initialize() or not.
>
>Signed-off-by: Andreas Dilger <andreas.dilger@...el.com>
>---
> lib/ext2fs/initialize.c |   13 +++++++------
> 1 files changed, 7 insertions(+), 6 deletions(-)
>
>diff --git a/lib/ext2fs/initialize.c b/lib/ext2fs/initialize.c
>index de358fd..d58a664 100644
>--- a/lib/ext2fs/initialize.c
>+++ b/lib/ext2fs/initialize.c
>@@ -91,7 +91,7 @@ errcode_t ext2fs_initialize(const char *name, int flags,
> 	unsigned int	rem;
> 	unsigned int	overhead = 0;
> 	unsigned int	ipg;
>-	unsigned int	flexbg_size;
>+	unsigned int	flexbg_factor;
> 	dgrp_t		i;
> 	blk64_t		free_blocks;
> 	blk64_t		flexbg_overhead;
>@@ -428,18 +428,19 @@ ipg_retry:
> 	 * This is a simple check, so that the backup superblock and
> 	 * other feature related blocks are not considered.
> 	 */
>-	flexbg_size = 1 << fs->super->s_log_groups_per_flex;
>+flex_retry:
>+	flexbg_factor = 1 << super->s_log_groups_per_flex;
> 	flexbg_overhead = super->s_first_data_block + 1 +
> 		fs->desc_blocks + super->s_reserved_gdt_blocks +
>-		(__u64)flexbg_size * (2 + fs->inode_blocks_per_group);
>+		(__u64)flexbg_factor * (2 + fs->inode_blocks_per_group);
>
> 	/*
> 	 * Disallow creating ext4 which breaks flex_bg metadata layout
> 	 * obviously.
> 	 */

This comment is unneeded any more,
But patch looks good to me, thanks.

Reviewed-by: Akira Fujita <a-fujita@...jp.nec.co.jp>


>-	if (flexbg_overhead > ext2fs_blocks_count(fs->super)) {
>-		retval = EXT2_ET_INVALID_ARGUMENT;
>-		goto cleanup;
>+	if (flexbg_overhead > ext2fs_blocks_count(super)) {
>+		super->s_log_groups_per_flex -= 1;
>+		goto flex_retry;
> 	}
>
> 	/*
>--
>1.7.3.4
>

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html