[PATCH] erofs-utils: fsck: fix issues related to --extract=X

[PATCH] erofs-utils: fsck: fix issues related to --extract=X

[Igor Ostapenko]

From: Igor Eisberg <igoreisberg@gmail.com>

Have to disagree with some changes made to my original patch.
1) Using tar options makes no sense here, since tar has default
   behaviors set for normal user (uses user's owner ID + perms)
   vs. root user (preserve original owner IDs + perms by default),
   and the options were there to override that preset behavior.
   fsck doesn't have any default behavior set, and the default
   values for preserve_owner and preserve_perms were left for
   the compiler to decide. This change sets the default values
   to false explicitly.
   "--no-same-owner" and "--no-same-permissions" are completely
   redundant in fsck's case, unlike tar.
   * "--same-owner" and "--same-permissions" were renamed to
     "--preserve-owner" and "--preserve-perms" to better represent
     what these options do, the word "same" is ambiguous and tells
     nothing to the user ("same" to what?).
   * Added "--preserve" as a shortcut for both options in one.
   * Fixed option descriptions as they had typos and were too
     ambiguous ("extract information" to where? separate file?).
2) Errors for chmod 0700 were not logged, failures were silent.
3) --extract=/ should fail with a proper error due to it being
   dangerous if used with sudo.

Signed-off-by: Igor Ostapenko <igoreisberg@gmail.com>
---
 fsck/main.c | 62 ++++++++++++++++++++++++++++++++---------------------
 1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/fsck/main.c b/fsck/main.c
index 14534b9..e2f4157 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -18,15 +18,17 @@
 static int erofsfsck_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
 
 struct erofsfsck_cfg {
-	bool corrupted;
 	bool print_comp_ratio;
 	bool check_decomp;
 	char *extract_path;
 	size_t extract_pos;
-	bool overwrite, preserve_owner, preserve_perms;
+	bool overwrite;
+	bool preserve_owner;
+	bool preserve_perms;
 	mode_t umask;
 	u64 physical_blocks;
 	u64 logical_blocks;
+	bool corrupted;
 };
 static struct erofsfsck_cfg fsckcfg;
 
@@ -34,11 +36,10 @@ static struct option long_options[] = {
 	{"help", no_argument, 0, 1},
 	{"extract", optional_argument, 0, 2},
 	{"device", required_argument, 0, 3},
-	{"no-same-owner", no_argument, 0, 4},
-	{"no-same-permissions", no_argument, 0, 5},
-	{"same-owner", no_argument, 0, 6},
-	{"same-permissions", no_argument, 0, 7},
-	{"overwrite", no_argument, 0, 8},
+	{"overwrite", no_argument, 0, 4},
+	{"preserve", no_argument, 0, 5},
+	{"preserve-owner", no_argument, 0, 6},
+	{"preserve-perms", no_argument, 0, 7},
 	{0, 0, 0, 0},
 };
 
@@ -66,11 +67,10 @@ static void usage(void)
 	      " --extract[=X]          check if all files are well encoded, optionally extract to X\n"
 	      " --help                 display this help and exit\n"
 	      "\nExtraction options (--extract=X is required):\n"
-	      " --no-same-owner        extract files as yourself\n"
-	      " --no-same-permissions  apply the user's umask when extracting permission\n"
-	      " --same-permissions     extract information about file permissions\n"
-	      " --same-owner           extract files about the file ownerships\n"
-	      " --overwrite            if file already exists then overwrite\n"
+	      " --overwrite            overwrite files that already exist\n"
+	      " --preserve             extract with original ownerships and permissions\n"
+	      " --preserve-owner       extract with original ownerships only\n"
+	      " --preserve-perms       extract with original permissions only\n"
 	      "\nSupported algorithms are: ", stderr);
 	print_available_decompressors(stderr, ", ");
 }
@@ -112,10 +112,16 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
 				if (len == 0)
 					return -EINVAL;
 
-				/* remove trailing slashes except root */
-				while (len > 1 && optarg[len - 1] == '/')
+				/* remove trailing slashes */
+				while (len > 0 && optarg[len - 1] == '/')
 					len--;
 
+				/* extracting directly to root is dangerous */
+				if (len == 0) {
+					erofs_err("invalid extract path: /");
+					return -EINVAL;
+				}
+
 				fsckcfg.extract_path = malloc(PATH_MAX);
 				if (!fsckcfg.extract_path)
 					return -ENOMEM;
@@ -131,10 +137,11 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
 			++sbi.extra_devices;
 			break;
 		case 4:
-			fsckcfg.preserve_owner = false;
+			fsckcfg.overwrite = true;
 			break;
 		case 5:
-			fsckcfg.preserve_perms = false;
+			fsckcfg.preserve_owner = true;
+			fsckcfg.preserve_perms = true;
 			break;
 		case 6:
 			fsckcfg.preserve_owner = true;
@@ -142,9 +149,6 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
 		case 7:
 			fsckcfg.preserve_perms = true;
 			break;
-		case 8:
-			fsckcfg.overwrite = true;
-			break;
 		default:
 			return -EINVAL;
 		}
@@ -465,7 +469,7 @@ static inline int erofs_extract_dir(struct erofs_inode *inode)
 		struct stat st;
 
 		if (errno != EEXIST) {
-			erofs_err("failed to create directory %s: %s",
+			erofs_err("failed to create directory: %s (%s)",
 				  fsckcfg.extract_path, strerror(errno));
 			return -errno;
 		}
@@ -481,8 +485,11 @@ static inline int erofs_extract_dir(struct erofs_inode *inode)
 		 * Try to change permissions of existing directory so
 		 * that we can write to it
 		 */
-		if (chmod(fsckcfg.extract_path, 0700) < 0)
+		if (chmod(fsckcfg.extract_path, 0700) < 0) {
+			erofs_err("failed to set permissions: %s (%s)",
+				  fsckcfg.extract_path, strerror(errno));
 			return -errno;
+		}
 	}
 	return 0;
 }
@@ -510,6 +517,8 @@ again:
 				}
 			} else if (errno == EACCES &&
 				   chmod(fsckcfg.extract_path, 0700) < 0) {
+					erofs_err("failed to set permissions: %s (%s)",
+						  fsckcfg.extract_path, strerror(errno));
 				return -errno;
 			}
 			tryagain = false;
@@ -680,13 +689,13 @@ int main(int argc, char **argv)
 
 	erofs_init_configure();
 
-	fsckcfg.corrupted = false;
 	fsckcfg.print_comp_ratio = false;
 	fsckcfg.check_decomp = false;
 	fsckcfg.extract_path = NULL;
 	fsckcfg.extract_pos = 0;
