| 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
| ||
|
Message-Id: <EA609DE0-CE36-4D41-B3CA-335DD777C468@dilger.ca>
Date: Fri, 13 Nov 2015 19:07:17 -0700
From: Andreas Dilger <adilger@...ger.ca>
To: Theodore Ts'o <tytso@....edu>
Cc: linux-ext4@...r.kernel.org
Subject: Re: [PATCH 2/4] LU-7368 e2fsck: skip quota update when interrupted
On Nov 13, 2015, at 6:10 PM, Andreas Dilger <andreas.dilger@...el.com> wrote:
> There is a bug in how e2fsck handles being interrupted by CTRL-C.
> If CTRL-C is pressed to kill e2fsck rather than e.g. kill -9, then
> the interrupt handler sets E2F_FLAG_CANCEL in the context but doesn't
> actually kill the process. Instead, e2fsck_pass1() checks this flag
> before processing the next inode.
Sigh, please remove the "LU-7368" label from the summary line, as that
is for internal tracking and I thought I'd done that for the patches
before sending them out.
If desired, this patch can be linked back to our bug tracker to get a
full discussion of the bug as below:
Lustre-change: http://review.whamcloud.com/17150
Lustre-bug-id: https://jira.hpdd.intel.com/browse/LU-7368
For "libext2fs: fix block-mapped file punch" it would be:
Lustre-change: http://review.whamcloud.com/17152
Lustre-bug-id: https://jira.hpdd.intel.com/browse/LU-7381
and for "e2fsck: fix e2fsck -fD directory truncation" it is:
Lustre-change: http://review.whamcloud.com/17153
Lustre-bug-id: https://jira.hpdd.intel.com/browse/LU-7381
Cheers, Andreas
> If a filesystem is running in fix mode (e2fsck -fy) is interrupted,
> and the quota feature is enabled, then the quota file will still be
> written to disk even though the inode scan was not complete and the
> quota information is totally inaccurate. Even worse, if the Pass 1
> inode and block scan was not finished, then the in-memory block
> bitmaps (which are used for block allocation during e2fsck) are also
> invalid, so any blocks allocated to the quota files may corrupt other
> files if those blocks were actually used.
>
> e2fsck 1.42.13.wc3 (28-Aug-2015)
> Pass 1: Checking inodes, blocks, and sizes
> ^C[QUOTA WARNING] Usage inconsistent for ID 0:
> actual (6455296, 168) != expected (8568832, 231)
> [QUOTA WARNING] Usage inconsistent for ID 695:
> actual (614932320256, 63981) != expected (2102405386240, 176432)
> Update quota info for quota type 0? yes
>
> [QUOTA WARNING] Usage inconsistent for ID 0:
> actual (6455296, 168) != expected (8568832, 231)
> [QUOTA WARNING] Usage inconsistent for ID 538:
> actual (614932320256, 63981) != expected (2102405386240, 176432)
> Update quota info for quota type 1? yes
>
> myth-OST0001: e2fsck canceled.
> myth-OST0001: ***** FILE SYSTEM WAS MODIFIED *****
>
> There may be a desire to flush out modified inodes and such that have
> been repaired, so that restarting an interrupted e2fsck will make
> progress, but the quota file update is plain wrong unless at least
> pass1 has finished, and the journal recreation is also dangerous if
> the block bitmaps have not been fully updated.
>
> Signed-off-by: Andreas Dilger <andreas.dilger@...el.com>
> ---
> e2fsck/e2fsck.c | 2 --
> e2fsck/e2fsck.h | 4 ++--
> e2fsck/pass1.c | 4 +++-
> e2fsck/pass2.c | 10 +++++-----
> e2fsck/unix.c | 18 ++++++++++--------
> 5 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/e2fsck/e2fsck.c b/e2fsck/e2fsck.c
> index 0ec1540..2002dc0 100644
> --- a/e2fsck/e2fsck.c
> +++ b/e2fsck/e2fsck.c
> @@ -203,8 +203,6 @@ static pass_t e2fsck_passes[] = {
> e2fsck_pass1, e2fsck_pass2, e2fsck_pass3, e2fsck_pass4,
> e2fsck_pass5, 0 };
>
> -#define E2F_FLAG_RUN_RETURN (E2F_FLAG_SIGNAL_MASK|E2F_FLAG_RESTART)
> -
> int e2fsck_run(e2fsck_t ctx)
> {
> int i;
> diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h
> index f904026..810030e 100644
> --- a/e2fsck/e2fsck.h
> +++ b/e2fsck/e2fsck.h
> @@ -173,10 +173,10 @@ struct resource_track {
> */
> #define E2F_FLAG_ABORT 0x0001 /* Abort signaled */
> #define E2F_FLAG_CANCEL 0x0002 /* Cancel signaled */
> -#define E2F_FLAG_SIGNAL_MASK 0x0003
> +#define E2F_FLAG_SIGNAL_MASK (E2F_FLAG_ABORT | E2F_FLAG_CANCEL)
> #define E2F_FLAG_RESTART 0x0004 /* Restart signaled */
> +#define E2F_FLAG_RUN_RETURN (E2F_FLAG_SIGNAL_MASK | E2F_FLAG_RESTART)
> #define E2F_FLAG_RESTART_LATER 0x0008 /* Restart after all iterations done */
> -
> #define E2F_FLAG_SETJMP_OK 0x0010 /* Setjmp valid for abort */
>
> #define E2F_FLAG_PROG_BAR 0x0020 /* Progress bar on screen */
> diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
> index 50a8b99..1c2b8fd 100644
> --- a/e2fsck/pass1.c
> +++ b/e2fsck/pass1.c
> @@ -766,7 +766,7 @@ void e2fsck_pass1(e2fsck_t ctx)
> inode, inode_size);
> ehandler_operation(old_op);
> if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
> - return;
> + goto endit;
> if (pctx.errcode == EXT2_ET_BAD_BLOCK_IN_INODE_TABLE) {
> if (!ctx->inode_bb_map)
> alloc_bb_map(ctx);
> @@ -1276,6 +1276,8 @@ endit:
>
> if ((ctx->flags & E2F_FLAG_SIGNAL_MASK) == 0)
> print_resource_track(ctx, _("Pass 1"), &rtrack, ctx->fs->io);
> + else
> + ctx->invalid_bitmaps++;
> }
>
> /*
> diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c
> index 2b7bff4..822eee4 100644
> --- a/e2fsck/pass2.c
> +++ b/e2fsck/pass2.c
> @@ -148,14 +148,14 @@ void e2fsck_pass2(e2fsck_t ctx)
>
> cd.pctx.errcode = ext2fs_dblist_iterate2(fs->dblist, check_dir_block,
> &cd);
> - if (ctx->flags & E2F_FLAG_SIGNAL_MASK || ctx->flags & E2F_FLAG_RESTART)
> - return;
> -
> if (ctx->flags & E2F_FLAG_RESTART_LATER) {
> ctx->flags |= E2F_FLAG_RESTART;
> - return;
> + ctx->flags &= ~E2F_FLAG_RESTART_LATER;
> }
>
> + if (ctx->flags & E2F_FLAG_RUN_RETURN)
> + return;
> +
> if (cd.pctx.errcode) {
> fix_problem(ctx, PR_2_DBLIST_ITERATE, &cd.pctx);
> ctx->flags |= E2F_FLAG_ABORT;
> @@ -739,7 +739,7 @@ static int check_dir_block(ext2_filsys fs,
> buf = cd->buf;
> ctx = cd->ctx;
>
> - if (ctx->flags & E2F_FLAG_SIGNAL_MASK || ctx->flags & E2F_FLAG_RESTART)
> + if (ctx->flags & E2F_FLAG_RUN_RETURN)
> return DIRENT_ABORT;
>
> if (ctx->progress && (ctx->progress)(ctx, 2, cd->count++, cd->max))
> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> index 66debcd..6d87f50 100644
> --- a/e2fsck/unix.c
> +++ b/e2fsck/unix.c
> @@ -1667,8 +1667,15 @@ print_unsupp_features:
> }
> no_journal:
>
> - if (ctx->qctx) {
> + if (run_result & E2F_FLAG_ABORT) {
> + fatal_error(ctx, _("aborted"));
> + } else if (run_result & E2F_FLAG_CANCEL) {
> + log_out(ctx, _("%s: e2fsck canceled.\n"), ctx->device_name ?
> + ctx->device_name : ctx->filesystem_name);
> + exit_value |= FSCK_CANCELED;
> + } else if (ctx->qctx && !ctx->invalid_bitmaps) {
> int i, needs_writeout;
> +
> for (i = 0; i < MAXQUOTAS; i++) {
> if (qtype != -1 && qtype != i)
> continue;
> @@ -1695,18 +1702,13 @@ no_journal:
> ext2fs_close_free(&fs);
> goto restart;
> }
> - if (run_result & E2F_FLAG_ABORT)
> - fatal_error(ctx, _("aborted"));
>
> #ifdef MTRACE
> mtrace_print("Cleanup");
> #endif
> was_changed = ext2fs_test_changed(fs);
> - if (run_result & E2F_FLAG_CANCEL) {
> - log_out(ctx, _("%s: e2fsck canceled.\n"), ctx->device_name ?
> - ctx->device_name : ctx->filesystem_name);
> - exit_value |= FSCK_CANCELED;
> - } else if (!(ctx->options & E2F_OPT_READONLY)) {
> + if (!(ctx->flags & E2F_FLAG_RUN_RETURN) &&
> + !(ctx->options & E2F_OPT_READONLY)) {
> if (ext2fs_test_valid(fs)) {
> if (!(sb->s_state & EXT2_VALID_FS))
> exit_value |= FSCK_NONDESTRUCT;
> --
> 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
Cheers, Andreas
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists