[PATCH] erofs-utils: fsck: fix issues related to --extract=X
Igor Ostapenko
igoreisberg at gmail.com
Fri Jan 21 11:31:16 AEDT 2022
From: Igor Eisberg <igoreisberg at 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 at 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
More information about the Linux-erofs
mailing list