-	fsckcfg.logical_blocks = 0;
-	fsckcfg.physical_blocks = 0;
+	fsckcfg.overwrite = false;
+	fsckcfg.preserve_owner = false;
+	fsckcfg.preserve_perms = false;
 
 	err = erofsfsck_parse_options_cfg(argc, argv);
 	if (err) {
@@ -696,6 +705,9 @@ int main(int argc, char **argv)
 	}
 
 	fsckcfg.umask = umask(0);
+	fsckcfg.logical_blocks = 0;
+	fsckcfg.physical_blocks = 0;
+	fsckcfg.corrupted = false;
 
 	err = dev_open_ro(cfg.c_img_path);
 	if (err) {
@@ -725,7 +737,7 @@ int main(int argc, char **argv)
 		if (!fsckcfg.extract_path)
 			erofs_info("No error found");
 		else
-			erofs_info("Extract data successfully");
+			erofs_info("Filesystem extracted successfully");
 
 		if (fsckcfg.print_comp_ratio) {
 			double comp_ratio =
-- 
2.30.2

Re: [PATCH] erofs-utils: fsck: fix issues related to --extract=X

[Gao Xiang]

Hi Igor,

On Fri, Jan 21, 2022 at 02:31:16AM +0200, Igor Ostapenko wrote:
> From: Igor Eisberg <igoreisberg@gmail.com>
> 
> Have to disagree with some changes made to my original patch.

Thanks for the patch. If you had opinion with the original patch,
I'm very happy that if you could comment on the original patch
at that time in public. erofs-utils works for long-term plans, 
we need to consider more scenarios in advance (I admit I cannot
think carefully very well as well). Sorry about slight modification
your original patch.

> 1) Using tar options makes no sense here, since tar has default
>    behaviors set for normal user (uses user's owner ID + perms)
>    vs. root user (preserve original owner IDs + perms by default),

So we can have such behavior as well (it seems many extractors
have such behavior, such as tar and unsquashfs.)

>    and the options were there to override that preset behavior.
>    fsck doesn't have any default behavior set, and the default
>    values for preserve_owner and preserve_perms were left for
>    the compiler to decide. This change sets the default values
>    to false explicitly.
>    "--no-same-owner" and "--no-same-permissions" are completely
>    redundant in fsck's case, unlike tar.

No matter what default behaviors we use now, it should have both
options with opposite behaviors. Otherwise, if some user write
a script and the default behavior then changes, it will break
completely (because such users don't need to rely on the default
behavior actually.)

>    * "--same-owner" and "--same-permissions" were renamed to
>      "--preserve-owner" and "--preserve-perms" to better represent
>      what these options do, the word "same" is ambiguous and tells
>      nothing to the user ("same" to what?).

I'm either ok with "--preserve-owner" and "--preserve-perms"
words.

>    * Added "--preserve" as a shortcut for both options in one.

Ok, if you like, I'm fine with this.

>    * Fixed option descriptions as they had typos and were too
>      ambiguous ("extract information" to where? separate file?).
> 2) Errors for chmod 0700 were not logged, failures were silent.

Yeah, thanks! (but to make sure that each line (except for the print
message) is no more than 80 chars.)

> 3) --extract=/ should fail with a proper error due to it being
>    dangerous if used with sudo.

I tend to disagree such idea, since add (or overwrite) files
to / may be useful. Also, tar can do like this, why not
fsck.erofs? If you think that is great dangerous, we could add
another '--force' option together with this.

Thanks,
Gao Xiang

