Date:   Fri, 15 Jun 2018 12:15:46 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Artem Blagodarenko <artem.blagodarenko@...il.com>
Cc:     Ext4 Developers List <linux-ext4@...r.kernel.org>,
        Alexey Lyashkov <alexey.lyashkov@...il.com>,
        Jim Garlick <garlick@...l.gov>
Subject: Re: [PATCH] e2fsck: allow deleting or zeroing shared blocks

On Jun 15, 2018, at 11:47 AM, Artem Blagodarenko <artem.blagodarenko@...il.com> wrote:
> 
> From: Andreas Dilger <andreas.dilger@...el.com>

This should be:

    From: Jim Garlick <garlick@...l.gov>

as I've only been keeping this patch alive in our tree (LLNL no longer
uses ext4 for Lustre).

Cheers, Andreas

> E2fsck fixes files that are found to be sharing blocks by cloning
> the shared blocks and giving each file a private copy in pass 1D.
> 
> Allowing all files claiming the shared blocks to have copies can
> inadvertantly bypass access restrictions.  Deleting all the files,
> zeroing the cloned blocks, or placing the files in the /lost+found
> directory after cloning may be preferable in some secure environments.
> 
> The following patches implement config file and command line options
> in e2fsck that allow pass 1D behavior to be tuned according to site
> policy.  It adds two extended options and config file counterparts.
> On the command line:
> 
> -E clone=dup|zero
> 
>    Select the block cloning method.  "dup" is old behavior,
>    and is the default.  "zero" is a new method that substitutes
>    zero-filled blocks for the shared blocks in all the files
>    that claim them.
> 
> -E shared=preserve|lost+found|delete
> 
>    Select the disposition of files containing shared blocks.
>    "preserve" is the old behavior which remains the default.
>    "lost+found" causes files to be unlinked after cloning so
>    they will be reconnected to /lost+found in pass 3.
>    "delete" skips cloning entirely and simply deletes the files.
> 
> In the config file:
>  [options]
>      clone=dup|zero
>      shared=preserve|lost+found|delete
> 
> Signed-off-by: Jim Garlick <garlick@...l.gov>
> Signed-off-by: Andreas Dilger <andreas.dilger@...el.com>
> ---
> e2fsck/e2fsck.8.in      | 13 +++++++++
> e2fsck/e2fsck.conf.5.in | 13 +++++++++
> e2fsck/e2fsck.h         | 13 +++++++++
> e2fsck/pass1b.c         | 48 +++++++++++++++++++++++++------
> e2fsck/problem.c        |  8 ++++++
> e2fsck/problem.h        |  6 ++++
> e2fsck/unix.c           | 76 +++++++++++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 168 insertions(+), 9 deletions(-)
> 
> diff --git a/e2fsck/e2fsck.8.in b/e2fsck/e2fsck.8.in
> index 1a3bd468..ec6427ee 100644
> --- a/e2fsck/e2fsck.8.in
> +++ b/e2fsck/e2fsck.8.in
> @@ -198,6 +198,19 @@ separated, and may take an argument using the equals ('=') sign.  The
> following options are supported:
> .RS 1.2i
> .TP
> +.BI clone= dup|zero
> +Resolve files with shared blocks in pass 1D by giving each file a private
> +copy of the blocks (dup);
> +or replacing the shared blocks with private, zero-filled blocks (zero).
> +The default is dup.
> +.TP
> +.BI shared= preserve|lost+found|delete
> +Files with shared blocks discovered in pass 1D are cloned and then left
> +in place (preserve);
> +cloned and then disconnected from their parent directory,
> +then reconnected to /lost+found in pass 3 (lost+found);
> +or simply deleted (delete).  The default is preserve.
> +.TP
> .BI ea_ver= extended_attribute_version
> Set the version of the extended attribute blocks which
> .B e2fsck
> diff --git a/e2fsck/e2fsck.conf.5.in b/e2fsck/e2fsck.conf.5.in
> index 708e2134..611ff7cb 100644
> --- a/e2fsck/e2fsck.conf.5.in
> +++ b/e2fsck/e2fsck.conf.5.in
> @@ -147,6 +147,19 @@ will offer to clear
> the test_fs flag if the ext4 filesystem is available on the system.  It
> defaults to true.
> .TP
> +.I clone
> +This string relation controls the default handling of shared blocks in pass 1D.
> +It can be set to dup or zero.  See the
> +.I "-E clone"
> +option description in e2fsck(8).
> +.TP
> +.I shared
> +This string relation controls the default disposition of files discovered to
> +have shared blocks in pass 1D.  It can be set to preserve, lost+found,
> +or delete.  See the
> +.I "-E shared"
> +option description in e2fsck(8).
> +.TP
> .I defer_check_on_battery
> This boolean relation controls whether or not the interval between
> filesystem checks (either based on time or number of mounts) should
> diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h
> index 5269650f..0014fc2c 100644
> --- a/e2fsck/e2fsck.h
> +++ b/e2fsck/e2fsck.h
> @@ -209,6 +209,17 @@ struct resource_track {
> #define E2F_PASS_5	5
> #define E2F_PASS_1B	6
> 
> +enum shared_opt {
> +	E2F_SHARED_PRESERVE = 0,
> +	E2F_SHARED_DELETE,
> +	E2F_SHARED_LPF
> +};
> +
> +enum clone_opt {
> +	E2F_CLONE_DUP = 0,
> +	E2F_CLONE_ZERO
> +};
> +
> /*
>  * Define the extended attribute refcount structure
>  */
> @@ -383,6 +394,8 @@ struct e2fsck_struct {
> 	time_t now;
> 	time_t time_fudge;	/* For working around buggy init scripts */
> 	int ext_attr_ver;
> +	enum shared_opt shared;
> +	enum clone_opt clone;
> 	profile_t	profile;
> 	int blocks_per_page;
> 	ext2_u32_list encrypted_dirs;
> diff --git a/e2fsck/pass1b.c b/e2fsck/pass1b.c
> index 392ff2c6..e63fb083 100644
> --- a/e2fsck/pass1b.c
> +++ b/e2fsck/pass1b.c
> @@ -523,6 +523,9 @@ static void pass1d(e2fsck_t ctx, char *block_buf)
> 			q = (struct dup_cluster *) dnode_get(m);
> 			if (q->num_bad > 1)
> 				file_ok = 0;
> +			if (q->num_bad == 1 && (ctx->clone == E2F_CLONE_ZERO ||
> +			    ctx->shared != E2F_SHARED_PRESERVE))
> +				file_ok = 0;
> 			if (check_if_fs_cluster(ctx, s->cluster)) {
> 				file_ok = 0;
> 				meta_data = 1;
> @@ -582,13 +585,26 @@ static void pass1d(e2fsck_t ctx, char *block_buf)
> 			fix_problem(ctx, PR_1D_DUP_BLOCKS_DEALT, &pctx);
> 			continue;
> 		}
> -		if (fix_problem(ctx, PR_1D_CLONE_QUESTION, &pctx)) {
> +		if (ctx->shared != E2F_SHARED_DELETE &&
> +		    fix_problem(ctx, PR_1D_CLONE_QUESTION, &pctx)) {
> 			pctx.errcode = clone_file(ctx, ino, p, block_buf);
> -			if (pctx.errcode)
> +			if (pctx.errcode) {
> 				fix_problem(ctx, PR_1D_CLONE_ERROR, &pctx);
> -			else
> -				continue;
> +				goto delete;
> +			}
> +			if (ctx->shared == E2F_SHARED_LPF &&
> +			    fix_problem(ctx, PR_1D_DISCONNECT_QUESTION, &pctx)) {
> +				pctx.errcode = ext2fs_unlink(fs, p->dir,
> +							     NULL, ino, 0);
> +				if (pctx.errcode) {
> +					fix_problem(ctx, PR_1D_DISCONNECT_ERROR,
> +						    &pctx);
> +					goto delete;
> +				}
> +			}
> +			continue;
> 		}
> +delete:
> 		if (fix_problem(ctx, PR_1D_DELETE_QUESTION, &pctx))
> 			delete_file(ctx, ino, p, block_buf);
> 		else
> @@ -606,7 +622,8 @@ static void decrement_badcount(e2fsck_t ctx, blk64_t block,
> {
> 	p->num_bad--;
> 	if (p->num_bad <= 0 ||
> -	    (p->num_bad == 1 && !check_if_fs_block(ctx, block))) {
> +	    (p->num_bad == 1 && !check_if_fs_block(ctx, block) &&
> +	    ctx->clone == E2F_CLONE_DUP)) {
> 		if (check_if_fs_cluster(ctx, EXT2FS_B2C(ctx->fs, block)))
> 			return;
> 		ext2fs_unmark_block_bitmap2(ctx->block_dup_map, block);
> @@ -799,6 +816,14 @@ static int clone_file_block(ext2_filsys fs,
> 
> 		p = (struct dup_cluster *) dnode_get(n);
> 
> +		if (!is_meta) {
> +			if (ctx->clone == E2F_CLONE_ZERO && p->num_bad == 0) {
> +				ext2fs_unmark_block_bitmap2(ctx->block_found_map,
> +							    *block_nr);
> +				ext2fs_block_alloc_stats(fs, *block_nr, -1);
> +			}
> +		}
> +
> 		cs->dup_cluster = c;
> 		/*
> 		 * Let's try an implied cluster allocation.  If we get the same
> @@ -838,10 +863,15 @@ cluster_alloc_ok:
>  		printf("Cloning block #%lld from %llu to %llu\n",
> 		       blockcnt, *block_nr, new_block);
> #endif
> -		retval = io_channel_read_blk64(fs->io, *block_nr, 1, cs->buf);
> -		if (retval) {
> -			cs->errcode = retval;
> -			return BLOCK_ABORT;
> +		if (ctx->clone == E2F_CLONE_ZERO) {
> +			memset(cs->buf, 0, fs->blocksize);
> +		} else {
> +			retval = io_channel_read_blk64(fs->io, *block_nr, 1,
> +						       cs->buf);
> +			if (retval) {
> +				cs->errcode = retval;
> +				return BLOCK_ABORT;
> +			}
> 		}
> 		retval = io_channel_write_blk64(fs->io, new_block, 1, cs->buf);
> 		if (retval) {
> diff --git a/e2fsck/problem.c b/e2fsck/problem.c
> index edc9d51f..4c1d8628 100644
> --- a/e2fsck/problem.c
> +++ b/e2fsck/problem.c
> @@ -1264,6 +1264,14 @@ static struct e2fsck_problem problem_table[] = {
> 	{ PR_1D_CLONE_ERROR,
> 	  N_("Couldn't clone file: %m\n"), PROMPT_NONE, 0 },
> 
> +	/* File with shared blocks found */
> +	{ PR_1D_DISCONNECT_QUESTION,
> +	  N_("File with shared blocks found\n"), PROMPT_CONNECT, 0 },
> +
> +	/* Couldn't unlink file (error) */
> +	{ PR_1D_DISCONNECT_ERROR,
> +	  N_("Couldn't unlink file: %m\n"), PROMPT_NONE, 0 },
> +
> 	/* Pass 1E Extent tree optimization	*/
> 
> 	/* Pass 1E: Optimizing extent trees */
> diff --git a/e2fsck/problem.h b/e2fsck/problem.h
> index 482d111a..516cbc35 100644
> --- a/e2fsck/problem.h
> +++ b/e2fsck/problem.h
> @@ -751,6 +751,12 @@ struct problem_context {
> /* Couldn't clone file (error) */
> #define PR_1D_CLONE_ERROR	0x013008
> 
> +/* File with shared blocks found */
> +#define PR_1D_DISCONNECT_QUESTION 0x013009
> +
> +/* Couldn't unlink file (error) */
> +#define PR_1D_DISCONNECT_ERROR	0x01300A
> +
> /*
>  * Pass 1e --- rebuilding extent trees
>  */
> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
> index 0294f5bd..e1de9060 100644
> --- a/e2fsck/unix.c
> +++ b/e2fsck/unix.c
> @@ -649,6 +649,49 @@ static void signal_cancel(int sig EXT2FS_ATTR((unused)))
> }
> #endif
> 
> +static void initialize_profile_options(e2fsck_t ctx)
> +{
> +	char *tmp;
> +
> +	/* [options] shared=preserve|lost+found|delete */
> +	tmp = NULL;
> +	ctx->shared = E2F_SHARED_PRESERVE;
> +	profile_get_string(ctx->profile, "options", "shared", 0,
> +			   "preserve", &tmp);
> +	if (tmp) {
> +		if (strcmp(tmp, "preserve") == 0)
> +			ctx->shared = E2F_SHARED_PRESERVE;
> +		else if (strcmp(tmp, "delete") == 0)
> +			ctx->shared = E2F_SHARED_DELETE;
> +		else if (strcmp(tmp, "lost+found") == 0)
> +			ctx->shared = E2F_SHARED_LPF;
> +		else {
> +			com_err(ctx->program_name, 0,
> +				_("configuration error: 'shared=%s'"), tmp);
> +			fatal_error(ctx, 0);
> +		}
> +		free(tmp);
> +	}
> +
> +	/* [options] clone=dup|zero */
> +	tmp = NULL;
> +	ctx->clone = E2F_CLONE_DUP;
> +	profile_get_string(ctx->profile, "options", "clone", 0,
> +			   "dup", &tmp);
> +	if (tmp) {
> +		if (strcmp(tmp, "dup") == 0)
> +			ctx->clone = E2F_CLONE_DUP;
> +		else if (strcmp(tmp, "zero") == 0)
> +			ctx->clone = E2F_CLONE_ZERO;
> +		else {
> +			com_err(ctx->program_name, 0,
> +				_("configuration error: 'clone=%s'"), tmp);
> +			fatal_error(ctx, 0);
> +		}
> +		free(tmp);
> +	}
> +}
> +
> static void parse_extended_opts(e2fsck_t ctx, const char *opts)
> {
> 	char	*buf, *token, *next, *p, *arg;
> @@ -699,6 +742,36 @@ static void parse_extended_opts(e2fsck_t ctx, const char *opts)
> 		} else if (strcmp(token, "fragcheck") == 0) {
> 			ctx->options |= E2F_OPT_FRAGCHECK;
> 			continue;
> +		/* -E shared=preserve|lost+found|delete */
> +		} else if (strcmp(token, "shared") == 0) {
> +			if (!arg) {
> +				extended_usage++;
> +				continue;
> +			}
> +			if (strcmp(arg, "preserve") == 0) {
> +				ctx->shared = E2F_SHARED_PRESERVE;
> +			} else if (strcmp(arg, "lost+found") == 0) {
> +				ctx->shared = E2F_SHARED_LPF;
> +			} else if (strcmp(arg, "delete") == 0) {
> +				ctx->shared = E2F_SHARED_DELETE;
> +			} else {
> +				extended_usage++;
> +				continue;
> +			}
> +		/* -E clone=dup|zero */
> +		} else if (strcmp(token, "clone") == 0) {
> +			if (!arg) {
> +				extended_usage++;
> +				continue;
> +			}
> +			if (strcmp(arg, "dup") == 0) {
> +				ctx->clone = E2F_CLONE_DUP;
> +			} else if (strcmp(arg, "zero") == 0) {
> +				ctx->clone = E2F_CLONE_ZERO;
> +			} else {
> +				extended_usage++;
> +				continue;
> +			}
> 		} else if (strcmp(token, "journal_only") == 0) {
> 			if (arg) {
> 				extended_usage++;
> @@ -760,6 +833,7 @@ static void parse_extended_opts(e2fsck_t ctx, const char *opts)
> 		fputs(_("\treadahead_kb=<buffer size>\n"), stderr);
> 		fputs("\tbmap2extent\n", stderr);
> 		fputs("\tfixes_only\n", stderr);
> +		fputs(("\tclone=<dup|zero>\n"), stderr);
> 		fputc('\n', stderr);
> 		exit(1);
> 	}
> @@ -844,6 +918,8 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx)
> 	if (c)
> 		ctx->options |= E2F_OPT_ICOUNT_FULLMAP;
> 
> +	initialize_profile_options(ctx);
> +
> 	phys_mem_kb = get_memory_size() / 1024;
> 	ctx->readahead_kb = ~0ULL;
> 	while ((c = getopt(argc, argv, "panyrcC:B:dE:fvtFVM:b:I:j:P:l:L:N:SsDkz:")) != EOF)
> --
> 2.14.3
> 


Cheers, Andreas






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

Powered by blists - more mailing lists