linux-ext4 - Re: [PATCH v2 2/4] e2image: add -b option to use supperblock backup

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <D7D92AC4-6A51-4BF5-8445-3EE487AEB382@dilger.ca>
Date:   Mon, 20 Nov 2017 12:45:47 -0700
From:   Andreas Dilger <adilger@...ger.ca>
To:     Artem Blagodarenko <artem.blagodarenko@...il.com>
Cc:     linux-ext4 <linux-ext4@...r.kernel.org>,
        Artem Blagodarenko <artem.blagodarenko@...gate.com>
Subject: Re: [PATCH v2 2/4] e2image: add -b option to use supperblock backup

On Nov 16, 2017, at 6:55 AM, Artem Blagodarenko <artem.blagodarenko@...il.com> wrote:
> 
> From: Artem Blagodarenko <artem.blagodarenko@...gate.com>
> 
> e2image has no ability to use superblock backup to copy metadata.
> This feature can be useful if someone wants to make partition
> image and fix it using e2fsck utility.
> 
> New -b option allows to pass superblock number, like e2fsck utility do.
> e2image doesn't change primary superblock and store is as is, so
> it can be fixed using e2fsck latter.
> 
> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@...il.com>
> ---
> lib/ext2fs/ext2fs.h |  4 ----
> misc/e2image.c      | 44 ++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
> index c394ec80..6774e32c 100644
> --- a/lib/ext2fs/ext2fs.h
> +++ b/lib/ext2fs/ext2fs.h
> @@ -1588,10 +1588,6 @@ extern errcode_t ext2fs_open2(const char *name, const char *io_options,
> 			      int flags, int superblock,
> 			      unsigned int block_size, io_manager manager,
> 			      ext2_filsys *ret_fs);
> -extern errcode_t ext2fs_try_open_fs(char *filesystem_name, char *io_options,
> -			     blk64_t superblock, int blocksize, int flags,
> -			     io_manager io_ptr, ext2_filsys *ret_fs);
> -

This should be part of the previous patch.

> /*
>  * The dgrp_t argument to these two functions is not actually a group number
>  * but a block number offset within a group table!  Convert with the formula
> diff --git a/misc/e2image.c b/misc/e2image.c
> index be001f0f..756a1dbc 100644
> --- a/misc/e2image.c
> +++ b/misc/e2image.c
> @@ -104,7 +104,8 @@ static int get_bits_from_size(size_t size)
> 
> static void usage(void)
> {
> -	fprintf(stderr, _("Usage: %s [ -r|Q ] [ -fr ] device image-file\n"),
> +	fprintf(stderr, _("Usage: %s [ -r|Q ] [ -b superblock ] [ -B blocksize]"
> +			  "[ -fr ] device image-file\n"),
> 		program_name);
> 	fprintf(stderr, _("       %s -I device image-file\n"), program_name);
> 	fprintf(stderr, _("       %s -ra  [  -cfnp  ] [ -o src_offset ] "
> @@ -1256,7 +1257,8 @@ static void output_qcow2_meta_data_blocks(ext2_filsys fs, int fd)
> 	free_qcow2_image(img);
> }
> 
> -static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags)
> +static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags,
> +				 blk64_t superblock)
> {
> 	struct process_block_struct	pb;
> 	struct ext2_inode		inode;
> @@ -1284,6 +1286,22 @@ static void write_raw_image_file(ext2_filsys fs, int fd, int type, int flags)
> 		}
> 	}
> 
> +	if (superblock) {
> +		int j;
> +
> +		ext2fs_mark_block_bitmap2(meta_block_map, superblock);
> +		meta_blocks_count++;
> +
> +		/*
> +		 * Mark the backup superblock descriptors
> +		 */
> +		for (j = 0; j < fs->desc_blocks; j++) {
> +			ext2fs_mark_block_bitmap2(meta_block_map,
> +			ext2fs_descriptor_block_loc2(fs, superblock, j));
> +		}
> +		meta_blocks_count += fs->desc_blocks;
> +	}
> +
> 	mark_table_blocks(fs);
> 	if (show_progress)
> 		fprintf(stderr, "%s", _("Scanning inodes...\n"));
> @@ -1463,6 +1481,8 @@ int main (int argc, char ** argv)
> 	int ignore_rw_mount = 0;
> 	int check = 0;
> 	struct stat st;
> +	blk64_t superblock = 0;
> +	int blocksize = 0;
> 
> #ifdef ENABLE_NLS
> 	setlocale(LC_MESSAGES, "");
> @@ -1476,7 +1496,7 @@ int main (int argc, char ** argv)
> 	if (argc && *argv)
> 		program_name = *argv;
> 	add_error_table(&et_ext2_error_table);
> -	while ((c = getopt(argc, argv, "nrsIQafo:O:pc")) != EOF)
> +	while ((c = getopt(argc, argv, "nrsIQafo:O:pcb:B:")) != EOF)

Options should preferably be in alphabetical order.  Even if the others
are not, at least these could be added at the start.

> @@ -1515,6 +1535,12 @@ int main (int argc, char ** argv)
> 		case 'c':
> 			check = 1;
> 			break;
> +		case 'b':
> +			superblock = strtoull(optarg, NULL, 0);
> +			break;
> +		case 'B':
> +			blocksize = strtoul(optarg, NULL, 0);
> +			break;

These should also be listed first.

Looks good otherwise.

Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)