> 
> Signed-off-by: Igor Ostapenko <igoreisberg@gmail.com>
> ---
>  fsck/main.c | 62 ++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 37 insertions(+), 25 deletions(-)
> 
> diff --git a/fsck/main.c b/fsck/main.c
> index 14534b9..e2f4157 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -18,15 +18,17 @@
>  static int erofsfsck_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
>  
>  struct erofsfsck_cfg {
> -	bool corrupted;
>  	bool print_comp_ratio;
>  	bool check_decomp;
>  	char *extract_path;
>  	size_t extract_pos;
> -	bool overwrite, preserve_owner, preserve_perms;
> +	bool overwrite;
> +	bool preserve_owner;
> +	bool preserve_perms;
>  	mode_t umask;
>  	u64 physical_blocks;
>  	u64 logical_blocks;
> +	bool corrupted;
>  };
>  static struct erofsfsck_cfg fsckcfg;
>  
> @@ -34,11 +36,10 @@ static struct option long_options[] = {
>  	{"help", no_argument, 0, 1},
>  	{"extract", optional_argument, 0, 2},
>  	{"device", required_argument, 0, 3},
> -	{"no-same-owner", no_argument, 0, 4},
> -	{"no-same-permissions", no_argument, 0, 5},
> -	{"same-owner", no_argument, 0, 6},
> -	{"same-permissions", no_argument, 0, 7},
> -	{"overwrite", no_argument, 0, 8},
> +	{"overwrite", no_argument, 0, 4},
> +	{"preserve", no_argument, 0, 5},
> +	{"preserve-owner", no_argument, 0, 6},
> +	{"preserve-perms", no_argument, 0, 7},
>  	{0, 0, 0, 0},
>  };
>  
> @@ -66,11 +67,10 @@ static void usage(void)
>  	      " --extract[=X]          check if all files are well encoded, optionally extract to X\n"
>  	      " --help                 display this help and exit\n"
>  	      "\nExtraction options (--extract=X is required):\n"
> -	      " --no-same-owner        extract files as yourself\n"
> -	      " --no-same-permissions  apply the user's umask when extracting permission\n"
> -	      " --same-permissions     extract information about file permissions\n"
> -	      " --same-owner           extract files about the file ownerships\n"
> -	      " --overwrite            if file already exists then overwrite\n"
> +	      " --overwrite            overwrite files that already exist\n"
> +	      " --preserve             extract with original ownerships and permissions\n"
> +	      " --preserve-owner       extract with original ownerships only\n"
> +	      " --preserve-perms       extract with original permissions only\n"
>  	      "\nSupported algorithms are: ", stderr);
>  	print_available_decompressors(stderr, ", ");
>  }
> @@ -112,10 +112,16 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
>  				if (len == 0)
>  					return -EINVAL;
>  
> -				/* remove trailing slashes except root */
> -				while (len > 1 && optarg[len - 1] == '/')
> +				/* remove trailing slashes */
> +				while (len > 0 && optarg[len - 1] == '/')
>  					len--;
>  
> +				/* extracting directly to root is dangerous */
> +				if (len == 0) {
> +					erofs_err("invalid extract path: /");
> +					return -EINVAL;
> +				}
> +
>  				fsckcfg.extract_path = malloc(PATH_MAX);
>  				if (!fsckcfg.extract_path)
>  					return -ENOMEM;
> @@ -131,10 +137,11 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
>  			++sbi.extra_devices;
>  			break;
>  		case 4:
> -			fsckcfg.preserve_owner = false;
> +			fsckcfg.overwrite = true;
>  			break;
>  		case 5:
> -			fsckcfg.preserve_perms = false;
> +			fsckcfg.preserve_owner = true;
> +			fsckcfg.preserve_perms = true;
>  			break;
>  		case 6:
>  			fsckcfg.preserve_owner = true;
> @@ -142,9 +149,6 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
>  		case 7:
>  			fsckcfg.preserve_perms = true;
>  			break;
> -		case 8:
> -			fsckcfg.overwrite = true;
> -			break;
>  		default:
>  			return -EINVAL;
>  		}
> @@ -465,7 +469,7 @@ static inline int erofs_extract_dir(struct erofs_inode *inode)
>  		struct stat st;
>  
>  		if (errno != EEXIST) {
> -			erofs_err("failed to create directory %s: %s",
> +			erofs_err("failed to create directory: %s (%s)",
>  				  fsckcfg.extract_path, strerror(errno));
>  			return -errno;
>  		}
> @@ -481,8 +485,11 @@ static inline int erofs_extract_dir(struct erofs_inode *inode)
>  		 * Try to change permissions of existing directory so
>  		 * that we can write to it
>  		 */
> -		if (chmod(fsckcfg.extract_path, 0700) < 0)
> +		if (chmod(fsckcfg.extract_path, 0700) < 0) {
> +			erofs_err("failed to set permissions: %s (%s)",
> +				  fsckcfg.extract_path, strerror(errno));
>  			return -errno;
> +		}
>  	}
>  	return 0;
>  }
> @@ -510,6 +517,8 @@ again:
>  				}
>  			} else if (errno == EACCES &&
>  				   chmod(fsckcfg.extract_path, 0700) < 0) {
> +					erofs_err("failed to set permissions: %s (%s)",
> +						  fsckcfg.extract_path, strerror(errno));
>  				return -errno;
>  			}
>  			tryagain = false;
> @@ -680,13 +689,13 @@ int main(int argc, char **argv)
>  
>  	erofs_init_configure();
>  
> -	fsckcfg.corrupted = false;
>  	fsckcfg.print_comp_ratio = false;
>  	fsckcfg.check_decomp = false;
>  	fsckcfg.extract_path = NULL;
>  	fsckcfg.extract_pos = 0;
> -	fsckcfg.logical_blocks = 0;
> -	fsckcfg.physical_blocks = 0;
> +	fsckcfg.overwrite = false;
> +	fsckcfg.preserve_owner = false;
> +	fsckcfg.preserve_perms = false;
>  
>  	err = erofsfsck_parse_options_cfg(argc, argv);
>  	if (err) {
> @@ -696,6 +705,9 @@ int main(int argc, char **argv)
>  	}
>  
>  	fsckcfg.umask = umask(0);
> +	fsckcfg.logical_blocks = 0;
> +	fsckcfg.physical_blocks = 0;
> +	fsckcfg.corrupted = false;
>  
>  	err = dev_open_ro(cfg.c_img_path);
>  	if (err) {
> @@ -725,7 +737,7 @@ int main(int argc, char **argv)
>  		if (!fsckcfg.extract_path)
>  			erofs_info("No error found");
>  		else
> -			erofs_info("Extract data successfully");
> +			erofs_info("Filesystem extracted successfully");
>  
>  		if (fsckcfg.print_comp_ratio) {
>  			double comp_ratio =
> -- 
> 2.30.2

Re: [PATCH] erofs-utils: fsck: fix issues related to --extract=X

[Gao Xiang]

ping? would you mind submitting another patch or make a reply
of this so I can hear your idea and take some part of it?

On Fri, Jan 21, 2022 at 09:47:16AM +0800, Gao Xiang wrote:
> Hi Igor,
> 
> On Fri, Jan 21, 2022 at 02:31:16AM +0200, Igor Ostapenko wrote:
> > From: Igor Eisberg <igoreisberg@gmail.com>
> > 
> > Have to disagree with some changes made to my original patch.
> 
> Thanks for the patch. If you had opinion with the original patch,
> I'm very happy that if you could comment on the original patch
> at that time in public. erofs-utils works for long-term plans, 
> we need to consider more scenarios in advance (I admit I cannot
> think carefully very well as well). Sorry about slight modification
> your original patch.
> 
> > 1) Using tar options makes no sense here, since tar has default
> >    behaviors set for normal user (uses user's owner ID + perms)
> >    vs. root user (preserve original owner IDs + perms by default),
> 
> So we can have such behavior as well (it seems many extractors
> have such behavior, such as tar and unsquashfs.)
> 
> >    and the options were there to override that preset behavior.
> >    fsck doesn't have any default behavior set, and the default
> >    values for preserve_owner and preserve_perms were left for
> >    the compiler to decide. This change sets the default values
> >    to false explicitly.
> >    "--no-same-owner" and "--no-same-permissions" are completely
> >    redundant in fsck's case, unlike tar.
> 
> No matter what default behaviors we use now, it should have both
> options with opposite behaviors. Otherwise, if some user write
> a script and the default behavior then changes, it will break
> completely (because such users don't need to rely on the default
> behavior actually.)
> 
> >    * "--same-owner" and "--same-permissions" were renamed to
> >      "--preserve-owner" and "--preserve-perms" to better represent
> >      what these options do, the word "same" is ambiguous and tells
> >      nothing to the user ("same" to what?).
> 
> I'm either ok with "--preserve-owner" and "--preserve-perms"
> words.
> 
> >    * Added "--preserve" as a shortcut for both options in one.
> 
> Ok, if you like, I'm fine with this.
> 
> >    * Fixed option descriptions as they had typos and were too
> >      ambiguous ("extract information" to where? separate file?).
> > 2) Errors for chmod 0700 were not logged, failures were silent.
> 
> Yeah, thanks! (but to make sure that each line (except for the print
> message) is no more than 80 chars.)
> 
> > 3) --extract=/ should fail with a proper error due to it being
> >    dangerous if used with sudo.
> 
> I tend to disagree such idea, since add (or overwrite) files
> to / may be useful. Also, tar can do like this, why not
> fsck.erofs? If you think that is great dangerous, we could add
> another '--force' option together with this.
> 
> Thanks,
> Gao Xiang
> 
> > 
> > Signed-off-by: Igor Ostapenko <igoreisberg@gmail.com>
> > ---
> >  fsck/main.c | 62 ++++++++++++++++++++++++++++++++---------------------
> >  1 file changed, 37 insertions(+), 25 deletions(-)
> > 
> > diff --git a/fsck/main.c b/fsck/main.c
> > index 14534b9..e2f4157 100644
> > --- a/fsck/main.c
> > +++ b/fsck/main.c
> > @@ -18,15 +18,17 @@
> >  static int erofsfsck_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
> >  
> >  struct erofsfsck_cfg {
> > -	bool corrupted;
> >  	bool print_comp_ratio;
> >  	bool check_decomp;
> >  	char *extract_path;
> >  	size_t extract_pos;
> > -	bool overwrite, preserve_owner, preserve_perms;
> > +	bool overwrite;
> > +	bool preserve_owner;
> > +	bool preserve_perms;
> >  	mode_t umask;
> >  	u64 physical_blocks;
> >  	u64 logical_blocks;
> > +	bool corrupted;
> >  };
> >  static struct erofsfsck_cfg fsckcfg;
> >  
> > @@ -34,11 +36,10 @@ static struct option long_options[] = {
> >  	{"help", no_argument, 0, 1},
> >  	{"extract", optional_argument, 0, 2},
> >  	{"device", required_argument, 0, 3},
> > -	{"no-same-owner", no_argument, 0, 4},
> > -	{"no-same-permissions", no_argument, 0, 5},
> > -	{"same-owner", no_argument, 0, 6},
> > -	{"same-permissions", no_argument, 0, 7},
> > -	{"overwrite", no_argument, 0, 8},
> > +	{"overwrite", no_argument, 0, 4},
> > +	{"preserve", no_argument, 0, 5},
> > +	{"preserve-owner", no_argument, 0, 6},
> > +	{"preserve-perms", no_argument, 0, 7},
> >  	{0, 0, 0, 0},
> >  };
> >  
> > @@ -66,11 +67,10 @@ static void usage(void)
> >  	      " --extract[=X]          check if all files are well encoded, optionally extract to X\n"
> >  	      " --help                 display this help and exit\n"
> >  	      "\nExtraction options (--extract=X is required):\n"
> > -	      " --no-same-owner        extract files as yourself\n"
> > -	      " --no-same-permissions  apply the user's umask when extracting permission\n"
> > -	      " --same-permissions     extract information about file permissions\n"
> > -	      " --same-owner           extract files about the file ownerships\n"
> > -	      " --overwrite            if file already exists then overwrite\n"
> > +	      " --overwrite            overwrite files that already exist\n"
> > +	      " --preserve             extract with original ownerships and permissions\n"
> > +	      " --preserve-owner       extract with original ownerships only\n"
> > +	      " --preserve-perms       extract with original permissions only\n"
> >  	      "\nSupported algorithms are: ", stderr);
> >  	print_available_decompressors(stderr, ", ");
> >  }
> > @@ -112,10 +112,16 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
> >  				if (len == 0)
> >  					return -EINVAL;
> >  
> > -				/* remove trailing slashes except root */
> > -				while (len > 1 && optarg[len - 1] == '/')
> > +				/* remove trailing slashes */
> > +				while (len > 0 && optarg[len - 1] == '/')
> >  					len--;
> >  
> > +				/* extracting directly to root is dangerous */
> > +				if (len == 0) {
> > +					erofs_err("invalid extract path: /");
> > +					return -EINVAL;
> > +				}
> > +
> >  				fsckcfg.extract_path = malloc(PATH_MAX);
> >  				if (!fsckcfg.extract_path)
> >  					return -ENOMEM;
> > @@ -131,10 +137,11 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
> >  			++sbi.extra_devices;
> >  			break;
> >  		case 4:
> > -			fsckcfg.preserve_owner = false;
> > +			fsckcfg.overwrite = true;
> >  			break;
> >  		case 5:
> > -			fsckcfg.preserve_perms = false;
> > +			fsckcfg.preserve_owner = true;
> > +			fsckcfg.preserve_perms = true;
> >  			break;
> >  		case 6:
> >  			fsckcfg.preserve_owner = true;
> > @@ -142,9 +149,6 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
> >  		case 7:
> >  			fsckcfg.preserve_perms = true;
> >  			break;
> > -		case 8:
> > -			fsckcfg.overwrite = true;
> > -			break;
> >  		default:
> >  			return -EINVAL;
> >  		}
> > @@ -465,7 +469,7 @@ static inline int erofs_extract_dir(struct erofs_inode *inode)
> >  		struct stat st;
> >  
> >  		if (errno != EEXIST) {
> > -			erofs_err("failed to create directory %s: %s",
> > +			erofs_err("failed to create directory: %s (%s)",
> >  				  fsckcfg.extract_path, strerror(errno));
> >  			return -errno;
> >  		}
> > @@ -481,8 +485,11 @@ static inline int erofs_extract_dir(struct erofs_inode *inode)
> >  		 * Try to change permissions of existing directory so
> >  		 * that we can write to it
> >  		 */
> > -		if (chmod(fsckcfg.extract_path, 0700) < 0)
> > +		if (chmod(fsckcfg.extract_path, 0700) < 0) {
> > +			erofs_err("failed to set permissions: %s (%s)",
> > +				  fsckcfg.extract_path, strerror(errno));
> >  			return -errno;
> > +		}
> >  	}
> >  	return 0;
> >  }
> > @@ -510,6 +517,8 @@ again:
> >  				}
> >  			} else if (errno == EACCES &&
> >  				   chmod(fsckcfg.extract_path, 0700) < 0) {
> > +					erofs_err("failed to set permissions: %s (%s)",
> > +						  fsckcfg.extract_path, strerror(errno));
> >  				return -errno;
> >  			}
> >  			tryagain = false;
> > @@ -680,13 +689,13 @@ int main(int argc, char **argv)
> >  
> >  	erofs_init_configure();
> >  
> > -	fsckcfg.corrupted = false;
> >  	fsckcfg.print_comp_ratio = false;
> >  	fsckcfg.check_decomp = false;
> >  	fsckcfg.extract_path = NULL;
> >  	fsckcfg.extract_pos = 0;
> > -	fsckcfg.logical_blocks = 0;
> > -	fsckcfg.physical_blocks = 0;
> > +	fsckcfg.overwrite = false;
> > +	fsckcfg.preserve_owner = false;
> > +	fsckcfg.preserve_perms = false;
> >  
> >  	err = erofsfsck_parse_options_cfg(argc, argv);
> >  	if (err) {
> > @@ -696,6 +705,9 @@ int main(int argc, char **argv)
> >  	}
> >  
> >  	fsckcfg.umask = umask(0);
> > +	fsckcfg.logical_blocks = 0;
> > +	fsckcfg.physical_blocks = 0;
> > +	fsckcfg.corrupted = false;
> >  
> >  	err = dev_open_ro(cfg.c_img_path);
> >  	if (err) {
> > @@ -725,7 +737,7 @@ int main(int argc, char **argv)
> >  		if (!fsckcfg.extract_path)
> >  			erofs_info("No error found");
> >  		else
> > -			erofs_info("Extract data successfully");
> > +			erofs_info("Filesystem extracted successfully");
> >  
> >  		if (fsckcfg.print_comp_ratio) {
> >  			double comp_ratio =
> > -- 
> > 2.30.2

Re: [PATCH] erofs-utils: fsck: fix issues related to --extract=X

[Gao Xiang]

On Fri, Jan 28, 2022 at 06:00:36PM +0200, Igor Ostapenko wrote:
> From: Igor Eisberg <igoreisberg@gmail.com>
> 
> Fix "--[no-]preserve[-owner/-perms] must be used..." error always thrown
> for superuser when fsck used without --extract=X, due to default values
> for preserve_owner/preserve_perms being already pre-set to true,
> as if the --preserve option was explicitly given by the user.
> 
> Signed-off-by: Igor Ostapenko <igoreisberg@gmail.com>

Folded, thanks.

[PATCH] erofs-utils: fsck: fix issues related to --extract=X

[Igor Ostapenko]

From: Igor Eisberg <igoreisberg@gmail.com>

Fix "--[no-]preserve[-owner/-perms] must be used..." error always thrown
for superuser when fsck used without --extract=X, due to default values
for preserve_owner/preserve_perms being already pre-set to true,
as if the --preserve option was explicitly given by the user.

Signed-off-by: Igor Ostapenko <igoreisberg@gmail.com>
---
 fsck/main.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/fsck/main.c b/fsck/main.c
index 5667f2a..9e3756d 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -95,7 +95,7 @@ static void erofsfsck_print_version(void)
 static int erofsfsck_parse_options_cfg(int argc, char **argv)
 {
 	int opt, ret;
-	bool no_preserve_owner = false, no_preserve_perms = false;
+	bool has_opt_preserve = false;
 
 	while ((opt = getopt_long(argc, argv, "Vd:p",
 				  long_options, NULL)) != -1) {
@@ -154,27 +154,27 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
 			break;
 		case 6:
 			fsckcfg.preserve_owner = fsckcfg.preserve_perms = true;
-			no_preserve_owner = no_preserve_perms = false;
+			has_opt_preserve = true;
 			break;
 		case 7:
 			fsckcfg.preserve_owner = true;
-			no_preserve_owner = false;
+			has_opt_preserve = true;
 			break;
 		case 8:
 			fsckcfg.preserve_perms = true;
-			no_preserve_perms = false;
+			has_opt_preserve = true;
 			break;
 		case 9:
 			fsckcfg.preserve_owner = fsckcfg.preserve_perms = false;
-			no_preserve_owner = no_preserve_perms = true;
+			has_opt_preserve = true;
 			break;
 		case 10:
 			fsckcfg.preserve_owner = false;
-			no_preserve_owner = true;
+			has_opt_preserve = true;
 			break;
 		case 11:
 			fsckcfg.preserve_perms = false;
-			no_preserve_perms = true;
+			has_opt_preserve = true;
 			break;
 		default:
 			return -EINVAL;
@@ -195,8 +195,7 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
 			erofs_err("--overwrite must be used together with --extract=X");
 			return -EINVAL;
 		}
-		if (fsckcfg.preserve_owner || fsckcfg.preserve_perms ||
-			  no_preserve_owner || no_preserve_perms) {
+		if (has_opt_preserve) {
 			erofs_err("--[no-]preserve[-owner/-perms] must be used together with --extract=X");
 			return -EINVAL;
 		}
-- 
2.30.2

Re: [PATCH] erofs-utils: fsck: fix issues related to --extract=X

[Gao Xiang]

On Fri, Jan 28, 2022 at 06:50:18AM +0200, Igor Ostapenko wrote:
> From: Igor Eisberg <igoreisberg@gmail.com>
> 
> Fix compile
> 
> Signed-off-by: Igor Ostapenko <igoreisberg@gmail.com>

Many thanks, I will fold these two patches into the first one
for applying.

Thanks,
Gao Xiang

> ---
>  fsck/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fsck/main.c b/fsck/main.c
> index 2f13870..6f17d0e 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -196,7 +196,7 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
>  			return -EINVAL;
>  		}
>  		if (fsckcfg.preserve_owner || fsckcfg.preserve_perms ||
> -			  fsckcfg.no_preserve_owner || fsckcfg.no_preserve_perms) {
> +			  no_preserve_owner || no_preserve_perms) {
>  			erofs_err("--[no-]preserve[-owner/-perms] must be used together with --extract=X");
>  			return -EINVAL;
>  		}
> -- 
> 2.30.2

[PATCH] erofs-utils: fsck: fix issues related to --extract=X

[Igor Ostapenko]

From: Igor Eisberg <igoreisberg@gmail.com>

Fix compile

Signed-off-by: Igor Ostapenko <igoreisberg@gmail.com>
---
 fsck/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fsck/main.c b/fsck/main.c
index 2f13870..6f17d0e 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -196,7 +196,7 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
 			return -EINVAL;
 		}
 		if (fsckcfg.preserve_owner || fsckcfg.preserve_perms ||
-			  fsckcfg.no_preserve_owner || fsckcfg.no_preserve_perms) {
+			  no_preserve_owner || no_preserve_perms) {
 			erofs_err("--[no-]preserve[-owner/-perms] must be used together with --extract=X");
 			return -EINVAL;
 		}
-- 
2.30.2

[PATCH] erofs-utils: fsck: fix issues related to --extract=X

[Igor Ostapenko]

From: Igor Eisberg <igoreisberg@gmail.com>

* Moved no_preserve_owner/no_preserve_perms to local scope.
* Added missing return -EINVAL.

Signed-off-by: Igor Ostapenko <igoreisberg@gmail.com>
---
 fsck/main.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/fsck/main.c b/fsck/main.c
index 9080a66..2f13870 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -31,8 +31,6 @@ struct erofsfsck_cfg {
 	bool overwrite;
 	bool preserve_owner;
 	bool preserve_perms;
-	bool no_preserve_owner;
-	bool no_preserve_perms;
 };
 static struct erofsfsck_cfg fsckcfg;
 
@@ -97,6 +95,7 @@ static void erofsfsck_print_version(void)
 static int erofsfsck_parse_options_cfg(int argc, char **argv)
 {
 	int opt, ret;
+	bool no_preserve_owner = false, no_preserve_perms = false;
 
 	while ((opt = getopt_long(argc, argv, "Vd:p",
 				  long_options, NULL)) != -1) {
@@ -154,32 +153,28 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
 			fsckcfg.overwrite = true;
 			break;
 		case 6:
-			fsckcfg.preserve_owner = true;
-			fsckcfg.preserve_perms = true;
-			fsckcfg.no_preserve_owner = false;
-			fsckcfg.no_preserve_perms = false;
+			fsckcfg.preserve_owner = fsckcfg.preserve_perms = true;
+			no_preserve_owner = no_preserve_perms = false;
 			break;
 		case 7:
 			fsckcfg.preserve_owner = true;
-			fsckcfg.no_preserve_owner = false;
+			no_preserve_owner = false;
 			break;
 		case 8:
 			fsckcfg.preserve_perms = true;
-			fsckcfg.no_preserve_perms = false;
+			no_preserve_perms = false;
 			break;
 		case 9:
-			fsckcfg.no_preserve_owner = true;
-			fsckcfg.no_preserve_perms = true;
-			fsckcfg.preserve_owner = false;
-			fsckcfg.preserve_perms = false;
+			fsckcfg.preserve_owner = fsckcfg.preserve_perms = false;
+			no_preserve_owner = no_preserve_perms = true;
 			break;
 		case 10:
-			fsckcfg.no_preserve_owner = true;
 			fsckcfg.preserve_owner = false;
+			no_preserve_owner = true;
 			break;
 		case 11:
-			fsckcfg.no_preserve_perms = true;
 			fsckcfg.preserve_perms = false;
+			no_preserve_perms = true;
 			break;
 		default:
 			return -EINVAL;
@@ -192,13 +187,19 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
 			return -EINVAL;
 		}
 	} else {
-		if (fsckcfg.force)
+		if (fsckcfg.force) {
 			erofs_err("--force must be used together with --extract=X");
-		if (fsckcfg.overwrite)
+			return -EINVAL;
+		}
+		if (fsckcfg.overwrite) {
 			erofs_err("--overwrite must be used together with --extract=X");
+			return -EINVAL;
+		}
 		if (fsckcfg.preserve_owner || fsckcfg.preserve_perms ||
-			  fsckcfg.no_preserve_owner || fsckcfg.no_preserve_perms)
+			  fsckcfg.no_preserve_owner || fsckcfg.no_preserve_perms) {
 			erofs_err("--[no-]preserve[-owner/-perms] must be used together with --extract=X");
+			return -EINVAL;
+		}
 	}
 
 	if (optind >= argc)
-- 
2.30.2

Re: [PATCH] erofs-utils: fsck: fix issues related to --extract=X

[Gao Xiang]

Hi Igor,

On Fri, Jan 28, 2022 at 06:05:11AM +0200, Igor Ostapenko wrote:
> From: Igor Eisberg <igoreisberg@gmail.com>
> 
> * Added tar-like default behaviors for --[no-]preserve options:
>   normal user - uses user's owner ID + umask on perms by default;
>   root user - preserve original owner IDs + perms by default;
>   and add appropriate error message when used without --extract=X.
> * "--[no-]same-owner" and "--[no-]same-permissions" were renamed
>   to "--[no-]preserve-owner" and "--[no-]preserve-perms" to
>   better represent what these options do, the word "same" is
>   ambiguous and tells nothing to the user ("same" to what?).
> * Added "--[no-]preserve" as shortcuts for both options in one.
> * Fixed option descriptions as they had typos and were too ambiguous
>   ("extract information" to where? separate file?).
> * Added --force option to allow extracting directly to root path.
> 
> Signed-off-by: Igor Ostapenko <igoreisberg@gmail.com>
> ---
>  fsck/main.c | 104 ++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 77 insertions(+), 27 deletions(-)
> 
> diff --git a/fsck/main.c b/fsck/main.c
> index 92e0c76..9080a66 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -18,15 +18,21 @@
>  static int erofsfsck_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
>  
>  struct erofsfsck_cfg {
> +	bool superuser;
> +	mode_t umask;
>  	bool corrupted;
> +	u64 physical_blocks;
> +	u64 logical_blocks;
>  	bool print_comp_ratio;
>  	bool check_decomp;
>  	char *extract_path;
>  	size_t extract_pos;
> -	bool overwrite, preserve_owner, preserve_perms;
> -	mode_t umask;
> -	u64 physical_blocks;
> -	u64 logical_blocks;
> +	bool force;
> +	bool overwrite;
> +	bool preserve_owner;
> +	bool preserve_perms;
> +	bool no_preserve_owner;
> +	bool no_preserve_perms;

Thanks for the patch, just a minor comment.

How about moving no_preserve_owner, no_preserve_perms to
erofsfsck_parse_options_cfg as local varibles instead?

Otherwise looks good to me.

Thanks,
Gao Xiang

>  };
>  static struct erofsfsck_cfg fsckcfg;
>  
> @@ -34,11 +40,14 @@ static struct option long_options[] = {
>  	{"help", no_argument, 0, 1},
>  	{"extract", optional_argument, 0, 2},
>  	{"device", required_argument, 0, 3},
> -	{"no-same-owner", no_argument, 0, 4},
> -	{"no-same-permissions", no_argument, 0, 5},
> -	{"same-owner", no_argument, 0, 6},
> -	{"same-permissions", no_argument, 0, 7},
> -	{"overwrite", no_argument, 0, 8},
> +	{"force", no_argument, 0, 4},
> +	{"overwrite", no_argument, 0, 5},
> +	{"preserve", no_argument, 0, 6},
> +	{"preserve-owner", no_argument, 0, 7},
> +	{"preserve-perms", no_argument, 0, 8},
> +	{"no-preserve", no_argument, 0, 9},
> +	{"no-preserve-owner", no_argument, 0, 10},
> +	{"no-preserve-perms", no_argument, 0, 11},
>  	{0, 0, 0, 0},
>  };
>  
> @@ -66,11 +75,16 @@ static void usage(void)
>  	      " --extract[=X]          check if all files are well encoded, optionally extract to X\n"
>  	      " --help                 display this help and exit\n"
>  	      "\nExtraction options (--extract=X is required):\n"
> -	      " --no-same-owner        extract files as yourself\n"
> -	      " --no-same-permissions  apply the user's umask when extracting permission\n"
> -	      " --same-permissions     extract information about file permissions\n"
> -	      " --same-owner           extract files about the file ownerships\n"
> -	      " --overwrite            if file already exists then overwrite\n"
> +	      " --force                allow extracting to root\n"
> +	      " --overwrite            overwrite files that already exist\n"
> +	      " --preserve             extract with the same ownership and permissions as on the filesystem\n"
> +	      "                        (default for superuser)\n"
> +	      " --preserve-owner       extract with the same ownership as on the filesystem\n"
> +	      " --preserve-perms       extract with the same permissions as on the filesystem\n"
> +	      " --no-preserve          extract as yourself and apply user's umask on permissions\n"
> +	      "                        (default for ordinary users)\n"
> +	      " --no-preserve-owner    extract as yourself\n"
> +	      " --no-preserve-perms    apply user's umask when extracting permissions\n"
>  	      "\nSupported algorithms are: ", stderr);
>  	print_available_decompressors(stderr, ", ");
>  }
> @@ -121,6 +135,9 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
>  					return -ENOMEM;
>  				strncpy(fsckcfg.extract_path, optarg, len);
>  				fsckcfg.extract_path[len] = '\0';
> +				/* if path is root, start writing from position 0 */
> +				if (len == 1 && fsckcfg.extract_path[0] == '/')
> +					len = 0;
>  				fsckcfg.extract_pos = len;
>  			}
>  			break;
> @@ -131,33 +148,62 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
>  			++sbi.extra_devices;
>  			break;
>  		case 4:
> -			fsckcfg.preserve_owner = false;
> +			fsckcfg.force = true;
>  			break;
>  		case 5:
> -			fsckcfg.preserve_perms = false;
> +			fsckcfg.overwrite = true;
>  			break;
>  		case 6:
>  			fsckcfg.preserve_owner = true;
> +			fsckcfg.preserve_perms = true;
> +			fsckcfg.no_preserve_owner = false;
> +			fsckcfg.no_preserve_perms = false;
>  			break;
>  		case 7:
> -			fsckcfg.preserve_perms = true;
> +			fsckcfg.preserve_owner = true;
> +			fsckcfg.no_preserve_owner = false;
>  			break;
>  		case 8:
> -			fsckcfg.overwrite = true;
> +			fsckcfg.preserve_perms = true;
> +			fsckcfg.no_preserve_perms = false;
> +			break;
> +		case 9:
> +			fsckcfg.no_preserve_owner = true;
> +			fsckcfg.no_preserve_perms = true;
> +			fsckcfg.preserve_owner = false;
> +			fsckcfg.preserve_perms = false;
> +			break;
> +		case 10:
> +			fsckcfg.no_preserve_owner = true;
> +			fsckcfg.preserve_owner = false;
> +			break;
> +		case 11:
> +			fsckcfg.no_preserve_perms = true;
> +			fsckcfg.preserve_perms = false;
>  			break;
>  		default:
>  			return -EINVAL;
>  		}
>  	}
>  
> +	if (fsckcfg.extract_path) {
> +		if (!fsckcfg.extract_pos && !fsckcfg.force) {
> +			erofs_err("--extract=/ must be used together with --force");
> +			return -EINVAL;
> +		}
> +	} else {
> +		if (fsckcfg.force)
> +			erofs_err("--force must be used together with --extract=X");
> +		if (fsckcfg.overwrite)
> +			erofs_err("--overwrite must be used together with --extract=X");
> +		if (fsckcfg.preserve_owner || fsckcfg.preserve_perms ||
> +			  fsckcfg.no_preserve_owner || fsckcfg.no_preserve_perms)
> +			erofs_err("--[no-]preserve[-owner/-perms] must be used together with --extract=X");
> +	}
> +
>  	if (optind >= argc)
>  		return -EINVAL;
>  
> -	if (!fsckcfg.extract_path)
> -		if (fsckcfg.overwrite || fsckcfg.preserve_owner ||
> -		    fsckcfg.preserve_perms)
> -			return -EINVAL;
> -
>  	cfg.c_img_path = strdup(argv[optind++]);
>  	if (!cfg.c_img_path)
>  		return -ENOMEM;
> @@ -680,13 +726,19 @@ int main(int argc, char **argv)
>  
>  	erofs_init_configure();
>  
> +	fsckcfg.superuser = geteuid() == 0;
> +	fsckcfg.umask = umask(0);
>  	fsckcfg.corrupted = false;
> +	fsckcfg.logical_blocks = 0;
> +	fsckcfg.physical_blocks = 0;
>  	fsckcfg.print_comp_ratio = false;
>  	fsckcfg.check_decomp = false;
>  	fsckcfg.extract_path = NULL;
>  	fsckcfg.extract_pos = 0;
> -	fsckcfg.logical_blocks = 0;
> -	fsckcfg.physical_blocks = 0;
> +	fsckcfg.force = false;
> +	fsckcfg.overwrite = false;
> +	fsckcfg.preserve_owner = fsckcfg.superuser;
> +	fsckcfg.preserve_perms = fsckcfg.superuser;
>  
>  	err = erofsfsck_parse_options_cfg(argc, argv);
>  	if (err) {
> @@ -695,8 +747,6 @@ int main(int argc, char **argv)
>  		goto exit;
>  	}
>  
> -	fsckcfg.umask = umask(0);
> -
>  	err = dev_open_ro(cfg.c_img_path);
>  	if (err) {
>  		erofs_err("failed to open image file");
> -- 
> 2.30.2

[PATCH] erofs-utils: fsck: fix issues related to --extract=X

[Igor Ostapenko]

From: Igor Eisberg <igoreisberg@gmail.com>

* Added tar-like default behaviors for --[no-]preserve options:
  normal user - uses user's owner ID + umask on perms by default;
  root user - preserve original owner IDs + perms by default;
  and add appropriate error message when used without --extract=X.
* "--[no-]same-owner" and "--[no-]same-permissions" were renamed
  to "--[no-]preserve-owner" and "--[no-]preserve-perms" to
  better represent what these options do, the word "same" is
  ambiguous and tells nothing to the user ("same" to what?).
* Added "--[no-]preserve" as shortcuts for both options in one.
* Fixed option descriptions as they had typos and were too ambiguous
  ("extract information" to where? separate file?).
* Added --force option to allow extracting directly to root path.

Signed-off-by: Igor Ostapenko <igoreisberg@gmail.com>
---
 fsck/main.c | 104 ++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 77 insertions(+), 27 deletions(-)

diff --git a/fsck/main.c b/fsck/main.c
index 92e0c76..9080a66 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -18,15 +18,21 @@
 static int erofsfsck_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
 
 struct erofsfsck_cfg {
+	bool superuser;
+	mode_t umask;
 	bool corrupted;
+	u64 physical_blocks;
+	u64 logical_blocks;
 	bool print_comp_ratio;
 	bool check_decomp;
 	char *extract_path;
 	size_t extract_pos;
-	bool overwrite, preserve_owner, preserve_perms;
-	mode_t umask;
-	u64 physical_blocks;
-	u64 logical_blocks;
+	bool force;
+	bool overwrite;
+	bool preserve_owner;
+	bool preserve_perms;
+	bool no_preserve_owner;
+	bool no_preserve_perms;
 };
 static struct erofsfsck_cfg fsckcfg;
 
@@ -34,11 +40,14 @@ static struct option long_options[] = {
 	{"help", no_argument, 0, 1},
 	{"extract", optional_argument, 0, 2},
 	{"device", required_argument, 0, 3},
-	{"no-same-owner", no_argument, 0, 4},
-	{"no-same-permissions", no_argument, 0, 5},
-	{"same-owner", no_argument, 0, 6},
-	{"same-permissions", no_argument, 0, 7},
-	{"overwrite", no_argument, 0, 8},
+	{"force", no_argument, 0, 4},
+	{"overwrite", no_argument, 0, 5},
+	{"preserve", no_argument, 0, 6},
+	{"preserve-owner", no_argument, 0, 7},
+	{"preserve-perms", no_argument, 0, 8},
+	{"no-preserve", no_argument, 0, 9},
+	{"no-preserve-owner", no_argument, 0, 10},
+	{"no-preserve-perms", no_argument, 0, 11},
 	{0, 0, 0, 0},
 };
 
@@ -66,11 +75,16 @@ static void usage(void)
 	      " --extract[=X]          check if all files are well encoded, optionally extract to X\n"
 	      " --help                 display this help and exit\n"
 	      "\nExtraction options (--extract=X is required):\n"
-	      " --no-same-owner        extract files as yourself\n"
-	      " --no-same-permissions  apply the user's umask when extracting permission\n"
-	      " --same-permissions     extract information about file permissions\n"
-	      " --same-owner           extract files about the file ownerships\n"
-	      " --overwrite            if file already exists then overwrite\n"
+	      " --force                allow extracting to root\n"
+	      " --overwrite            overwrite files that already exist\n"
+	      " --preserve             extract with the same ownership and permissions as on the filesystem\n"
+	      "                        (default for superuser)\n"
+	      " --preserve-owner       extract with the same ownership as on the filesystem\n"
+	      " --preserve-perms       extract with the same permissions as on the filesystem\n"
+	      " --no-preserve          extract as yourself and apply user's umask on permissions\n"
+	      "                        (default for ordinary users)\n"
+	      " --no-preserve-owner    extract as yourself\n"
+	      " --no-preserve-perms    apply user's umask when extracting permissions\n"
 	      "\nSupported algorithms are: ", stderr);
 	print_available_decompressors(stderr, ", ");
 }
