| 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
| ||
|
Date: Tue, 5 Jun 2018 16:23:49 +0300
From: Artem Blagodarenko <artem.blagodarenko@...il.com>
To: Andreas Dilger <adilger@...ger.ca>
Cc: Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH v5 4/4] ext2fs: automaticlly open backup superblocks
Hello Andreas,
This issue was discussed on weekly ext4 developer concall (can not remain the date, months ago). If I understood Theodore right way , moving this code out of exported from ext2fs library was the main requirements for this new feature to be accepted. Theodore, I am right?
Thanks.
Artem Blagodarenko.
> On 4 Jun 2018, at 20:58, Andreas Dilger <adilger@...ger.ca> wrote:
>
> 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
>
>
>
>
>
Powered by blists - more mailing lists