linux-ext4 - Re: [PATCH v5 4/4] ext2fs: automaticlly open backup superblocks

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: <5953F497-2045-4615-8552-2D584B99228A@dilger.ca>
Date:   Mon, 4 Jun 2018 11:58:25 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Artem Blagodarenko <artem.blagodarenko@...il.com>
Cc:     Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH v5 4/4] ext2fs: automaticlly open backup superblocks

On May 26, 2018, at 4:46 PM, Artem Blagodarenko <artem.blagodarenko@...il.com> wrote:
> 
> e2image and e2fsck automatically try to open some backup superblocks,
> if only blocksize is set or passed superblock can't be opened.
> Try few backup superblocks (e.g. {1, 3, 5, 7, 9} * blocksize * 8).
> 
> This code is moved to lib/support/.

I don't recall seeing an explanation of why this is in lib/support/
instead of lib/ext2fs?  Maybe as ext2fs_open_backups(), or is there
some reason we don't want to export this function?

> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@...il.com>
> ---
> 
> diff --git a/e2fsck/message.c b/e2fsck/message.c
> index 727f71d5..415d7609 100644
> --- a/e2fsck/message.c
> +++ b/e2fsck/message.c
> @@ -465,7 +465,8 @@ static _INLINE_ void expand_percent_expression(FILE *f, ext2_filsys fs,
> 		fprintf(f, "%*lld", width, (long long) ctx->blkcount);
> 		break;
> 	case 'S':
> -		fprintf(f, "%llu", get_backup_sb(NULL, fs, NULL, NULL));
> +		fprintf(f, "%llu", get_first_backup_sb(NULL, NULL, fs,
> +							  NULL, NULL));

(style) align after '('

> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> index 491e9eb6..1a5e556f 100644
> --- a/e2fsck/unix.c
> +++ b/e2fsck/unix.c
> @@ -1479,11 +1479,19 @@ restart:
> 				retval ? _("Superblock invalid,") :
> 				_("Group descriptors look bad..."));
> 			orig_superblock = ctx->superblock;
> -			get_backup_sb(ctx, fs, ctx->filesystem_name, io_ptr);
> -			if (fs)
> -				ext2fs_close_free(&fs);
> 			orig_retval = retval;
> -			retval = try_open_fs(ctx, flags, io_ptr, &fs);
> +			retval = try_backups(ctx->filesystem_name,
> +						    ctx->io_options,
> +						    flags,
> +						    &ctx->superblock,
> +						    &ctx->blocksize, io_ptr,
> +						    &fs);

(style) align after '(', pack onto fewest lines possible

> diff --git a/lib/support/sb_backup.c b/lib/support/sb_backup.c
> new file mode 100644
> index 00000000..5660e237
> --- /dev/null
> +++ b/lib/support/sb_backup.c

(style) remove extra blank lines

> +blk64_t get_first_backup_sb(blk64_t *superblock, unsigned int *block_size,
> +			       ext2_filsys fs, const char *name,
> +			       io_manager manager)

(style) align after '('

> +{
> +	struct ext2_super_block *sb;
> +	io_channel		io = NULL;
> +	void			*buf = NULL;
> +	int			try_blocksize;
> +	blk64_t			try_superblock, ret_sb = 8193;
> +
> +	/* superblock and block_size can be NULL if fs->super is passed */
> +	if (fs && fs->super) {
> +		ret_sb = (fs->super->s_blocks_per_group +
> +			  fs->super->s_first_data_block);

(style) no need for parenthesis here

> +		if (superblock)
> +			*superblock = ret_sb;
> +		if(block_size)

(style) space after "if"

> +}
> +
> +

(style) remove extra blank line

> +errcode_t try_backups(const char *name, const char *io_options,
> +			    int flags, blk64_t *superblock,
> +			    unsigned int *block_size, io_manager manager,
> +			    ext2_filsys *ret_fs)

(style) align after '('

> +{
> +	errcode_t retval;
> +	blk64_t try_block_number;
> +	unsigned int i;
> +
> +	/*
> +	 * Get first superblock location based on heuristic.
> +	 * Blocksize is also returned and used to find next
> +	 * superblock copy location.
> +	 */
> +	try_block_number = get_first_backup_sb(superblock, block_size,
> +						  *ret_fs, name, manager);

(style) align after '('

> +
> +}
> +
> +

(style) remove blank lines at end

> diff --git a/lib/support/sb_backup.h b/lib/support/sb_backup.h
> new file mode 100644
> index 00000000..f1d48bff
> --- /dev/null
> +++ b/lib/support/sb_backup.h
> @@ -0,0 +1,20 @@
> +blk64_t get_first_backup_sb(blk64_t *superblock, unsigned int *block_size,
> +			       ext2_filsys fs, const char *name,
> +			       io_manager manager);
> +
> +errcode_t try_backups(const char *name, const char *io_options,
> +			    int flags, blk64_t *superblock,
> +			    unsigned int *block_size, io_manager manager,
> +			    ext2_filsys *ret_fs);

(style) align after '(

> diff --git a/misc/e2image.c b/misc/e2image.c
> index 727e3876..d8569068 100644
> --- a/misc/e2image.c
> +++ b/misc/e2image.c
> @@ -1612,6 +1612,13 @@ int main (int argc, char ** argv)
> 	sprintf(offset_opt, "offset=%llu", source_offset);
> 	retval = ext2fs_open2(device_name, offset_opt, open_flag,
> 			      superblock, blocksize, unix_io_manager, &fs);
> +	if (retval & (superblock | blocksize)) {
> +		printf(_("Try backups in other location.\n"));
> +		retval = try_backups(device_name, offset_opt, open_flag,
> +					    &superblock, &blocksize,
> +					    unix_io_manager, &fs);

(style) align after '('


Cheers, Andreas






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