| 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: Mon, 23 Nov 2020 11:55:29 -0800
From: harshad shirwadkar <harshadshirwadkar@...il.com>
To: Saranya Muruganandam <saranyamohan@...gle.com>
Cc: Ext4 Developers List <linux-ext4@...r.kernel.org>,
"Theodore Y. Ts'o" <tytso@....edu>,
Andreas Dilger <adilger.kernel@...ger.ca>,
Li Xi <lixi@....com>, Wang Shilong <wshilong@....com>
Subject: Re: [RFC PATCH v3 02/61] e2fsck: copy context when using multi-thread fsck
Thanks for the patch. I noticed that this patch doesn't compile. It's
because it needs "global_ctx" to be present in the e2fsck_struct but
it gets added later in the patch series in the patch 1e1b0216
("LU-8465 e2fsck: open io-channel when copying fs"). Given that this
and also a couple of other patches need global_ctx, should we split
1e1b0216 ("LU-8465 e2fsck: open io-channel when copying fs") should we
add global_ctx in this patch or as a separate patch before this?
On Wed, Nov 18, 2020 at 7:43 AM Saranya Muruganandam
<saranyamohan@...gle.com> wrote:
>
> From: Li Xi <lixi@....com>
>
> This patch only copy the context to a new one when -m is enabled.
> It doesn't actually start any thread. When pass1 test finishes,
> the new context is copied back to the original context.
>
> Since the signal handler only changes the original context, so
> add global_ctx in "struct e2fsck_struct" and use that to check
> whether there is any signal of canceling.
>
> This patch handles the long jump properly so that all the existing
> tests can be passed even the context has been copied. Otherwise,
> test f_expisize_ea_del would fail when aborting.
>
> Signed-off-by: Li Xi <lixi@....com>
> Signed-off-by: Wang Shilong <wshilong@....com>
> Signed-off-by: Saranya Muruganandam <saranyamohan@...gle.com>
> ---
> e2fsck/pass1.c | 114 +++++++++++++++++++++++++++++++++++++++++++++----
> e2fsck/unix.c | 1 +
> 2 files changed, 107 insertions(+), 8 deletions(-)
>
> diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
> index 8eecd958..64d237d3 100644
> --- a/e2fsck/pass1.c
> +++ b/e2fsck/pass1.c
> @@ -1144,7 +1144,22 @@ static int quota_inum_is_reserved(ext2_filsys fs, ext2_ino_t ino)
> return 0;
> }
>
> -void e2fsck_pass1(e2fsck_t ctx)
> +static int e2fsck_should_abort(e2fsck_t ctx)
> +{
> + e2fsck_t global_ctx;
> +
> + if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> + return 1;
> +
> + if (ctx->global_ctx) {
> + global_ctx = ctx->global_ctx;
> + if (global_ctx->flags & E2F_FLAG_SIGNAL_MASK)
> + return 1;
> + }
> + return 0;
> +}
> +
> +void e2fsck_pass1_thread(e2fsck_t ctx)
> {
> int i;
> __u64 max_sizes;
> @@ -1360,7 +1375,7 @@ void e2fsck_pass1(e2fsck_t ctx)
> if (ino > ino_threshold)
> pass1_readahead(ctx, &ra_group, &ino_threshold);
> ehandler_operation(old_op);
> - if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> + if (e2fsck_should_abort(ctx))
> goto endit;
> if (pctx.errcode == EXT2_ET_BAD_BLOCK_IN_INODE_TABLE) {
> /*
> @@ -1955,7 +1970,7 @@ void e2fsck_pass1(e2fsck_t ctx)
> if (process_inode_count >= ctx->process_inode_size) {
> process_inodes(ctx, block_buf);
>
> - if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> + if (e2fsck_should_abort(ctx))
> goto endit;
> }
> }
> @@ -2068,6 +2083,89 @@ endit:
> else
> ctx->invalid_bitmaps++;
> }
> +
> +static errcode_t e2fsck_pass1_thread_prepare(e2fsck_t global_ctx, e2fsck_t *thread_ctx)
> +{
> + errcode_t retval;
> + e2fsck_t thread_context;
> +
> + retval = ext2fs_get_mem(sizeof(struct e2fsck_struct), &thread_context);
> + if (retval) {
> + com_err(global_ctx->program_name, retval, "while allocating memory");
> + return retval;
> + }
> + memcpy(thread_context, global_ctx, sizeof(struct e2fsck_struct));
> + thread_context->fs->priv_data = thread_context;
> + thread_context->global_ctx = global_ctx;
> +
> + *thread_ctx = thread_context;
> + return 0;
> +}
> +
> +static int e2fsck_pass1_thread_join(e2fsck_t global_ctx, e2fsck_t thread_ctx)
> +{
> + int flags = global_ctx->flags;
> +#ifdef HAVE_SETJMP_H
> + jmp_buf old_jmp;
> +
> + memcpy(old_jmp, global_ctx->abort_loc, sizeof(jmp_buf));
> +#endif
> + memcpy(global_ctx, thread_ctx, sizeof(struct e2fsck_struct));
> +#ifdef HAVE_SETJMP_H
> + memcpy(global_ctx->abort_loc, old_jmp, sizeof(jmp_buf));
> +#endif
> + /* Keep the global singal flags*/
> + global_ctx->flags |= (flags & E2F_FLAG_SIGNAL_MASK) |
> + (global_ctx->flags & E2F_FLAG_SIGNAL_MASK);
> +
> + global_ctx->fs->priv_data = global_ctx;
> + ext2fs_free_mem(&thread_ctx);
> + return 0;
> +}
> +
> +void e2fsck_pass1_multithread(e2fsck_t ctx)
> +{
> + errcode_t retval;
> + e2fsck_t thread_ctx;
> +
> + retval = e2fsck_pass1_thread_prepare(ctx, &thread_ctx);
> + if (retval) {
> + com_err(ctx->program_name, 0,
> + _("while preparing pass1 thread\n"));
> + ctx->flags |= E2F_FLAG_ABORT;
> + return;
> + }
> +
> +#ifdef HAVE_SETJMP_H
> + /*
> + * When fatal_error() happens, jump to here. The thread
> + * context's flags will be saved, but its abort_loc will
> + * be overwritten by original jump buffer for the later
> + * tests.
> + */
> + if (setjmp(thread_ctx->abort_loc)) {
> + thread_ctx->flags &= ~E2F_FLAG_SETJMP_OK;
> + e2fsck_pass1_thread_join(ctx, thread_ctx);
> + return;
> + }
> + thread_ctx->flags |= E2F_FLAG_SETJMP_OK;
> +#endif
> +
> + e2fsck_pass1_thread(thread_ctx);
> + retval = e2fsck_pass1_thread_join(ctx, thread_ctx);
> + if (retval) {
> + com_err(ctx->program_name, 0,
> + _("while joining pass1 thread\n"));
> + ctx->flags |= E2F_FLAG_ABORT;
> + return;
> + }
> +}
> +
> +void e2fsck_pass1(e2fsck_t ctx)
> +{
> + e2fsck_pass1_multithread(ctx);
> +}
> +
> #undef FINISH_INODE_LOOP
>
> /*
> @@ -2130,7 +2228,7 @@ static void process_inodes(e2fsck_t ctx, char *block_buf)
> ehandler_operation(buf);
> check_blocks(ctx, &pctx, block_buf,
> &inodes_to_process[i].ea_ibody_quota);
> - if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> + if (e2fsck_should_abort(ctx))
> break;
> }
> ctx->stashed_inode = old_stashed_inode;
> @@ -3300,7 +3398,7 @@ static void check_blocks(e2fsck_t ctx, struct problem_context *pctx,
> inlinedata_fs = ext2fs_has_feature_inline_data(ctx->fs->super);
>
> if (check_ext_attr(ctx, pctx, block_buf, &ea_block_quota)) {
> - if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> + if (e2fsck_should_abort(ctx))
> goto out;
> pb.num_blocks += EXT2FS_B2C(ctx->fs, ea_block_quota.blocks);
> }
> @@ -3355,7 +3453,7 @@ static void check_blocks(e2fsck_t ctx, struct problem_context *pctx,
> }
> end_problem_latch(ctx, PR_LATCH_BLOCK);
> end_problem_latch(ctx, PR_LATCH_TOOBIG);
> - if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> + if (e2fsck_should_abort(ctx))
> goto out;
> if (pctx->errcode)
> fix_problem(ctx, PR_1_BLOCK_ITERATE, pctx);
> @@ -3836,7 +3934,7 @@ static int process_bad_block(ext2_filsys fs,
> *block_nr = 0;
> return BLOCK_CHANGED;
> }
> - if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> + if (e2fsck_should_abort(ctx))
> return BLOCK_ABORT;
> } else
> mark_block_used(ctx, blk);
> @@ -3933,7 +4031,7 @@ static int process_bad_block(ext2_filsys fs,
> *block_nr = 0;
> return BLOCK_CHANGED;
> }
> - if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> + if (e2fsck_should_abort(ctx))
> return BLOCK_ABORT;
> return 0;
> }
> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> index 051b31a5..42f616e2 100644
> --- a/e2fsck/unix.c
> +++ b/e2fsck/unix.c
> @@ -1445,6 +1445,7 @@ int main (int argc, char *argv[])
> }
> reserve_stdio_fds();
>
> + ctx->global_ctx = NULL;
> set_up_logging(ctx);
> if (ctx->logf) {
> int i;
> --
> 2.29.2.299.gdc1121823c-goog
>
Powered by blists - more mailing lists