@@ -121,6 +135,9 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
 					return -ENOMEM;
 				strncpy(fsckcfg.extract_path, optarg, len);
 				fsckcfg.extract_path[len] = '\0';
+				/* if path is root, start writing from position 0 */
+				if (len == 1 && fsckcfg.extract_path[0] == '/')
+					len = 0;
 				fsckcfg.extract_pos = len;
 			}
 			break;
@@ -131,33 +148,62 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
 			++sbi.extra_devices;
 			break;
 		case 4:
-			fsckcfg.preserve_owner = false;
+			fsckcfg.force = true;
 			break;
 		case 5:
-			fsckcfg.preserve_perms = false;
+			fsckcfg.overwrite = true;
 			break;
 		case 6:
 			fsckcfg.preserve_owner = true;
+			fsckcfg.preserve_perms = true;
+			fsckcfg.no_preserve_owner = false;
+			fsckcfg.no_preserve_perms = false;
 			break;
 		case 7:
-			fsckcfg.preserve_perms = true;
+			fsckcfg.preserve_owner = true;
+			fsckcfg.no_preserve_owner = false;
 			break;
 		case 8:
-			fsckcfg.overwrite = true;
+			fsckcfg.preserve_perms = true;
+			fsckcfg.no_preserve_perms = false;
+			break;
+		case 9:
+			fsckcfg.no_preserve_owner = true;
+			fsckcfg.no_preserve_perms = true;
+			fsckcfg.preserve_owner = false;
+			fsckcfg.preserve_perms = false;
+			break;
+		case 10:
+			fsckcfg.no_preserve_owner = true;
+			fsckcfg.preserve_owner = false;
+			break;
+		case 11:
+			fsckcfg.no_preserve_perms = true;
+			fsckcfg.preserve_perms = false;
 			break;
 		default:
 			return -EINVAL;
 		}
 	}
 
+	if (fsckcfg.extract_path) {
+		if (!fsckcfg.extract_pos && !fsckcfg.force) {
+			erofs_err("--extract=/ must be used together with --force");
+			return -EINVAL;
+		}
+	} else {
+		if (fsckcfg.force)
+			erofs_err("--force must be used together with --extract=X");
+		if (fsckcfg.overwrite)
+			erofs_err("--overwrite must be used together with --extract=X");
+		if (fsckcfg.preserve_owner || fsckcfg.preserve_perms ||
+			  fsckcfg.no_preserve_owner || fsckcfg.no_preserve_perms)
+			erofs_err("--[no-]preserve[-owner/-perms] must be used together with --extract=X");
+	}
+
 	if (optind >= argc)
 		return -EINVAL;
 
-	if (!fsckcfg.extract_path)
-		if (fsckcfg.overwrite || fsckcfg.preserve_owner ||
-		    fsckcfg.preserve_perms)
-			return -EINVAL;
-
 	cfg.c_img_path = strdup(argv[optind++]);
 	if (!cfg.c_img_path)
 		return -ENOMEM;
@@ -680,13 +726,19 @@ int main(int argc, char **argv)
 
 	erofs_init_configure();
 
+	fsckcfg.superuser = geteuid() == 0;
+	fsckcfg.umask = umask(0);
 	fsckcfg.corrupted = false;
+	fsckcfg.logical_blocks = 0;
+	fsckcfg.physical_blocks = 0;
 	fsckcfg.print_comp_ratio = false;
 	fsckcfg.check_decomp = false;
 	fsckcfg.extract_path = NULL;
 	fsckcfg.extract_pos = 0;
-	fsckcfg.logical_blocks = 0;
-	fsckcfg.physical_blocks = 0;
+	fsckcfg.force = false;
+	fsckcfg.overwrite = false;
+	fsckcfg.preserve_owner = fsckcfg.superuser;
+	fsckcfg.preserve_perms = fsckcfg.superuser;
 
 	err = erofsfsck_parse_options_cfg(argc, argv);
 	if (err) {
@@ -695,8 +747,6 @@ int main(int argc, char **argv)
 		goto exit;
 	}
 
-	fsckcfg.umask = umask(0);
-
 	err = dev_open_ro(cfg.c_img_path);
 	if (err) {
 		erofs_err("failed to open image file");
-- 
2.30.2