* [PATCH] erofs-utils: introduce fsck.erofs
@ 2021-10-25 19:48 Daeho Jeong
2021-10-25 23:24 ` Gao Xiang
0 siblings, 1 reply; 13+ messages in thread
From: Daeho Jeong @ 2021-10-25 19:48 UTC (permalink / raw)
To: linux-erofs, bluce.liguifu, miaoxie, fangwei1; +Cc: yuchao0, Daeho Jeong
From: Daeho Jeong <daehojeong@google.com>
I made a fsck.erofs tool to check erofs filesystem image integrity
and calculate filesystem compression ratio.
Here are options to support now.
fsck.erofs [options] IMAGE
-V print the version number of fsck.erofs and exit.
-d# set output message level to # (maximum 9)\n
-c print total compression ratio of all compressed files
-C print total compression ratio of all files
Signed-off-by: Daeho Jeong <daehojeong@google.com>
---
Makefile.am | 2 +-
configure.ac | 3 +-
fsck/Makefile.am | 9 +
fsck/main.c | 548 +++++++++++++++++++++++++++++++++++++++
include/erofs/internal.h | 5 +
include/erofs_fs.h | 13 +
lib/data.c | 4 +-
lib/namei.c | 2 +-
lib/super.c | 1 +
mkfs/main.c | 13 -
10 files changed, 582 insertions(+), 18 deletions(-)
create mode 100644 fsck/Makefile.am
create mode 100644 fsck/main.c
diff --git a/Makefile.am b/Makefile.am
index 24e1d38..fc464e8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2,7 +2,7 @@
ACLOCAL_AMFLAGS = -I m4
-SUBDIRS = man lib mkfs dump
+SUBDIRS = man lib mkfs dump fsck
if ENABLE_FUSE
SUBDIRS += fuse
endif
diff --git a/configure.ac b/configure.ac
index b2c3225..5698b2e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -298,5 +298,6 @@ AC_CONFIG_FILES([Makefile
lib/Makefile
mkfs/Makefile
dump/Makefile
- fuse/Makefile])
+ fuse/Makefile
+ fsck/Makefile])
AC_OUTPUT
diff --git a/fsck/Makefile.am b/fsck/Makefile.am
new file mode 100644
index 0000000..82973ba
--- /dev/null
+++ b/fsck/Makefile.am
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Makefile.am
+
+AUTOMAKE_OPTIONS = foreign
+bin_PROGRAMS = fsck.erofs
+AM_CPPFLAGS = ${libuuid_CFLAGS}
+fsck_erofs_SOURCES = main.c
+fsck_erofs_CFLAGS = -Wall -Werror -I$(top_srcdir)/include
+fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libuuid_LIBS} ${liblz4_LIBS}
diff --git a/fsck/main.c b/fsck/main.c
new file mode 100644
index 0000000..c397d19
--- /dev/null
+++ b/fsck/main.c
@@ -0,0 +1,548 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021 Google LLC
+ * Author: Daeho Jeong <daehojeong@google.com>
+ */
+#include <stdlib.h>
+#include <getopt.h>
+#include <time.h>
+#include "erofs/print.h"
+#include "erofs/io.h"
+
+enum {
+ NO_PRINT_COMP_RATIO = 0,
+ PRINT_COMP_RATIO_ALL = 1,
+ PRINT_COMP_RATIO_COMP_FILE = 2,
+};
+
+static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
+
+struct erofsfsck_cfg {
+ bool corrupted;
+ int print_comp_ratio;
+ u64 ondisk_len;
+ u64 logical_len;
+};
+static struct erofsfsck_cfg fsckcfg;
+
+static struct option long_options[] = {
+ {"help", no_argument, 0, 1},
+ {0, 0, 0, 0},
+};
+
+static void usage(void)
+{
+ fputs("usage: [options] IMAGE\n\n"
+ "Check erofs filesystem integrity of IMAGE, and [options] are:\n"
+ " -V print the version number of fsck.erofs and exit.\n"
+ " -d# set output message level to # (maximum 9)\n"
+ " -c print total compression ratio of all compressed files\n"
+ " -C print total compression ratio of all files\n"
+ " --help display this help and exit.\n",
+ stderr);
+}
+
+static void erofsfsck_print_version(void)
+{
+ fprintf(stderr, "fsck.erofs %s\n", cfg.c_version);
+}
+
+static int erofsfsck_parse_options_cfg(int argc, char **argv)
+{
+ int opt, i;
+
+ while ((opt = getopt_long(argc, argv, "Vd:Cc",
+ long_options, NULL)) != -1) {
+ switch (opt) {
+ case 'V':
+ erofsfsck_print_version();
+ exit(0);
+ case 'd':
+ i = atoi(optarg);
+ if (i < EROFS_MSG_MIN || i > EROFS_MSG_MAX) {
+ erofs_err("invalid debug level %d", i);
+ return -EINVAL;
+ }
+ cfg.c_dbg_lvl = i;
+ break;
+ case 'C':
+ fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_ALL;
+ break;
+ case 'c':
+ fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_COMP_FILE;
+ break;
+ case 1:
+ usage();
+ exit(0);
+ default:
+ return -EINVAL;
+ }
+ }
+
+ if (optind >= argc)
+ return -EINVAL;
+
+ cfg.c_img_path = strdup(argv[optind++]);
+ if (!cfg.c_img_path)
+ return -ENOMEM;
+
+ if (optind < argc) {
+ erofs_err("unexpected argument: %s\n", argv[optind]);
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int erofs_check_sb_chksum(void)
+{
+ int ret;
+ u8 buf[EROFS_BLKSIZ];
+ u32 crc;
+ struct erofs_super_block *sb;
+
+ ret = blk_read(buf, 0, 1);
+ if (ret) {
+ erofs_err("failed to read superblock to check checksum: "
+ "errno(%d)", ret);
+ return -1;
+ }
+
+ sb = (struct erofs_super_block *)(buf + EROFS_SUPER_OFFSET);
+ sb->checksum = 0;
+
+ crc = crc32c(~0, (u8 *)sb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
+ if (crc != sbi.checksum) {
+ erofs_err("superblock checksum doesn't match: saved(0x%08x) "
+ "calculated(0x%08x)", sbi.checksum, crc);
+ fsckcfg.corrupted = true;
+ return -1;
+ }
+ return 0;
+}
+
+static int check_special_dentries(struct erofs_dirent *de, const char *de_name,
+ unsigned int de_namelen, erofs_nid_t nid,
+ erofs_nid_t pnid, bool is_curdir)
+{
+ unsigned int dirname_len = is_curdir ? 1 : 2;
+ const char *dirname = is_curdir ? "." : "..";
+ erofs_nid_t correct_nid = is_curdir ? nid : pnid;
+
+ if (de_namelen != dirname_len || memcmp(de_name, dirname, de_namelen)) {
+ char *dbgname = strndup(de_name, de_namelen);
+
+ BUG_ON(!dbgname);
+ if (is_curdir)
+ erofs_err("wrong current dir name(%s) @ nid(%llu)",
+ dbgname, nid | 0ULL);
+ else
+ erofs_err("wrong parent dir name(%s) @ nid(%llu)",
+ dbgname, nid | 0ULL);
+ free(dbgname);
+ return -1;
+ }
+
+ if (de->nid != correct_nid) {
+ if (is_curdir)
+ erofs_err("wrong current dir nid(%llu) @ nid(%llu)",
+ de->nid | 0ULL, nid | 0ULL);
+ else
+ erofs_err("wrong parent dir nid(%llu): "
+ "pnid(%llu) @ nid(%llu)",
+ de->nid | 0ULL, pnid | 0ULL, nid | 0ULL);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int traverse_dirents(erofs_nid_t pnid, erofs_nid_t nid,
+ void *dentry_blk, erofs_off_t offset,
+ unsigned int next_nameoff, unsigned int maxsize)
+{
+ struct erofs_dirent *de = dentry_blk;
+ const struct erofs_dirent *end = dentry_blk + next_nameoff;
+ unsigned int idx = 0;
+
+ erofs_dbg("traversing pnid(%llu), nid(%llu)", pnid | 0ULL, nid | 0ULL);
+
+ if (offset == 0 && (next_nameoff < 2 * sizeof(struct erofs_dirent))) {
+ erofs_err("too small dirents of size(%d) in nid(%llu)",
+ next_nameoff, nid | 0ULL);
+ return -EFSCORRUPTED;
+ }
+
+ while (de < end) {
+ const char *de_name;
+ unsigned int de_namelen;
+ unsigned int nameoff;
+ char *dbgname;
+
+ nameoff = le16_to_cpu(de->nameoff);
+ de_name = (char *)dentry_blk + nameoff;
+
+ /* the last dirent check */
+ if (de + 1 >= end)
+ de_namelen = strnlen(de_name, maxsize - nameoff);
+ else
+ de_namelen = le16_to_cpu(de[1].nameoff) - nameoff;
+
+ if (cfg.c_dbg_lvl >= EROFS_DBG) {
+ dbgname = strndup(de_name, de_namelen);
+ BUG_ON(!dbgname);
+ erofs_dbg("traversed filename(%s)", dbgname);
+ free(dbgname);
+ }
+
+ /* corrupted entry check */
+ if (nameoff != next_nameoff) {
+ erofs_err("bogus dirent with nameoff(%u): expected(%d) "
+ "@ nid(%llu), offset(%llu), idx(%u)",
+ nameoff, next_nameoff, nid | 0ULL,
+ offset | 0ULL, idx);
+ return -EFSCORRUPTED;
+ }
+
+ if (nameoff + de_namelen > maxsize ||
+ de_namelen > EROFS_NAME_LEN) {
+ erofs_err("bogus dirent with namelen(%u) @ nid(%llu), "
+ "offset(%llu), idx(%u)",
+ de_namelen, nid | 0ULL, offset | 0ULL, idx);
+ return -EFSCORRUPTED;
+ }
+
+ if (offset == 0 && (idx == 0 || idx == 1)) {
+ if (check_special_dentries(de, de_name, de_namelen, nid,
+ pnid, idx == 0))
+ return -EFSCORRUPTED;
+ } else {
+ erofs_check_inode(nid, de->nid);
+ if (fsckcfg.corrupted)
+ return -EFSCORRUPTED;
+ }
+
+ next_nameoff += de_namelen;
+ ++de;
+ ++idx;
+ }
+
+ erofs_dbg("traversing ... done nid(%llu)", nid | 0ULL);
+ return 0;
+}
+
+static int verify_raw_data_chunk(struct erofs_inode *inode)
+{
+ struct erofs_map_blocks map = {
+ .index = UINT_MAX,
+ };
+ int ret;
+ erofs_off_t ptr = 0;
+
+ if (fsckcfg.print_comp_ratio == PRINT_COMP_RATIO_ALL) {
+ fsckcfg.logical_len += inode->i_size;
+ fsckcfg.ondisk_len += inode->i_size;
+ }
+
+ while (ptr < inode->i_size) {
+ map.m_la = ptr;
+ ret = erofs_map_blocks(inode, &map, 0);
+ if (ret)
+ return ret;
+
+ if (map.m_plen != map.m_llen || ptr != map.m_la) {
+ erofs_err("broken data chunk layout m_la %" PRIu64
+ " ptr %" PRIu64 " m_llen %" PRIu64 " m_plen %"
+ PRIu64, map.m_la, ptr, map.m_llen,
+ map.m_plen);
+ return -EFSCORRUPTED;
+ }
+
+ if (!(map.m_flags & EROFS_MAP_MAPPED) && !map.m_llen) {
+ /* readched EOF */
+ ptr = inode->i_size;
+ continue;
+ }
+
+ ptr += map.m_llen;
+ }
+ return 0;
+}
+
+static int verify_compressed_chunk(struct erofs_inode *inode)
+{
+ struct erofs_map_blocks map = {
+ .index = UINT_MAX,
+ };
+ int ret = 0;
+ bool count_pchunk = fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO;
+ u64 pchunk_len = 0;
+ erofs_off_t offset = 0, end = inode->i_size;
+
+ while (end > offset) {
+ map.m_la = end - 1;
+
+ ret = z_erofs_map_blocks_iter(inode, &map);
+ if (ret)
+ return ret;
+
+ if (end > map.m_la + map.m_llen) {
+ erofs_err("broken compressed chunk layout m_la %" PRIu64
+ " m_llen %" PRIu64 " end %" PRIu64,
+ map.m_la, map.m_llen, end);
+ return -EFSCORRUPTED;
+ }
+
+ if (count_pchunk)
+ pchunk_len += map.m_plen;
+
+ end = map.m_la;
+ }
+
+ if (count_pchunk) {
+ fsckcfg.logical_len += inode->i_size;
+ fsckcfg.ondisk_len += pchunk_len;
+ }
+
+ return 0;
+}
+
+static int erofs_verify_xattr(struct erofs_inode *inode)
+{
+ unsigned int xattr_hdr_size = sizeof(struct erofs_xattr_ibody_header);
+ unsigned int xattr_entry_size = sizeof(struct erofs_xattr_entry);
+ erofs_off_t addr;
+ unsigned int ofs, xattr_shared_count;
+ struct erofs_xattr_ibody_header *ih;
+ struct erofs_xattr_entry *entry;
+ int i, remaining = inode->xattr_isize, ret = 0;
+ char *buf = calloc(EROFS_BLKSIZ, 1);
+
+ BUG_ON(!buf);
+
+ if (inode->xattr_isize == xattr_hdr_size) {
+ erofs_err("xattr_isize %d of nid %llu is not supported yet",
+ inode->xattr_isize, inode->nid | 0ULL);
+ ret = -EFSCORRUPTED;
+ goto out;
+ } else if (inode->xattr_isize < xattr_hdr_size) {
+ if (inode->xattr_isize) {
+ erofs_err("bogus xattr ibody @ nid %llu",
+ inode->nid | 0ULL);
+ ret = -EFSCORRUPTED;
+ goto out;
+ }
+ }
+
+ addr = iloc(inode->nid) + inode->inode_isize;
+ ret = dev_read(buf, addr, xattr_hdr_size);
+ if (ret < 0) {
+ erofs_err("an error occurred when reading xattr header "
+ "of nid(%llu): errno(%d)", inode->nid | 0ULL, ret);
+ goto out;
+ }
+ ih = (struct erofs_xattr_ibody_header *)buf;
+ xattr_shared_count = ih->h_shared_count;
+
+ ofs = erofs_blkoff(addr) + xattr_hdr_size;
+ addr += xattr_hdr_size;
+ remaining -= xattr_hdr_size;
+ for (i = 0; i < xattr_shared_count; ++i) {
+ if (ofs >= EROFS_BLKSIZ) {
+ if (ofs != EROFS_BLKSIZ) {
+ erofs_err("unaligned xattr entry in "
+ "xattr shared area of nid(%llu)",
+ inode->nid | 0ULL);
+ ret = -EFSCORRUPTED;
+ goto out;
+ }
+ ofs = 0;
+ }
+ ofs += xattr_entry_size;
+ addr += xattr_entry_size;
+ remaining -= xattr_entry_size;
+ }
+
+ while (remaining > 0) {
+ unsigned int entry_sz;
+
+ ret = dev_read(buf, addr, xattr_entry_size);
+ if (ret) {
+ erofs_err("an error occurred when reading xattr entry "
+ "of nid(%llu): errno(%d)",
+ inode->nid | 0ULL, ret);
+ goto out;
+ }
+
+ entry = (struct erofs_xattr_entry *)buf;
+ entry_sz = erofs_xattr_entry_size(entry);
+ if (remaining < entry_sz) {
+ erofs_err("xattr on-disk corruption: xattr entry "
+ "beyond xattr_isize of nid(%llu)",
+ inode->nid | 0ULL);
+ ret = -EFSCORRUPTED;
+ goto out;
+ }
+ addr += entry_sz;
+ remaining -= entry_sz;
+ }
+out:
+ free(buf);
+ return ret;
+}
+
+static int erofs_verify_data_chunk(struct erofs_inode *inode)
+{
+ int ret;
+
+ erofs_dbg("verify data chunk of nid(%llu): type(%d)",
+ inode->nid | 0ULL, inode->datalayout);
+
+ switch (inode->datalayout) {
+ case EROFS_INODE_FLAT_PLAIN:
+ case EROFS_INODE_FLAT_INLINE:
+ case EROFS_INODE_CHUNK_BASED:
+ ret = verify_raw_data_chunk(inode);
+ break;
+ case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
+ case EROFS_INODE_FLAT_COMPRESSION:
+ ret = verify_compressed_chunk(inode);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ if (ret == -EIO)
+ erofs_err("I/O error occurred when verifying "
+ "data chunk of nid(%llu)", inode->nid | 0ULL);
+
+ return ret;
+}
+
+static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid)
+{
+ int ret;
+ struct erofs_inode *inode;
+ char *buf;
+ erofs_off_t offset;
+
+ erofs_dbg("check inode: nid(%llu)", nid | 0ULL);
+ inode = calloc(1, sizeof(struct erofs_inode));
+ BUG_ON(!inode);
+ buf = calloc(EROFS_BLKSIZ, 1);
+ BUG_ON(!buf);
+
+ inode->nid = nid;
+ ret = erofs_read_inode_from_disk(inode);
+ if (ret) {
+ if (ret == -EIO)
+ erofs_err("I/O error occurred when reading nid(%llu)",
+ nid | 0ULL);
+ goto out;
+ }
+
+ /* verify xattr field */
+ ret = erofs_verify_xattr(inode);
+ if (ret)
+ goto out;
+
+ /* verify data chunk layout */
+ ret = erofs_verify_data_chunk(inode);
+ if (ret)
+ goto out;
+
+ if ((inode->i_mode & S_IFMT) != S_IFDIR)
+ goto out;
+
+ offset = 0;
+ while (offset < inode->i_size) {
+ erofs_off_t maxsize = min_t(erofs_off_t,
+ inode->i_size - offset, EROFS_BLKSIZ);
+ struct erofs_dirent *de = (void *)buf;
+ unsigned int nameoff;
+
+ ret = erofs_pread(inode, buf, maxsize, offset);
+ if (ret) {
+ erofs_err("I/O error occurred when reading dirents: "
+ "nid(%llu), offset(%llu), size(%llu)",
+ nid | 0ULL, offset | 0ULL, maxsize | 0ULL);
+ goto out;
+ }
+
+ nameoff = le16_to_cpu(de->nameoff);
+ if (nameoff < sizeof(struct erofs_dirent) ||
+ nameoff >= PAGE_SIZE) {
+ erofs_err("invalid de[0].nameoff %u @ nid(%llu), "
+ "offset(%llu)",
+ nameoff, nid | 0ULL, offset | 0ULL);
+ ret = -EFSCORRUPTED;
+ goto out;
+ }
+
+ ret = traverse_dirents(pnid, nid, buf, offset,
+ nameoff, maxsize);
+ if (ret)
+ goto out;
+
+ offset += maxsize;
+ }
+out:
+ free(buf);
+ free(inode);
+ if (ret && ret != -EIO)
+ fsckcfg.corrupted = true;
+}
+
+int main(int argc, char **argv)
+{
+ int err;
+
+ erofs_init_configure();
+
+ fsckcfg.corrupted = false;
+ fsckcfg.print_comp_ratio = NO_PRINT_COMP_RATIO;
+ fsckcfg.logical_len = 0;
+ fsckcfg.ondisk_len = 0;
+
+ err = erofsfsck_parse_options_cfg(argc, argv);
+ if (err) {
+ if (err == -EINVAL)
+ usage();
+ goto exit;
+ }
+
+ err = dev_open_ro(cfg.c_img_path);
+ if (err) {
+ erofs_err("failed to open image file");
+ goto exit;
+ }
+
+ err = erofs_read_superblock();
+ if (err) {
+ erofs_err("failed to read superblock");
+ goto exit;
+ }
+
+ if (erofs_sb_has_sb_chksum() && erofs_check_sb_chksum()) {
+ erofs_err("failed to verify superblock checksum");
+ goto exit;
+ }
+
+ erofs_check_inode(sbi.root_nid, sbi.root_nid);
+
+ if (fsckcfg.corrupted) {
+ fprintf(stderr, "Found some filesystem corruption\n");
+ } else {
+ fprintf(stderr, "No error found\n");
+ if (fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO) {
+ double comp_ratio = (double)fsckcfg.ondisk_len * 100 /
+ (double)fsckcfg.logical_len;
+ fprintf(stderr, "Compression Ratio: %.2f(%%)\n",
+ comp_ratio);
+ }
+ }
+
+exit:
+ erofs_exit_configure();
+ return err;
+}
diff --git a/include/erofs/internal.h b/include/erofs/internal.h
index 8b154ed..80065b2 100644
--- a/include/erofs/internal.h
+++ b/include/erofs/internal.h
@@ -82,6 +82,8 @@ struct erofs_sb_info {
u16 available_compr_algs;
u16 lz4_max_distance;
+
+ u32 checksum;
};
/* global sbi */
@@ -264,10 +266,13 @@ int erofs_read_superblock(void);
/* namei.c */
int erofs_ilookup(const char *path, struct erofs_inode *vi);
+int erofs_read_inode_from_disk(struct erofs_inode *vi);
/* data.c */
int erofs_pread(struct erofs_inode *inode, char *buf,
erofs_off_t count, erofs_off_t offset);
+int erofs_map_blocks(struct erofs_inode *inode,
+ struct erofs_map_blocks *map, int flags);
/* zmap.c */
int z_erofs_fill_inode(struct erofs_inode *vi);
int z_erofs_map_blocks_iter(struct erofs_inode *vi,
diff --git a/include/erofs_fs.h b/include/erofs_fs.h
index 66a68e3..62e9981 100644
--- a/include/erofs_fs.h
+++ b/include/erofs_fs.h
@@ -400,4 +400,17 @@ static inline void erofs_check_ondisk_layout_definitions(void)
Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
}
+#define CRC32C_POLY_LE 0x82F63B78
+static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
+{
+ int i;
+
+ while (len--) {
+ crc ^= *in++;
+ for (i = 0; i < 8; i++)
+ crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
+ }
+ return crc;
+}
+
#endif
diff --git a/lib/data.c b/lib/data.c
index 641d840..6cb7eeb 100644
--- a/lib/data.c
+++ b/lib/data.c
@@ -61,8 +61,8 @@ err_out:
return err;
}
-static int erofs_map_blocks(struct erofs_inode *inode,
- struct erofs_map_blocks *map, int flags)
+int erofs_map_blocks(struct erofs_inode *inode,
+ struct erofs_map_blocks *map, int flags)
{
struct erofs_inode *vi = inode;
struct erofs_inode_chunk_index *idx;
diff --git a/lib/namei.c b/lib/namei.c
index b4bdabf..56f199a 100644
--- a/lib/namei.c
+++ b/lib/namei.c
@@ -22,7 +22,7 @@ static dev_t erofs_new_decode_dev(u32 dev)
return makedev(major, minor);
}
-static int erofs_read_inode_from_disk(struct erofs_inode *vi)
+int erofs_read_inode_from_disk(struct erofs_inode *vi)
{
int ret, ifmt;
char buf[sizeof(struct erofs_inode_extended)];
diff --git a/lib/super.c b/lib/super.c
index 0fa69ab..0c30403 100644
--- a/lib/super.c
+++ b/lib/super.c
@@ -62,6 +62,7 @@ int erofs_read_superblock(void)
sbi.islotbits = EROFS_ISLOTBITS;
sbi.root_nid = le16_to_cpu(dsb->root_nid);
sbi.inos = le64_to_cpu(dsb->inos);
+ sbi.checksum = le32_to_cpu(dsb->checksum);
sbi.build_time = le64_to_cpu(dsb->build_time);
sbi.build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
diff --git a/mkfs/main.c b/mkfs/main.c
index 1c8dea5..b9b46f5 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -424,19 +424,6 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
return 0;
}
-#define CRC32C_POLY_LE 0x82F63B78
-static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
-{
- int i;
-
- while (len--) {
- crc ^= *in++;
- for (i = 0; i < 8; i++)
- crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
- }
- return crc;
-}
-
static int erofs_mkfs_superblock_csum_set(void)
{
int ret;
--
2.33.0.1079.g6e70778dc9-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] erofs-utils: introduce fsck.erofs
2021-10-25 19:48 [PATCH] erofs-utils: introduce fsck.erofs Daeho Jeong
@ 2021-10-25 23:24 ` Gao Xiang
2021-10-26 2:42 ` Guo Xuenan
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Gao Xiang @ 2021-10-25 23:24 UTC (permalink / raw)
To: Daeho Jeong; +Cc: Daeho Jeong, linux-erofs, miaoxie, Wang Qi
(Sorry due to just mutt+vim edit by mistake. try to send properly
again..)
Hi Daeho,
On Mon, Oct 25, 2021 at 12:48:09PM -0700, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
>
> I made a fsck.erofs tool to check erofs filesystem image integrity
> and calculate filesystem compression ratio.
> Here are options to support now.
>
> fsck.erofs [options] IMAGE
> -V print the version number of fsck.erofs and exit.
> -d# set output message level to # (maximum 9)\n
> -c print total compression ratio of all compressed files
> -C print total compression ratio of all files
>
> Signed-off-by: Daeho Jeong <daehojeong@google.com>
Many thanks for the patch! I also think some fsck feature is useful in
order to check the image integration.
IMO, Compression ratio calculation is more like a dump fs feature.
Wang Qi developed the erofsdump yet he told me that he is working for
another urgent stuffs for now so he delayed the work:
https://lore.kernel.org/r/20210915093537.2579575-1-guoxuenan@huawei.com
fsck and dump have the similar logic (and like f2fs-tools, we can also
have one program for this 2 features). If fsck is actively developed,
I'm pretty fine to use the fsck codebase and merge erofsdump into it
later instead.
Some comments as below:
> ---
> Makefile.am | 2 +-
> configure.ac | 3 +-
> fsck/Makefile.am | 9 +
> fsck/main.c | 548 +++++++++++++++++++++++++++++++++++++++
> include/erofs/internal.h | 5 +
> include/erofs_fs.h | 13 +
> lib/data.c | 4 +-
> lib/namei.c | 2 +-
> lib/super.c | 1 +
> mkfs/main.c | 13 -
> 10 files changed, 582 insertions(+), 18 deletions(-)
> create mode 100644 fsck/Makefile.am
> create mode 100644 fsck/main.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 24e1d38..fc464e8 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -2,7 +2,7 @@
>
> ACLOCAL_AMFLAGS = -I m4
>
> -SUBDIRS = man lib mkfs dump
> +SUBDIRS = man lib mkfs dump fsck
> if ENABLE_FUSE
> SUBDIRS += fuse
> endif
> diff --git a/configure.ac b/configure.ac
> index b2c3225..5698b2e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -298,5 +298,6 @@ AC_CONFIG_FILES([Makefile
> lib/Makefile
> mkfs/Makefile
> dump/Makefile
> - fuse/Makefile])
> + fuse/Makefile
> + fsck/Makefile])
> AC_OUTPUT
> diff --git a/fsck/Makefile.am b/fsck/Makefile.am
> new file mode 100644
> index 0000000..82973ba
> --- /dev/null
> +++ b/fsck/Makefile.am
> @@ -0,0 +1,9 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Makefile.am
> +
> +AUTOMAKE_OPTIONS = foreign
> +bin_PROGRAMS = fsck.erofs
> +AM_CPPFLAGS = ${libuuid_CFLAGS}
> +fsck_erofs_SOURCES = main.c
> +fsck_erofs_CFLAGS = -Wall -Werror -I$(top_srcdir)/include
> +fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libuuid_LIBS} ${liblz4_LIBS}
> diff --git a/fsck/main.c b/fsck/main.c
> new file mode 100644
> index 0000000..c397d19
> --- /dev/null
> +++ b/fsck/main.c
> @@ -0,0 +1,548 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2021 Google LLC
> + * Author: Daeho Jeong <daehojeong@google.com>
> + */
> +#include <stdlib.h>
> +#include <getopt.h>
> +#include <time.h>
> +#include "erofs/print.h"
> +#include "erofs/io.h"
> +
> +enum {
> + NO_PRINT_COMP_RATIO = 0,
> + PRINT_COMP_RATIO_ALL = 1,
> + PRINT_COMP_RATIO_COMP_FILE = 2,
> +};
> +
> +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
> +
> +struct erofsfsck_cfg {
> + bool corrupted;
> + int print_comp_ratio;
> + u64 ondisk_len;
> + u64 logical_len;
> +};
> +static struct erofsfsck_cfg fsckcfg;
> +
> +static struct option long_options[] = {
> + {"help", no_argument, 0, 1},
> + {0, 0, 0, 0},
> +};
> +
> +static void usage(void)
> +{
> + fputs("usage: [options] IMAGE\n\n"
> + "Check erofs filesystem integrity of IMAGE, and [options] are:\n"
> + " -V print the version number of fsck.erofs and exit.\n"
> + " -d# set output message level to # (maximum 9)\n"
> + " -c print total compression ratio of all compressed files\n"
> + " -C print total compression ratio of all files\n"
Apart from erofsdump consideration.. Can we merge these 2 options into one?
> + " --help display this help and exit.\n",
> + stderr);
> +}
> +
> +static void erofsfsck_print_version(void)
> +{
> + fprintf(stderr, "fsck.erofs %s\n", cfg.c_version);
> +}
> +
> +static int erofsfsck_parse_options_cfg(int argc, char **argv)
> +{
> + int opt, i;
> +
> + while ((opt = getopt_long(argc, argv, "Vd:Cc",
> + long_options, NULL)) != -1) {
> + switch (opt) {
> + case 'V':
> + erofsfsck_print_version();
> + exit(0);
> + case 'd':
> + i = atoi(optarg);
> + if (i < EROFS_MSG_MIN || i > EROFS_MSG_MAX) {
> + erofs_err("invalid debug level %d", i);
> + return -EINVAL;
> + }
> + cfg.c_dbg_lvl = i;
> + break;
> + case 'C':
> + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_ALL;
> + break;
> + case 'c':
> + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_COMP_FILE;
> + break;
> + case 1:
> + usage();
> + exit(0);
> + default:
> + return -EINVAL;
> + }
> + }
> +
> + if (optind >= argc)
> + return -EINVAL;
> +
> + cfg.c_img_path = strdup(argv[optind++]);
> + if (!cfg.c_img_path)
> + return -ENOMEM;
> +
> + if (optind < argc) {
> + erofs_err("unexpected argument: %s\n", argv[optind]);
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int erofs_check_sb_chksum(void)
> +{
> + int ret;
> + u8 buf[EROFS_BLKSIZ];
> + u32 crc;
> + struct erofs_super_block *sb;
> +
> + ret = blk_read(buf, 0, 1);
> + if (ret) {
> + erofs_err("failed to read superblock to check checksum: "
> + "errno(%d)", ret);
> + return -1;
> + }
> +
> + sb = (struct erofs_super_block *)(buf + EROFS_SUPER_OFFSET);
> + sb->checksum = 0;
> +
> + crc = crc32c(~0, (u8 *)sb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
> + if (crc != sbi.checksum) {
> + erofs_err("superblock checksum doesn't match: saved(0x%08x) "
> + "calculated(0x%08x)", sbi.checksum, crc);
> + fsckcfg.corrupted = true;
> + return -1;
> + }
> + return 0;
> +}
> +
> +static int check_special_dentries(struct erofs_dirent *de, const char *de_name,
> + unsigned int de_namelen, erofs_nid_t nid,
> + erofs_nid_t pnid, bool is_curdir)
> +{
> + unsigned int dirname_len = is_curdir ? 1 : 2;
> + const char *dirname = is_curdir ? "." : "..";
> + erofs_nid_t correct_nid = is_curdir ? nid : pnid;
> +
> + if (de_namelen != dirname_len || memcmp(de_name, dirname, de_namelen)) {
> + char *dbgname = strndup(de_name, de_namelen);
> +
> + BUG_ON(!dbgname);
> + if (is_curdir)
> + erofs_err("wrong current dir name(%s) @ nid(%llu)",
> + dbgname, nid | 0ULL);
> + else
> + erofs_err("wrong parent dir name(%s) @ nid(%llu)",
> + dbgname, nid | 0ULL);
> + free(dbgname);
> + return -1;
> + }
> +
> + if (de->nid != correct_nid) {
> + if (is_curdir)
> + erofs_err("wrong current dir nid(%llu) @ nid(%llu)",
> + de->nid | 0ULL, nid | 0ULL);
> + else
> + erofs_err("wrong parent dir nid(%llu): "
> + "pnid(%llu) @ nid(%llu)",
> + de->nid | 0ULL, pnid | 0ULL, nid | 0ULL);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int traverse_dirents(erofs_nid_t pnid, erofs_nid_t nid,
> + void *dentry_blk, erofs_off_t offset,
> + unsigned int next_nameoff, unsigned int maxsize)
Not quite sure if we could introduce some
erofs_foreach_dirent() and customized callbacks to cleanup similar functions
in fuse and fsck...
> +{
> + struct erofs_dirent *de = dentry_blk;
> + const struct erofs_dirent *end = dentry_blk + next_nameoff;
> + unsigned int idx = 0;
> +
> + erofs_dbg("traversing pnid(%llu), nid(%llu)", pnid | 0ULL, nid | 0ULL);
> +
> + if (offset == 0 && (next_nameoff < 2 * sizeof(struct erofs_dirent))) {
> + erofs_err("too small dirents of size(%d) in nid(%llu)",
> + next_nameoff, nid | 0ULL);
> + return -EFSCORRUPTED;
> + }
> +
> + while (de < end) {
> + const char *de_name;
> + unsigned int de_namelen;
> + unsigned int nameoff;
> + char *dbgname;
> +
> + nameoff = le16_to_cpu(de->nameoff);
> + de_name = (char *)dentry_blk + nameoff;
> +
> + /* the last dirent check */
> + if (de + 1 >= end)
> + de_namelen = strnlen(de_name, maxsize - nameoff);
> + else
> + de_namelen = le16_to_cpu(de[1].nameoff) - nameoff;
> +
> + if (cfg.c_dbg_lvl >= EROFS_DBG) {
> + dbgname = strndup(de_name, de_namelen);
> + BUG_ON(!dbgname);
> + erofs_dbg("traversed filename(%s)", dbgname);
> + free(dbgname);
> + }
> +
> + /* corrupted entry check */
> + if (nameoff != next_nameoff) {
> + erofs_err("bogus dirent with nameoff(%u): expected(%d) "
> + "@ nid(%llu), offset(%llu), idx(%u)",
> + nameoff, next_nameoff, nid | 0ULL,
> + offset | 0ULL, idx);
> + return -EFSCORRUPTED;
> + }
> +
> + if (nameoff + de_namelen > maxsize ||
> + de_namelen > EROFS_NAME_LEN) {
> + erofs_err("bogus dirent with namelen(%u) @ nid(%llu), "
> + "offset(%llu), idx(%u)",
> + de_namelen, nid | 0ULL, offset | 0ULL, idx);
> + return -EFSCORRUPTED;
> + }
> +
> + if (offset == 0 && (idx == 0 || idx == 1)) {
We may not assume "." and ".." special dirents as 0 or 1... Actually
all dirents are sorted in the alphabetical order. I'm not sure if
some dirents are smaller than "." and "..".
We could also check if all dirents are in alphabetical order as well
since it's used to do binary search runtimely.
> + if (check_special_dentries(de, de_name, de_namelen, nid,
> + pnid, idx == 0))
> + return -EFSCORRUPTED;
> + } else {
> + erofs_check_inode(nid, de->nid);
> + if (fsckcfg.corrupted)
> + return -EFSCORRUPTED;
> + }
> +
> + next_nameoff += de_namelen;
> + ++de;
> + ++idx;
> + }
> +
> + erofs_dbg("traversing ... done nid(%llu)", nid | 0ULL);
> + return 0;
> +}
> +
> +static int verify_raw_data_chunk(struct erofs_inode *inode)
> +{
> + struct erofs_map_blocks map = {
> + .index = UINT_MAX,
> + };
> + int ret;
> + erofs_off_t ptr = 0;
> +
> + if (fsckcfg.print_comp_ratio == PRINT_COMP_RATIO_ALL) {
> + fsckcfg.logical_len += inode->i_size;
> + fsckcfg.ondisk_len += inode->i_size;
> + }
> +
> + while (ptr < inode->i_size) {
> + map.m_la = ptr;
> + ret = erofs_map_blocks(inode, &map, 0);
> + if (ret)
> + return ret;
> +
> + if (map.m_plen != map.m_llen || ptr != map.m_la) {
> + erofs_err("broken data chunk layout m_la %" PRIu64
> + " ptr %" PRIu64 " m_llen %" PRIu64 " m_plen %"
> + PRIu64, map.m_la, ptr, map.m_llen,
> + map.m_plen);
> + return -EFSCORRUPTED;
> + }
> +
> + if (!(map.m_flags & EROFS_MAP_MAPPED) && !map.m_llen) {
> + /* readched EOF */
> + ptr = inode->i_size;
> + continue;
> + }
> +
> + ptr += map.m_llen;
> + }
> + return 0;
> +}
> +
> +static int verify_compressed_chunk(struct erofs_inode *inode)
> +{
> + struct erofs_map_blocks map = {
> + .index = UINT_MAX,
> + };
> + int ret = 0;
> + bool count_pchunk = fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO;
> + u64 pchunk_len = 0;
> + erofs_off_t offset = 0, end = inode->i_size;
> +
> + while (end > offset) {
> + map.m_la = end - 1;
> +
> + ret = z_erofs_map_blocks_iter(inode, &map);
> + if (ret)
> + return ret;
> +
> + if (end > map.m_la + map.m_llen) {
> + erofs_err("broken compressed chunk layout m_la %" PRIu64
> + " m_llen %" PRIu64 " end %" PRIu64,
> + map.m_la, map.m_llen, end);
> + return -EFSCORRUPTED;
> + }
IMO, A simple way to check this is to try to decompress by calling
z_erofs_decompress() with .partial_decoding == 0 and see if any
error when decompressing as well.
> +
> + if (count_pchunk)
> + pchunk_len += map.m_plen;
> +
> + end = map.m_la;
> + }
> +
> + if (count_pchunk) {
> + fsckcfg.logical_len += inode->i_size;
> + fsckcfg.ondisk_len += pchunk_len;
we can also check another field called compressed_blocks in on-disk
inode...
> + }
> +
> + return 0;
> +}
> +
> +static int erofs_verify_xattr(struct erofs_inode *inode)
> +{
> + unsigned int xattr_hdr_size = sizeof(struct erofs_xattr_ibody_header);
> + unsigned int xattr_entry_size = sizeof(struct erofs_xattr_entry);
> + erofs_off_t addr;
> + unsigned int ofs, xattr_shared_count;
> + struct erofs_xattr_ibody_header *ih;
> + struct erofs_xattr_entry *entry;
> + int i, remaining = inode->xattr_isize, ret = 0;
> + char *buf = calloc(EROFS_BLKSIZ, 1);
> +
> + BUG_ON(!buf);
> +
> + if (inode->xattr_isize == xattr_hdr_size) {
> + erofs_err("xattr_isize %d of nid %llu is not supported yet",
> + inode->xattr_isize, inode->nid | 0ULL);
> + ret = -EFSCORRUPTED;
> + goto out;
> + } else if (inode->xattr_isize < xattr_hdr_size) {
> + if (inode->xattr_isize) {
> + erofs_err("bogus xattr ibody @ nid %llu",
> + inode->nid | 0ULL);
> + ret = -EFSCORRUPTED;
> + goto out;
> + }
> + }
> +
> + addr = iloc(inode->nid) + inode->inode_isize;
> + ret = dev_read(buf, addr, xattr_hdr_size);
> + if (ret < 0) {
> + erofs_err("an error occurred when reading xattr header "
> + "of nid(%llu): errno(%d)", inode->nid | 0ULL, ret);
> + goto out;
> + }
> + ih = (struct erofs_xattr_ibody_header *)buf;
> + xattr_shared_count = ih->h_shared_count;
> +
> + ofs = erofs_blkoff(addr) + xattr_hdr_size;
> + addr += xattr_hdr_size;
> + remaining -= xattr_hdr_size;
> + for (i = 0; i < xattr_shared_count; ++i) {
> + if (ofs >= EROFS_BLKSIZ) {
> + if (ofs != EROFS_BLKSIZ) {
> + erofs_err("unaligned xattr entry in "
> + "xattr shared area of nid(%llu)",
> + inode->nid | 0ULL);
> + ret = -EFSCORRUPTED;
IMO, it won't happen due to proper alignment.. but I'm fine as this too.
Thanks,
Gao Xiang
> + goto out;
> + }
> + ofs = 0;
> + }
> + ofs += xattr_entry_size;
> + addr += xattr_entry_size;
> + remaining -= xattr_entry_size;
> + }
> +
> + while (remaining > 0) {
> + unsigned int entry_sz;
> +
> + ret = dev_read(buf, addr, xattr_entry_size);
> + if (ret) {
> + erofs_err("an error occurred when reading xattr entry "
> + "of nid(%llu): errno(%d)",
> + inode->nid | 0ULL, ret);
> + goto out;
> + }
> +
> + entry = (struct erofs_xattr_entry *)buf;
> + entry_sz = erofs_xattr_entry_size(entry);
> + if (remaining < entry_sz) {
> + erofs_err("xattr on-disk corruption: xattr entry "
> + "beyond xattr_isize of nid(%llu)",
> + inode->nid | 0ULL);
> + ret = -EFSCORRUPTED;
> + goto out;
> + }
> + addr += entry_sz;
> + remaining -= entry_sz;
> + }
> +out:
> + free(buf);
> + return ret;
> +}
> +
> +static int erofs_verify_data_chunk(struct erofs_inode *inode)
> +{
> + int ret;
> +
> + erofs_dbg("verify data chunk of nid(%llu): type(%d)",
> + inode->nid | 0ULL, inode->datalayout);
> +
> + switch (inode->datalayout) {
> + case EROFS_INODE_FLAT_PLAIN:
> + case EROFS_INODE_FLAT_INLINE:
> + case EROFS_INODE_CHUNK_BASED:
> + ret = verify_raw_data_chunk(inode);
> + break;
> + case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
> + case EROFS_INODE_FLAT_COMPRESSION:
> + ret = verify_compressed_chunk(inode);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + if (ret == -EIO)
> + erofs_err("I/O error occurred when verifying "
> + "data chunk of nid(%llu)", inode->nid | 0ULL);
> +
> + return ret;
> +}
> +
> +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid)
> +{
> + int ret;
> + struct erofs_inode *inode;
> + char *buf;
> + erofs_off_t offset;
> +
> + erofs_dbg("check inode: nid(%llu)", nid | 0ULL);
> + inode = calloc(1, sizeof(struct erofs_inode));
> + BUG_ON(!inode);
> + buf = calloc(EROFS_BLKSIZ, 1);
> + BUG_ON(!buf);
> +
> + inode->nid = nid;
> + ret = erofs_read_inode_from_disk(inode);
> + if (ret) {
> + if (ret == -EIO)
> + erofs_err("I/O error occurred when reading nid(%llu)",
> + nid | 0ULL);
> + goto out;
> + }
> +
> + /* verify xattr field */
> + ret = erofs_verify_xattr(inode);
> + if (ret)
> + goto out;
> +
> + /* verify data chunk layout */
> + ret = erofs_verify_data_chunk(inode);
> + if (ret)
> + goto out;
> +
> + if ((inode->i_mode & S_IFMT) != S_IFDIR)
> + goto out;
> +
> + offset = 0;
> + while (offset < inode->i_size) {
> + erofs_off_t maxsize = min_t(erofs_off_t,
> + inode->i_size - offset, EROFS_BLKSIZ);
> + struct erofs_dirent *de = (void *)buf;
> + unsigned int nameoff;
> +
> + ret = erofs_pread(inode, buf, maxsize, offset);
> + if (ret) {
> + erofs_err("I/O error occurred when reading dirents: "
> + "nid(%llu), offset(%llu), size(%llu)",
> + nid | 0ULL, offset | 0ULL, maxsize | 0ULL);
> + goto out;
> + }
> +
> + nameoff = le16_to_cpu(de->nameoff);
> + if (nameoff < sizeof(struct erofs_dirent) ||
> + nameoff >= PAGE_SIZE) {
> + erofs_err("invalid de[0].nameoff %u @ nid(%llu), "
> + "offset(%llu)",
> + nameoff, nid | 0ULL, offset | 0ULL);
> + ret = -EFSCORRUPTED;
> + goto out;
> + }
> +
> + ret = traverse_dirents(pnid, nid, buf, offset,
> + nameoff, maxsize);
> + if (ret)
> + goto out;
> +
> + offset += maxsize;
> + }
> +out:
> + free(buf);
> + free(inode);
> + if (ret && ret != -EIO)
> + fsckcfg.corrupted = true;
> +}
> +
> +int main(int argc, char **argv)
> +{
> + int err;
> +
> + erofs_init_configure();
> +
> + fsckcfg.corrupted = false;
> + fsckcfg.print_comp_ratio = NO_PRINT_COMP_RATIO;
> + fsckcfg.logical_len = 0;
> + fsckcfg.ondisk_len = 0;
> +
> + err = erofsfsck_parse_options_cfg(argc, argv);
> + if (err) {
> + if (err == -EINVAL)
> + usage();
> + goto exit;
> + }
> +
> + err = dev_open_ro(cfg.c_img_path);
> + if (err) {
> + erofs_err("failed to open image file");
> + goto exit;
> + }
> +
> + err = erofs_read_superblock();
> + if (err) {
> + erofs_err("failed to read superblock");
> + goto exit;
> + }
> +
> + if (erofs_sb_has_sb_chksum() && erofs_check_sb_chksum()) {
> + erofs_err("failed to verify superblock checksum");
> + goto exit;
> + }
> +
> + erofs_check_inode(sbi.root_nid, sbi.root_nid);
> +
> + if (fsckcfg.corrupted) {
> + fprintf(stderr, "Found some filesystem corruption\n");
> + } else {
> + fprintf(stderr, "No error found\n");
> + if (fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO) {
> + double comp_ratio = (double)fsckcfg.ondisk_len * 100 /
> + (double)fsckcfg.logical_len;
> + fprintf(stderr, "Compression Ratio: %.2f(%%)\n",
> + comp_ratio);
> + }
> + }
> +
> +exit:
> + erofs_exit_configure();
> + return err;
> +}
> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> index 8b154ed..80065b2 100644
> --- a/include/erofs/internal.h
> +++ b/include/erofs/internal.h
> @@ -82,6 +82,8 @@ struct erofs_sb_info {
>
> u16 available_compr_algs;
> u16 lz4_max_distance;
> +
> + u32 checksum;
> };
>
> /* global sbi */
> @@ -264,10 +266,13 @@ int erofs_read_superblock(void);
>
> /* namei.c */
> int erofs_ilookup(const char *path, struct erofs_inode *vi);
> +int erofs_read_inode_from_disk(struct erofs_inode *vi);
>
> /* data.c */
> int erofs_pread(struct erofs_inode *inode, char *buf,
> erofs_off_t count, erofs_off_t offset);
> +int erofs_map_blocks(struct erofs_inode *inode,
> + struct erofs_map_blocks *map, int flags);
> /* zmap.c */
> int z_erofs_fill_inode(struct erofs_inode *vi);
> int z_erofs_map_blocks_iter(struct erofs_inode *vi,
> diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> index 66a68e3..62e9981 100644
> --- a/include/erofs_fs.h
> +++ b/include/erofs_fs.h
> @@ -400,4 +400,17 @@ static inline void erofs_check_ondisk_layout_definitions(void)
> Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
> }
>
> +#define CRC32C_POLY_LE 0x82F63B78
> +static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
> +{
> + int i;
> +
> + while (len--) {
> + crc ^= *in++;
> + for (i = 0; i < 8; i++)
> + crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
> + }
> + return crc;
> +}
> +
> #endif
> diff --git a/lib/data.c b/lib/data.c
> index 641d840..6cb7eeb 100644
> --- a/lib/data.c
> +++ b/lib/data.c
> @@ -61,8 +61,8 @@ err_out:
> return err;
> }
>
> -static int erofs_map_blocks(struct erofs_inode *inode,
> - struct erofs_map_blocks *map, int flags)
> +int erofs_map_blocks(struct erofs_inode *inode,
> + struct erofs_map_blocks *map, int flags)
> {
> struct erofs_inode *vi = inode;
> struct erofs_inode_chunk_index *idx;
> diff --git a/lib/namei.c b/lib/namei.c
> index b4bdabf..56f199a 100644
> --- a/lib/namei.c
> +++ b/lib/namei.c
> @@ -22,7 +22,7 @@ static dev_t erofs_new_decode_dev(u32 dev)
> return makedev(major, minor);
> }
>
> -static int erofs_read_inode_from_disk(struct erofs_inode *vi)
> +int erofs_read_inode_from_disk(struct erofs_inode *vi)
> {
> int ret, ifmt;
> char buf[sizeof(struct erofs_inode_extended)];
> diff --git a/lib/super.c b/lib/super.c
> index 0fa69ab..0c30403 100644
> --- a/lib/super.c
> +++ b/lib/super.c
> @@ -62,6 +62,7 @@ int erofs_read_superblock(void)
> sbi.islotbits = EROFS_ISLOTBITS;
> sbi.root_nid = le16_to_cpu(dsb->root_nid);
> sbi.inos = le64_to_cpu(dsb->inos);
> + sbi.checksum = le32_to_cpu(dsb->checksum);
>
> sbi.build_time = le64_to_cpu(dsb->build_time);
> sbi.build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 1c8dea5..b9b46f5 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -424,19 +424,6 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
> return 0;
> }
>
> -#define CRC32C_POLY_LE 0x82F63B78
> -static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
> -{
> - int i;
> -
> - while (len--) {
> - crc ^= *in++;
> - for (i = 0; i < 8; i++)
> - crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
> - }
> - return crc;
> -}
> -
> static int erofs_mkfs_superblock_csum_set(void)
> {
> int ret;
> --
> 2.33.0.1079.g6e70778dc9-goog
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] erofs-utils: introduce fsck.erofs
2021-10-25 23:24 ` Gao Xiang
@ 2021-10-26 2:42 ` Guo Xuenan
2021-10-26 3:12 ` Gao Xiang
2021-10-26 11:34 ` Guo Xuenan
2021-10-26 18:35 ` Daeho Jeong
2 siblings, 1 reply; 13+ messages in thread
From: Guo Xuenan @ 2021-10-26 2:42 UTC (permalink / raw)
To: linux-erofs, xiang, Daeho Jeong; +Cc: miaoxie, Wang Qi
Hi Xiang，
Sorry，WangQi and i have been busy before. I will rearrange the erofs dump
code and send it out in the next few days.
Thanks.
在 2021/10/26 7:24, Gao Xiang 写道:
> (Sorry due to just mutt+vim edit by mistake. try to send properly
> again..)
>
> Hi Daeho,
>
> On Mon, Oct 25, 2021 at 12:48:09PM -0700, Daeho Jeong wrote:
>> From: Daeho Jeong <daehojeong@google.com>
>>
>> I made a fsck.erofs tool to check erofs filesystem image integrity
>> and calculate filesystem compression ratio.
>> Here are options to support now.
>>
>> fsck.erofs [options] IMAGE
>> -V print the version number of fsck.erofs and exit.
>> -d# set output message level to # (maximum 9)\n
>> -c print total compression ratio of all compressed files
>> -C print total compression ratio of all files
>>
>> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> Many thanks for the patch! I also think some fsck feature is useful in
> order to check the image integration.
>
> IMO, Compression ratio calculation is more like a dump fs feature.
> Wang Qi developed the erofsdump yet he told me that he is working for
> another urgent stuffs for now so he delayed the work:
> https://lore.kernel.org/r/20210915093537.2579575-1-guoxuenan@huawei.com
>
> fsck and dump have the similar logic (and like f2fs-tools, we can also
> have one program for this 2 features). If fsck is actively developed,
> I'm pretty fine to use the fsck codebase and merge erofsdump into it
> later instead.
> Some comments as below:
>
>> ---
>> Makefile.am | 2 +-
>> configure.ac | 3 +-
>> fsck/Makefile.am | 9 +
>> fsck/main.c | 548 +++++++++++++++++++++++++++++++++++++++
>> include/erofs/internal.h | 5 +
>> include/erofs_fs.h | 13 +
>> lib/data.c | 4 +-
>> lib/namei.c | 2 +-
>> lib/super.c | 1 +
>> mkfs/main.c | 13 -
>> 10 files changed, 582 insertions(+), 18 deletions(-)
>> create mode 100644 fsck/Makefile.am
>> create mode 100644 fsck/main.c
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 24e1d38..fc464e8 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -2,7 +2,7 @@
>>
>> ACLOCAL_AMFLAGS = -I m4
>>
>> -SUBDIRS = man lib mkfs dump
>> +SUBDIRS = man lib mkfs dump fsck
>> if ENABLE_FUSE
>> SUBDIRS += fuse
>> endif
>> diff --git a/configure.ac b/configure.ac
>> index b2c3225..5698b2e 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -298,5 +298,6 @@ AC_CONFIG_FILES([Makefile
>> lib/Makefile
>> mkfs/Makefile
>> dump/Makefile
>> - fuse/Makefile])
>> + fuse/Makefile
>> + fsck/Makefile])
>> AC_OUTPUT
>> diff --git a/fsck/Makefile.am b/fsck/Makefile.am
>> new file mode 100644
>> index 0000000..82973ba
>> --- /dev/null
>> +++ b/fsck/Makefile.am
>> @@ -0,0 +1,9 @@
>> +# SPDX-License-Identifier: GPL-2.0+
>> +# Makefile.am
>> +
>> +AUTOMAKE_OPTIONS = foreign
>> +bin_PROGRAMS = fsck.erofs
>> +AM_CPPFLAGS = ${libuuid_CFLAGS}
>> +fsck_erofs_SOURCES = main.c
>> +fsck_erofs_CFLAGS = -Wall -Werror -I$(top_srcdir)/include
>> +fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libuuid_LIBS} ${liblz4_LIBS}
>> diff --git a/fsck/main.c b/fsck/main.c
>> new file mode 100644
>> index 0000000..c397d19
>> --- /dev/null
>> +++ b/fsck/main.c
>> @@ -0,0 +1,548 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright 2021 Google LLC
>> + * Author: Daeho Jeong <daehojeong@google.com>
>> + */
>> +#include <stdlib.h>
>> +#include <getopt.h>
>> +#include <time.h>
>> +#include "erofs/print.h"
>> +#include "erofs/io.h"
>> +
>> +enum {
>> + NO_PRINT_COMP_RATIO = 0,
>> + PRINT_COMP_RATIO_ALL = 1,
>> + PRINT_COMP_RATIO_COMP_FILE = 2,
>> +};
>> +
>> +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
>> +
>> +struct erofsfsck_cfg {
>> + bool corrupted;
>> + int print_comp_ratio;
>> + u64 ondisk_len;
>> + u64 logical_len;
>> +};
>> +static struct erofsfsck_cfg fsckcfg;
>> +
>> +static struct option long_options[] = {
>> + {"help", no_argument, 0, 1},
>> + {0, 0, 0, 0},
>> +};
>> +
>> +static void usage(void)
>> +{
>> + fputs("usage: [options] IMAGE\n\n"
>> + "Check erofs filesystem integrity of IMAGE, and [options] are:\n"
>> + " -V print the version number of fsck.erofs and exit.\n"
>> + " -d# set output message level to # (maximum 9)\n"
>> + " -c print total compression ratio of all compressed files\n"
>> + " -C print total compression ratio of all files\n"
> Apart from erofsdump consideration.. Can we merge these 2 options into one?
>
>> + " --help display this help and exit.\n",
>> + stderr);
>> +}
>> +
>> +static void erofsfsck_print_version(void)
>> +{
>> + fprintf(stderr, "fsck.erofs %s\n", cfg.c_version);
>> +}
>> +
>> +static int erofsfsck_parse_options_cfg(int argc, char **argv)
>> +{
>> + int opt, i;
>> +
>> + while ((opt = getopt_long(argc, argv, "Vd:Cc",
>> + long_options, NULL)) != -1) {
>> + switch (opt) {
>> + case 'V':
>> + erofsfsck_print_version();
>> + exit(0);
>> + case 'd':
>> + i = atoi(optarg);
>> + if (i < EROFS_MSG_MIN || i > EROFS_MSG_MAX) {
>> + erofs_err("invalid debug level %d", i);
>> + return -EINVAL;
>> + }
>> + cfg.c_dbg_lvl = i;
>> + break;
>> + case 'C':
>> + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_ALL;
>> + break;
>> + case 'c':
>> + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_COMP_FILE;
>> + break;
>> + case 1:
>> + usage();
>> + exit(0);
>> + default:
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + if (optind >= argc)
>> + return -EINVAL;
>> +
>> + cfg.c_img_path = strdup(argv[optind++]);
>> + if (!cfg.c_img_path)
>> + return -ENOMEM;
>> +
>> + if (optind < argc) {
>> + erofs_err("unexpected argument: %s\n", argv[optind]);
>> + return -EINVAL;
>> + }
>> + return 0;
>> +}
>> +
>> +static int erofs_check_sb_chksum(void)
>> +{
>> + int ret;
>> + u8 buf[EROFS_BLKSIZ];
>> + u32 crc;
>> + struct erofs_super_block *sb;
>> +
>> + ret = blk_read(buf, 0, 1);
>> + if (ret) {
>> + erofs_err("failed to read superblock to check checksum: "
>> + "errno(%d)", ret);
>> + return -1;
>> + }
>> +
>> + sb = (struct erofs_super_block *)(buf + EROFS_SUPER_OFFSET);
>> + sb->checksum = 0;
>> +
>> + crc = crc32c(~0, (u8 *)sb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
>> + if (crc != sbi.checksum) {
>> + erofs_err("superblock checksum doesn't match: saved(0x%08x) "
>> + "calculated(0x%08x)", sbi.checksum, crc);
>> + fsckcfg.corrupted = true;
>> + return -1;
>> + }
>> + return 0;
>> +}
>> +
>> +static int check_special_dentries(struct erofs_dirent *de, const char *de_name,
>> + unsigned int de_namelen, erofs_nid_t nid,
>> + erofs_nid_t pnid, bool is_curdir)
>> +{
>> + unsigned int dirname_len = is_curdir ? 1 : 2;
>> + const char *dirname = is_curdir ? "." : "..";
>> + erofs_nid_t correct_nid = is_curdir ? nid : pnid;
>> +
>> + if (de_namelen != dirname_len || memcmp(de_name, dirname, de_namelen)) {
>> + char *dbgname = strndup(de_name, de_namelen);
>> +
>> + BUG_ON(!dbgname);
>> + if (is_curdir)
>> + erofs_err("wrong current dir name(%s) @ nid(%llu)",
>> + dbgname, nid | 0ULL);
>> + else
>> + erofs_err("wrong parent dir name(%s) @ nid(%llu)",
>> + dbgname, nid | 0ULL);
>> + free(dbgname);
>> + return -1;
>> + }
>> +
>> + if (de->nid != correct_nid) {
>> + if (is_curdir)
>> + erofs_err("wrong current dir nid(%llu) @ nid(%llu)",
>> + de->nid | 0ULL, nid | 0ULL);
>> + else
>> + erofs_err("wrong parent dir nid(%llu): "
>> + "pnid(%llu) @ nid(%llu)",
>> + de->nid | 0ULL, pnid | 0ULL, nid | 0ULL);
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int traverse_dirents(erofs_nid_t pnid, erofs_nid_t nid,
>> + void *dentry_blk, erofs_off_t offset,
>> + unsigned int next_nameoff, unsigned int maxsize)
> Not quite sure if we could introduce some
> erofs_foreach_dirent() and customized callbacks to cleanup similar functions
> in fuse and fsck...
>
>> +{
>> + struct erofs_dirent *de = dentry_blk;
>> + const struct erofs_dirent *end = dentry_blk + next_nameoff;
>> + unsigned int idx = 0;
>> +
>> + erofs_dbg("traversing pnid(%llu), nid(%llu)", pnid | 0ULL, nid | 0ULL);
>> +
>> + if (offset == 0 && (next_nameoff < 2 * sizeof(struct erofs_dirent))) {
>> + erofs_err("too small dirents of size(%d) in nid(%llu)",
>> + next_nameoff, nid | 0ULL);
>> + return -EFSCORRUPTED;
>> + }
>> +
>> + while (de < end) {
>> + const char *de_name;
>> + unsigned int de_namelen;
>> + unsigned int nameoff;
>> + char *dbgname;
>> +
>> + nameoff = le16_to_cpu(de->nameoff);
>> + de_name = (char *)dentry_blk + nameoff;
>> +
>> + /* the last dirent check */
>> + if (de + 1 >= end)
>> + de_namelen = strnlen(de_name, maxsize - nameoff);
>> + else
>> + de_namelen = le16_to_cpu(de[1].nameoff) - nameoff;
>> +
>> + if (cfg.c_dbg_lvl >= EROFS_DBG) {
>> + dbgname = strndup(de_name, de_namelen);
>> + BUG_ON(!dbgname);
>> + erofs_dbg("traversed filename(%s)", dbgname);
>> + free(dbgname);
>> + }
>> +
>> + /* corrupted entry check */
>> + if (nameoff != next_nameoff) {
>> + erofs_err("bogus dirent with nameoff(%u): expected(%d) "
>> + "@ nid(%llu), offset(%llu), idx(%u)",
>> + nameoff, next_nameoff, nid | 0ULL,
>> + offset | 0ULL, idx);
>> + return -EFSCORRUPTED;
>> + }
>> +
>> + if (nameoff + de_namelen > maxsize ||
>> + de_namelen > EROFS_NAME_LEN) {
>> + erofs_err("bogus dirent with namelen(%u) @ nid(%llu), "
>> + "offset(%llu), idx(%u)",
>> + de_namelen, nid | 0ULL, offset | 0ULL, idx);
>> + return -EFSCORRUPTED;
>> + }
>> +
>> + if (offset == 0 && (idx == 0 || idx == 1)) {
> We may not assume "." and ".." special dirents as 0 or 1... Actually
> all dirents are sorted in the alphabetical order. I'm not sure if
> some dirents are smaller than "." and "..".
>
> We could also check if all dirents are in alphabetical order as well
> since it's used to do binary search runtimely.
>
>> + if (check_special_dentries(de, de_name, de_namelen, nid,
>> + pnid, idx == 0))
>> + return -EFSCORRUPTED;
>> + } else {
>> + erofs_check_inode(nid, de->nid);
>> + if (fsckcfg.corrupted)
>> + return -EFSCORRUPTED;
>> + }
>> +
>> + next_nameoff += de_namelen;
>> + ++de;
>> + ++idx;
>> + }
>> +
>> + erofs_dbg("traversing ... done nid(%llu)", nid | 0ULL);
>> + return 0;
>> +}
>> +
>> +static int verify_raw_data_chunk(struct erofs_inode *inode)
>> +{
>> + struct erofs_map_blocks map = {
>> + .index = UINT_MAX,
>> + };
>> + int ret;
>> + erofs_off_t ptr = 0;
>> +
>> + if (fsckcfg.print_comp_ratio == PRINT_COMP_RATIO_ALL) {
>> + fsckcfg.logical_len += inode->i_size;
>> + fsckcfg.ondisk_len += inode->i_size;
>> + }
>> +
>> + while (ptr < inode->i_size) {
>> + map.m_la = ptr;
>> + ret = erofs_map_blocks(inode, &map, 0);
>> + if (ret)
>> + return ret;
>> +
>> + if (map.m_plen != map.m_llen || ptr != map.m_la) {
>> + erofs_err("broken data chunk layout m_la %" PRIu64
>> + " ptr %" PRIu64 " m_llen %" PRIu64 " m_plen %"
>> + PRIu64, map.m_la, ptr, map.m_llen,
>> + map.m_plen);
>> + return -EFSCORRUPTED;
>> + }
>> +
>> + if (!(map.m_flags & EROFS_MAP_MAPPED) && !map.m_llen) {
>> + /* readched EOF */
>> + ptr = inode->i_size;
>> + continue;
>> + }
>> +
>> + ptr += map.m_llen;
>> + }
>> + return 0;
>> +}
>> +
>> +static int verify_compressed_chunk(struct erofs_inode *inode)
>> +{
>> + struct erofs_map_blocks map = {
>> + .index = UINT_MAX,
>> + };
>> + int ret = 0;
>> + bool count_pchunk = fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO;
>> + u64 pchunk_len = 0;
>> + erofs_off_t offset = 0, end = inode->i_size;
>> +
>> + while (end > offset) {
>> + map.m_la = end - 1;
>> +
>> + ret = z_erofs_map_blocks_iter(inode, &map);
>> + if (ret)
>> + return ret;
>> +
>> + if (end > map.m_la + map.m_llen) {
>> + erofs_err("broken compressed chunk layout m_la %" PRIu64
>> + " m_llen %" PRIu64 " end %" PRIu64,
>> + map.m_la, map.m_llen, end);
>> + return -EFSCORRUPTED;
>> + }
> IMO, A simple way to check this is to try to decompress by calling
> z_erofs_decompress() with .partial_decoding == 0 and see if any
> error when decompressing as well.
>
>> +
>> + if (count_pchunk)
>> + pchunk_len += map.m_plen;
>> +
>> + end = map.m_la;
>> + }
>> +
>> + if (count_pchunk) {
>> + fsckcfg.logical_len += inode->i_size;
>> + fsckcfg.ondisk_len += pchunk_len;
> we can also check another field called compressed_blocks in on-disk
> inode...
>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int erofs_verify_xattr(struct erofs_inode *inode)
>> +{
>> + unsigned int xattr_hdr_size = sizeof(struct erofs_xattr_ibody_header);
>> + unsigned int xattr_entry_size = sizeof(struct erofs_xattr_entry);
>> + erofs_off_t addr;
>> + unsigned int ofs, xattr_shared_count;
>> + struct erofs_xattr_ibody_header *ih;
>> + struct erofs_xattr_entry *entry;
>> + int i, remaining = inode->xattr_isize, ret = 0;
>> + char *buf = calloc(EROFS_BLKSIZ, 1);
>> +
>> + BUG_ON(!buf);
>> +
>> + if (inode->xattr_isize == xattr_hdr_size) {
>> + erofs_err("xattr_isize %d of nid %llu is not supported yet",
>> + inode->xattr_isize, inode->nid | 0ULL);
>> + ret = -EFSCORRUPTED;
>> + goto out;
>> + } else if (inode->xattr_isize < xattr_hdr_size) {
>> + if (inode->xattr_isize) {
>> + erofs_err("bogus xattr ibody @ nid %llu",
>> + inode->nid | 0ULL);
>> + ret = -EFSCORRUPTED;
>> + goto out;
>> + }
>> + }
>> +
>> + addr = iloc(inode->nid) + inode->inode_isize;
>> + ret = dev_read(buf, addr, xattr_hdr_size);
>> + if (ret < 0) {
>> + erofs_err("an error occurred when reading xattr header "
>> + "of nid(%llu): errno(%d)", inode->nid | 0ULL, ret);
>> + goto out;
>> + }
>> + ih = (struct erofs_xattr_ibody_header *)buf;
>> + xattr_shared_count = ih->h_shared_count;
>> +
>> + ofs = erofs_blkoff(addr) + xattr_hdr_size;
>> + addr += xattr_hdr_size;
>> + remaining -= xattr_hdr_size;
>> + for (i = 0; i < xattr_shared_count; ++i) {
>> + if (ofs >= EROFS_BLKSIZ) {
>> + if (ofs != EROFS_BLKSIZ) {
>> + erofs_err("unaligned xattr entry in "
>> + "xattr shared area of nid(%llu)",
>> + inode->nid | 0ULL);
>> + ret = -EFSCORRUPTED;
> IMO, it won't happen due to proper alignment.. but I'm fine as this too.
>
> Thanks,
> Gao Xiang
>
>> + goto out;
>> + }
>> + ofs = 0;
>> + }
>> + ofs += xattr_entry_size;
>> + addr += xattr_entry_size;
>> + remaining -= xattr_entry_size;
>> + }
>> +
>> + while (remaining > 0) {
>> + unsigned int entry_sz;
>> +
>> + ret = dev_read(buf, addr, xattr_entry_size);
>> + if (ret) {
>> + erofs_err("an error occurred when reading xattr entry "
>> + "of nid(%llu): errno(%d)",
>> + inode->nid | 0ULL, ret);
>> + goto out;
>> + }
>> +
>> + entry = (struct erofs_xattr_entry *)buf;
>> + entry_sz = erofs_xattr_entry_size(entry);
>> + if (remaining < entry_sz) {
>> + erofs_err("xattr on-disk corruption: xattr entry "
>> + "beyond xattr_isize of nid(%llu)",
>> + inode->nid | 0ULL);
>> + ret = -EFSCORRUPTED;
>> + goto out;
>> + }
>> + addr += entry_sz;
>> + remaining -= entry_sz;
>> + }
>> +out:
>> + free(buf);
>> + return ret;
>> +}
>> +
>> +static int erofs_verify_data_chunk(struct erofs_inode *inode)
>> +{
>> + int ret;
>> +
>> + erofs_dbg("verify data chunk of nid(%llu): type(%d)",
>> + inode->nid | 0ULL, inode->datalayout);
>> +
>> + switch (inode->datalayout) {
>> + case EROFS_INODE_FLAT_PLAIN:
>> + case EROFS_INODE_FLAT_INLINE:
>> + case EROFS_INODE_CHUNK_BASED:
>> + ret = verify_raw_data_chunk(inode);
>> + break;
>> + case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
>> + case EROFS_INODE_FLAT_COMPRESSION:
>> + ret = verify_compressed_chunk(inode);
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> +
>> + if (ret == -EIO)
>> + erofs_err("I/O error occurred when verifying "
>> + "data chunk of nid(%llu)", inode->nid | 0ULL);
>> +
>> + return ret;
>> +}
>> +
>> +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid)
>> +{
>> + int ret;
>> + struct erofs_inode *inode;
>> + char *buf;
>> + erofs_off_t offset;
>> +
>> + erofs_dbg("check inode: nid(%llu)", nid | 0ULL);
>> + inode = calloc(1, sizeof(struct erofs_inode));
>> + BUG_ON(!inode);
>> + buf = calloc(EROFS_BLKSIZ, 1);
>> + BUG_ON(!buf);
>> +
>> + inode->nid = nid;
>> + ret = erofs_read_inode_from_disk(inode);
>> + if (ret) {
>> + if (ret == -EIO)
>> + erofs_err("I/O error occurred when reading nid(%llu)",
>> + nid | 0ULL);
>> + goto out;
>> + }
>> +
>> + /* verify xattr field */
>> + ret = erofs_verify_xattr(inode);
>> + if (ret)
>> + goto out;
>> +
>> + /* verify data chunk layout */
>> + ret = erofs_verify_data_chunk(inode);
>> + if (ret)
>> + goto out;
>> +
>> + if ((inode->i_mode & S_IFMT) != S_IFDIR)
>> + goto out;
>> +
>> + offset = 0;
>> + while (offset < inode->i_size) {
>> + erofs_off_t maxsize = min_t(erofs_off_t,
>> + inode->i_size - offset, EROFS_BLKSIZ);
>> + struct erofs_dirent *de = (void *)buf;
>> + unsigned int nameoff;
>> +
>> + ret = erofs_pread(inode, buf, maxsize, offset);
>> + if (ret) {
>> + erofs_err("I/O error occurred when reading dirents: "
>> + "nid(%llu), offset(%llu), size(%llu)",
>> + nid | 0ULL, offset | 0ULL, maxsize | 0ULL);
>> + goto out;
>> + }
>> +
>> + nameoff = le16_to_cpu(de->nameoff);
>> + if (nameoff < sizeof(struct erofs_dirent) ||
>> + nameoff >= PAGE_SIZE) {
>> + erofs_err("invalid de[0].nameoff %u @ nid(%llu), "
>> + "offset(%llu)",
>> + nameoff, nid | 0ULL, offset | 0ULL);
>> + ret = -EFSCORRUPTED;
>> + goto out;
>> + }
>> +
>> + ret = traverse_dirents(pnid, nid, buf, offset,
>> + nameoff, maxsize);
>> + if (ret)
>> + goto out;
>> +
>> + offset += maxsize;
>> + }
>> +out:
>> + free(buf);
>> + free(inode);
>> + if (ret && ret != -EIO)
>> + fsckcfg.corrupted = true;
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> + int err;
>> +
>> + erofs_init_configure();
>> +
>> + fsckcfg.corrupted = false;
>> + fsckcfg.print_comp_ratio = NO_PRINT_COMP_RATIO;
>> + fsckcfg.logical_len = 0;
>> + fsckcfg.ondisk_len = 0;
>> +
>> + err = erofsfsck_parse_options_cfg(argc, argv);
>> + if (err) {
>> + if (err == -EINVAL)
>> + usage();
>> + goto exit;
>> + }
>> +
>> + err = dev_open_ro(cfg.c_img_path);
>> + if (err) {
>> + erofs_err("failed to open image file");
>> + goto exit;
>> + }
>> +
>> + err = erofs_read_superblock();
>> + if (err) {
>> + erofs_err("failed to read superblock");
>> + goto exit;
>> + }
>> +
>> + if (erofs_sb_has_sb_chksum() && erofs_check_sb_chksum()) {
>> + erofs_err("failed to verify superblock checksum");
>> + goto exit;
>> + }
>> +
>> + erofs_check_inode(sbi.root_nid, sbi.root_nid);
>> +
>> + if (fsckcfg.corrupted) {
>> + fprintf(stderr, "Found some filesystem corruption\n");
>> + } else {
>> + fprintf(stderr, "No error found\n");
>> + if (fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO) {
>> + double comp_ratio = (double)fsckcfg.ondisk_len * 100 /
>> + (double)fsckcfg.logical_len;
>> + fprintf(stderr, "Compression Ratio: %.2f(%%)\n",
>> + comp_ratio);
>> + }
>> + }
>> +
>> +exit:
>> + erofs_exit_configure();
>> + return err;
>> +}
>> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
>> index 8b154ed..80065b2 100644
>> --- a/include/erofs/internal.h
>> +++ b/include/erofs/internal.h
>> @@ -82,6 +82,8 @@ struct erofs_sb_info {
>>
>> u16 available_compr_algs;
>> u16 lz4_max_distance;
>> +
>> + u32 checksum;
>> };
>>
>> /* global sbi */
>> @@ -264,10 +266,13 @@ int erofs_read_superblock(void);
>>
>> /* namei.c */
>> int erofs_ilookup(const char *path, struct erofs_inode *vi);
>> +int erofs_read_inode_from_disk(struct erofs_inode *vi);
>>
>> /* data.c */
>> int erofs_pread(struct erofs_inode *inode, char *buf,
>> erofs_off_t count, erofs_off_t offset);
>> +int erofs_map_blocks(struct erofs_inode *inode,
>> + struct erofs_map_blocks *map, int flags);
>> /* zmap.c */
>> int z_erofs_fill_inode(struct erofs_inode *vi);
>> int z_erofs_map_blocks_iter(struct erofs_inode *vi,
>> diff --git a/include/erofs_fs.h b/include/erofs_fs.h
>> index 66a68e3..62e9981 100644
>> --- a/include/erofs_fs.h
>> +++ b/include/erofs_fs.h
>> @@ -400,4 +400,17 @@ static inline void erofs_check_ondisk_layout_definitions(void)
>> Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
>> }
>>
>> +#define CRC32C_POLY_LE 0x82F63B78
>> +static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
>> +{
>> + int i;
>> +
>> + while (len--) {
>> + crc ^= *in++;
>> + for (i = 0; i < 8; i++)
>> + crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
>> + }
>> + return crc;
>> +}
>> +
>> #endif
>> diff --git a/lib/data.c b/lib/data.c
>> index 641d840..6cb7eeb 100644
>> --- a/lib/data.c
>> +++ b/lib/data.c
>> @@ -61,8 +61,8 @@ err_out:
>> return err;
>> }
>>
>> -static int erofs_map_blocks(struct erofs_inode *inode,
>> - struct erofs_map_blocks *map, int flags)
>> +int erofs_map_blocks(struct erofs_inode *inode,
>> + struct erofs_map_blocks *map, int flags)
>> {
>> struct erofs_inode *vi = inode;
>> struct erofs_inode_chunk_index *idx;
>> diff --git a/lib/namei.c b/lib/namei.c
>> index b4bdabf..56f199a 100644
>> --- a/lib/namei.c
>> +++ b/lib/namei.c
>> @@ -22,7 +22,7 @@ static dev_t erofs_new_decode_dev(u32 dev)
>> return makedev(major, minor);
>> }
>>
>> -static int erofs_read_inode_from_disk(struct erofs_inode *vi)
>> +int erofs_read_inode_from_disk(struct erofs_inode *vi)
>> {
>> int ret, ifmt;
>> char buf[sizeof(struct erofs_inode_extended)];
>> diff --git a/lib/super.c b/lib/super.c
>> index 0fa69ab..0c30403 100644
>> --- a/lib/super.c
>> +++ b/lib/super.c
>> @@ -62,6 +62,7 @@ int erofs_read_superblock(void)
>> sbi.islotbits = EROFS_ISLOTBITS;
>> sbi.root_nid = le16_to_cpu(dsb->root_nid);
>> sbi.inos = le64_to_cpu(dsb->inos);
>> + sbi.checksum = le32_to_cpu(dsb->checksum);
>>
>> sbi.build_time = le64_to_cpu(dsb->build_time);
>> sbi.build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
>> diff --git a/mkfs/main.c b/mkfs/main.c
>> index 1c8dea5..b9b46f5 100644
>> --- a/mkfs/main.c
>> +++ b/mkfs/main.c
>> @@ -424,19 +424,6 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
>> return 0;
>> }
>>
>> -#define CRC32C_POLY_LE 0x82F63B78
>> -static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
>> -{
>> - int i;
>> -
>> - while (len--) {
>> - crc ^= *in++;
>> - for (i = 0; i < 8; i++)
>> - crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
>> - }
>> - return crc;
>> -}
>> -
>> static int erofs_mkfs_superblock_csum_set(void)
>> {
>> int ret;
>> --
>> 2.33.0.1079.g6e70778dc9-goog
>>
> .
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] erofs-utils: introduce fsck.erofs
2021-10-26 2:42 ` Guo Xuenan
@ 2021-10-26 3:12 ` Gao Xiang
0 siblings, 0 replies; 13+ messages in thread
From: Gao Xiang @ 2021-10-26 3:12 UTC (permalink / raw)
To: Guo Xuenan; +Cc: Daeho Jeong, linux-erofs, miaoxie, Wang Qi
On Tue, Oct 26, 2021 at 10:42:12AM +0800, Guo Xuenan wrote:
> Hi Xiang，
>
> Sorry，WangQi and i have been busy before. I will rearrange the erofs dump
>
> code and send it out in the next few days.
>
Yeah, anyway, many thanks all of you for fsck and dump features!
Let's sort out and get into shape smoothly.
Thanks,
Gao Xiang
> Thanks.
>
> 在 2021/10/26 7:24, Gao Xiang 写道:
>
> > (Sorry due to just mutt+vim edit by mistake. try to send properly
> > again..)
> >
> > Hi Daeho,
> >
> > On Mon, Oct 25, 2021 at 12:48:09PM -0700, Daeho Jeong wrote:
> > > From: Daeho Jeong <daehojeong@google.com>
> > >
> > > I made a fsck.erofs tool to check erofs filesystem image integrity
> > > and calculate filesystem compression ratio.
> > > Here are options to support now.
> > >
> > > fsck.erofs [options] IMAGE
> > > -V print the version number of fsck.erofs and exit.
> > > -d# set output message level to # (maximum 9)\n
> > > -c print total compression ratio of all compressed files
> > > -C print total compression ratio of all files
> > >
> > > Signed-off-by: Daeho Jeong <daehojeong@google.com>
> > Many thanks for the patch! I also think some fsck feature is useful in
> > order to check the image integration.
> >
> > IMO, Compression ratio calculation is more like a dump fs feature.
> > Wang Qi developed the erofsdump yet he told me that he is working for
> > another urgent stuffs for now so he delayed the work:
> > https://lore.kernel.org/r/20210915093537.2579575-1-guoxuenan@huawei.com
> >
> > fsck and dump have the similar logic (and like f2fs-tools, we can also
> > have one program for this 2 features). If fsck is actively developed,
> > I'm pretty fine to use the fsck codebase and merge erofsdump into it
> > later instead.
> > Some comments as below:
> >
> > > ---
> > > Makefile.am | 2 +-
> > > configure.ac | 3 +-
> > > fsck/Makefile.am | 9 +
> > > fsck/main.c | 548 +++++++++++++++++++++++++++++++++++++++
> > > include/erofs/internal.h | 5 +
> > > include/erofs_fs.h | 13 +
> > > lib/data.c | 4 +-
> > > lib/namei.c | 2 +-
> > > lib/super.c | 1 +
> > > mkfs/main.c | 13 -
> > > 10 files changed, 582 insertions(+), 18 deletions(-)
> > > create mode 100644 fsck/Makefile.am
> > > create mode 100644 fsck/main.c
> > >
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 24e1d38..fc464e8 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -2,7 +2,7 @@
> > > ACLOCAL_AMFLAGS = -I m4
> > > -SUBDIRS = man lib mkfs dump
> > > +SUBDIRS = man lib mkfs dump fsck
> > > if ENABLE_FUSE
> > > SUBDIRS += fuse
> > > endif
> > > diff --git a/configure.ac b/configure.ac
> > > index b2c3225..5698b2e 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -298,5 +298,6 @@ AC_CONFIG_FILES([Makefile
> > > lib/Makefile
> > > mkfs/Makefile
> > > dump/Makefile
> > > - fuse/Makefile])
> > > + fuse/Makefile
> > > + fsck/Makefile])
> > > AC_OUTPUT
> > > diff --git a/fsck/Makefile.am b/fsck/Makefile.am
> > > new file mode 100644
> > > index 0000000..82973ba
> > > --- /dev/null
> > > +++ b/fsck/Makefile.am
> > > @@ -0,0 +1,9 @@
> > > +# SPDX-License-Identifier: GPL-2.0+
> > > +# Makefile.am
> > > +
> > > +AUTOMAKE_OPTIONS = foreign
> > > +bin_PROGRAMS = fsck.erofs
> > > +AM_CPPFLAGS = ${libuuid_CFLAGS}
> > > +fsck_erofs_SOURCES = main.c
> > > +fsck_erofs_CFLAGS = -Wall -Werror -I$(top_srcdir)/include
> > > +fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libuuid_LIBS} ${liblz4_LIBS}
> > > diff --git a/fsck/main.c b/fsck/main.c
> > > new file mode 100644
> > > index 0000000..c397d19
> > > --- /dev/null
> > > +++ b/fsck/main.c
> > > @@ -0,0 +1,548 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright 2021 Google LLC
> > > + * Author: Daeho Jeong <daehojeong@google.com>
> > > + */
> > > +#include <stdlib.h>
> > > +#include <getopt.h>
> > > +#include <time.h>
> > > +#include "erofs/print.h"
> > > +#include "erofs/io.h"
> > > +
> > > +enum {
> > > + NO_PRINT_COMP_RATIO = 0,
> > > + PRINT_COMP_RATIO_ALL = 1,
> > > + PRINT_COMP_RATIO_COMP_FILE = 2,
> > > +};
> > > +
> > > +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
> > > +
> > > +struct erofsfsck_cfg {
> > > + bool corrupted;
> > > + int print_comp_ratio;
> > > + u64 ondisk_len;
> > > + u64 logical_len;
> > > +};
> > > +static struct erofsfsck_cfg fsckcfg;
> > > +
> > > +static struct option long_options[] = {
> > > + {"help", no_argument, 0, 1},
> > > + {0, 0, 0, 0},
> > > +};
> > > +
> > > +static void usage(void)
> > > +{
> > > + fputs("usage: [options] IMAGE\n\n"
> > > + "Check erofs filesystem integrity of IMAGE, and [options] are:\n"
> > > + " -V print the version number of fsck.erofs and exit.\n"
> > > + " -d# set output message level to # (maximum 9)\n"
> > > + " -c print total compression ratio of all compressed files\n"
> > > + " -C print total compression ratio of all files\n"
> > Apart from erofsdump consideration.. Can we merge these 2 options into one?
> >
> > > + " --help display this help and exit.\n",
> > > + stderr);
> > > +}
> > > +
> > > +static void erofsfsck_print_version(void)
> > > +{
> > > + fprintf(stderr, "fsck.erofs %s\n", cfg.c_version);
> > > +}
> > > +
> > > +static int erofsfsck_parse_options_cfg(int argc, char **argv)
> > > +{
> > > + int opt, i;
> > > +
> > > + while ((opt = getopt_long(argc, argv, "Vd:Cc",
> > > + long_options, NULL)) != -1) {
> > > + switch (opt) {
> > > + case 'V':
> > > + erofsfsck_print_version();
> > > + exit(0);
> > > + case 'd':
> > > + i = atoi(optarg);
> > > + if (i < EROFS_MSG_MIN || i > EROFS_MSG_MAX) {
> > > + erofs_err("invalid debug level %d", i);
> > > + return -EINVAL;
> > > + }
> > > + cfg.c_dbg_lvl = i;
> > > + break;
> > > + case 'C':
> > > + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_ALL;
> > > + break;
> > > + case 'c':
> > > + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_COMP_FILE;
> > > + break;
> > > + case 1:
> > > + usage();
> > > + exit(0);
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > + }
> > > +
> > > + if (optind >= argc)
> > > + return -EINVAL;
> > > +
> > > + cfg.c_img_path = strdup(argv[optind++]);
> > > + if (!cfg.c_img_path)
> > > + return -ENOMEM;
> > > +
> > > + if (optind < argc) {
> > > + erofs_err("unexpected argument: %s\n", argv[optind]);
> > > + return -EINVAL;
> > > + }
> > > + return 0;
> > > +}
> > > +
> > > +static int erofs_check_sb_chksum(void)
> > > +{
> > > + int ret;
> > > + u8 buf[EROFS_BLKSIZ];
> > > + u32 crc;
> > > + struct erofs_super_block *sb;
> > > +
> > > + ret = blk_read(buf, 0, 1);
> > > + if (ret) {
> > > + erofs_err("failed to read superblock to check checksum: "
> > > + "errno(%d)", ret);
> > > + return -1;
> > > + }
> > > +
> > > + sb = (struct erofs_super_block *)(buf + EROFS_SUPER_OFFSET);
> > > + sb->checksum = 0;
> > > +
> > > + crc = crc32c(~0, (u8 *)sb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
> > > + if (crc != sbi.checksum) {
> > > + erofs_err("superblock checksum doesn't match: saved(0x%08x) "
> > > + "calculated(0x%08x)", sbi.checksum, crc);
> > > + fsckcfg.corrupted = true;
> > > + return -1;
> > > + }
> > > + return 0;
> > > +}
> > > +
> > > +static int check_special_dentries(struct erofs_dirent *de, const char *de_name,
> > > + unsigned int de_namelen, erofs_nid_t nid,
> > > + erofs_nid_t pnid, bool is_curdir)
> > > +{
> > > + unsigned int dirname_len = is_curdir ? 1 : 2;
> > > + const char *dirname = is_curdir ? "." : "..";
> > > + erofs_nid_t correct_nid = is_curdir ? nid : pnid;
> > > +
> > > + if (de_namelen != dirname_len || memcmp(de_name, dirname, de_namelen)) {
> > > + char *dbgname = strndup(de_name, de_namelen);
> > > +
> > > + BUG_ON(!dbgname);
> > > + if (is_curdir)
> > > + erofs_err("wrong current dir name(%s) @ nid(%llu)",
> > > + dbgname, nid | 0ULL);
> > > + else
> > > + erofs_err("wrong parent dir name(%s) @ nid(%llu)",
> > > + dbgname, nid | 0ULL);
> > > + free(dbgname);
> > > + return -1;
> > > + }
> > > +
> > > + if (de->nid != correct_nid) {
> > > + if (is_curdir)
> > > + erofs_err("wrong current dir nid(%llu) @ nid(%llu)",
> > > + de->nid | 0ULL, nid | 0ULL);
> > > + else
> > > + erofs_err("wrong parent dir nid(%llu): "
> > > + "pnid(%llu) @ nid(%llu)",
> > > + de->nid | 0ULL, pnid | 0ULL, nid | 0ULL);
> > > + return -1;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int traverse_dirents(erofs_nid_t pnid, erofs_nid_t nid,
> > > + void *dentry_blk, erofs_off_t offset,
> > > + unsigned int next_nameoff, unsigned int maxsize)
> > Not quite sure if we could introduce some
> > erofs_foreach_dirent() and customized callbacks to cleanup similar functions
> > in fuse and fsck...
> >
> > > +{
> > > + struct erofs_dirent *de = dentry_blk;
> > > + const struct erofs_dirent *end = dentry_blk + next_nameoff;
> > > + unsigned int idx = 0;
> > > +
> > > + erofs_dbg("traversing pnid(%llu), nid(%llu)", pnid | 0ULL, nid | 0ULL);
> > > +
> > > + if (offset == 0 && (next_nameoff < 2 * sizeof(struct erofs_dirent))) {
> > > + erofs_err("too small dirents of size(%d) in nid(%llu)",
> > > + next_nameoff, nid | 0ULL);
> > > + return -EFSCORRUPTED;
> > > + }
> > > +
> > > + while (de < end) {
> > > + const char *de_name;
> > > + unsigned int de_namelen;
> > > + unsigned int nameoff;
> > > + char *dbgname;
> > > +
> > > + nameoff = le16_to_cpu(de->nameoff);
> > > + de_name = (char *)dentry_blk + nameoff;
> > > +
> > > + /* the last dirent check */
> > > + if (de + 1 >= end)
> > > + de_namelen = strnlen(de_name, maxsize - nameoff);
> > > + else
> > > + de_namelen = le16_to_cpu(de[1].nameoff) - nameoff;
> > > +
> > > + if (cfg.c_dbg_lvl >= EROFS_DBG) {
> > > + dbgname = strndup(de_name, de_namelen);
> > > + BUG_ON(!dbgname);
> > > + erofs_dbg("traversed filename(%s)", dbgname);
> > > + free(dbgname);
> > > + }
> > > +
> > > + /* corrupted entry check */
> > > + if (nameoff != next_nameoff) {
> > > + erofs_err("bogus dirent with nameoff(%u): expected(%d) "
> > > + "@ nid(%llu), offset(%llu), idx(%u)",
> > > + nameoff, next_nameoff, nid | 0ULL,
> > > + offset | 0ULL, idx);
> > > + return -EFSCORRUPTED;
> > > + }
> > > +
> > > + if (nameoff + de_namelen > maxsize ||
> > > + de_namelen > EROFS_NAME_LEN) {
> > > + erofs_err("bogus dirent with namelen(%u) @ nid(%llu), "
> > > + "offset(%llu), idx(%u)",
> > > + de_namelen, nid | 0ULL, offset | 0ULL, idx);
> > > + return -EFSCORRUPTED;
> > > + }
> > > +
> > > + if (offset == 0 && (idx == 0 || idx == 1)) {
> > We may not assume "." and ".." special dirents as 0 or 1... Actually
> > all dirents are sorted in the alphabetical order. I'm not sure if
> > some dirents are smaller than "." and "..".
> >
> > We could also check if all dirents are in alphabetical order as well
> > since it's used to do binary search runtimely.
> >
> > > + if (check_special_dentries(de, de_name, de_namelen, nid,
> > > + pnid, idx == 0))
> > > + return -EFSCORRUPTED;
> > > + } else {
> > > + erofs_check_inode(nid, de->nid);
> > > + if (fsckcfg.corrupted)
> > > + return -EFSCORRUPTED;
> > > + }
> > > +
> > > + next_nameoff += de_namelen;
> > > + ++de;
> > > + ++idx;
> > > + }
> > > +
> > > + erofs_dbg("traversing ... done nid(%llu)", nid | 0ULL);
> > > + return 0;
> > > +}
> > > +
> > > +static int verify_raw_data_chunk(struct erofs_inode *inode)
> > > +{
> > > + struct erofs_map_blocks map = {
> > > + .index = UINT_MAX,
> > > + };
> > > + int ret;
> > > + erofs_off_t ptr = 0;
> > > +
> > > + if (fsckcfg.print_comp_ratio == PRINT_COMP_RATIO_ALL) {
> > > + fsckcfg.logical_len += inode->i_size;
> > > + fsckcfg.ondisk_len += inode->i_size;
> > > + }
> > > +
> > > + while (ptr < inode->i_size) {
> > > + map.m_la = ptr;
> > > + ret = erofs_map_blocks(inode, &map, 0);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + if (map.m_plen != map.m_llen || ptr != map.m_la) {
> > > + erofs_err("broken data chunk layout m_la %" PRIu64
> > > + " ptr %" PRIu64 " m_llen %" PRIu64 " m_plen %"
> > > + PRIu64, map.m_la, ptr, map.m_llen,
> > > + map.m_plen);
> > > + return -EFSCORRUPTED;
> > > + }
> > > +
> > > + if (!(map.m_flags & EROFS_MAP_MAPPED) && !map.m_llen) {
> > > + /* readched EOF */
> > > + ptr = inode->i_size;
> > > + continue;
> > > + }
> > > +
> > > + ptr += map.m_llen;
> > > + }
> > > + return 0;
> > > +}
> > > +
> > > +static int verify_compressed_chunk(struct erofs_inode *inode)
> > > +{
> > > + struct erofs_map_blocks map = {
> > > + .index = UINT_MAX,
> > > + };
> > > + int ret = 0;
> > > + bool count_pchunk = fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO;
> > > + u64 pchunk_len = 0;
> > > + erofs_off_t offset = 0, end = inode->i_size;
> > > +
> > > + while (end > offset) {
> > > + map.m_la = end - 1;
> > > +
> > > + ret = z_erofs_map_blocks_iter(inode, &map);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + if (end > map.m_la + map.m_llen) {
> > > + erofs_err("broken compressed chunk layout m_la %" PRIu64
> > > + " m_llen %" PRIu64 " end %" PRIu64,
> > > + map.m_la, map.m_llen, end);
> > > + return -EFSCORRUPTED;
> > > + }
> > IMO, A simple way to check this is to try to decompress by calling
> > z_erofs_decompress() with .partial_decoding == 0 and see if any
> > error when decompressing as well.
> >
> > > +
> > > + if (count_pchunk)
> > > + pchunk_len += map.m_plen;
> > > +
> > > + end = map.m_la;
> > > + }
> > > +
> > > + if (count_pchunk) {
> > > + fsckcfg.logical_len += inode->i_size;
> > > + fsckcfg.ondisk_len += pchunk_len;
> > we can also check another field called compressed_blocks in on-disk
> > inode...
> >
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int erofs_verify_xattr(struct erofs_inode *inode)
> > > +{
> > > + unsigned int xattr_hdr_size = sizeof(struct erofs_xattr_ibody_header);
> > > + unsigned int xattr_entry_size = sizeof(struct erofs_xattr_entry);
> > > + erofs_off_t addr;
> > > + unsigned int ofs, xattr_shared_count;
> > > + struct erofs_xattr_ibody_header *ih;
> > > + struct erofs_xattr_entry *entry;
> > > + int i, remaining = inode->xattr_isize, ret = 0;
> > > + char *buf = calloc(EROFS_BLKSIZ, 1);
> > > +
> > > + BUG_ON(!buf);
> > > +
> > > + if (inode->xattr_isize == xattr_hdr_size) {
> > > + erofs_err("xattr_isize %d of nid %llu is not supported yet",
> > > + inode->xattr_isize, inode->nid | 0ULL);
> > > + ret = -EFSCORRUPTED;
> > > + goto out;
> > > + } else if (inode->xattr_isize < xattr_hdr_size) {
> > > + if (inode->xattr_isize) {
> > > + erofs_err("bogus xattr ibody @ nid %llu",
> > > + inode->nid | 0ULL);
> > > + ret = -EFSCORRUPTED;
> > > + goto out;
> > > + }
> > > + }
> > > +
> > > + addr = iloc(inode->nid) + inode->inode_isize;
> > > + ret = dev_read(buf, addr, xattr_hdr_size);
> > > + if (ret < 0) {
> > > + erofs_err("an error occurred when reading xattr header "
> > > + "of nid(%llu): errno(%d)", inode->nid | 0ULL, ret);
> > > + goto out;
> > > + }
> > > + ih = (struct erofs_xattr_ibody_header *)buf;
> > > + xattr_shared_count = ih->h_shared_count;
> > > +
> > > + ofs = erofs_blkoff(addr) + xattr_hdr_size;
> > > + addr += xattr_hdr_size;
> > > + remaining -= xattr_hdr_size;
> > > + for (i = 0; i < xattr_shared_count; ++i) {
> > > + if (ofs >= EROFS_BLKSIZ) {
> > > + if (ofs != EROFS_BLKSIZ) {
> > > + erofs_err("unaligned xattr entry in "
> > > + "xattr shared area of nid(%llu)",
> > > + inode->nid | 0ULL);
> > > + ret = -EFSCORRUPTED;
> > IMO, it won't happen due to proper alignment.. but I'm fine as this too.
> >
> > Thanks,
> > Gao Xiang
> >
> > > + goto out;
> > > + }
> > > + ofs = 0;
> > > + }
> > > + ofs += xattr_entry_size;
> > > + addr += xattr_entry_size;
> > > + remaining -= xattr_entry_size;
> > > + }
> > > +
> > > + while (remaining > 0) {
> > > + unsigned int entry_sz;
> > > +
> > > + ret = dev_read(buf, addr, xattr_entry_size);
> > > + if (ret) {
> > > + erofs_err("an error occurred when reading xattr entry "
> > > + "of nid(%llu): errno(%d)",
> > > + inode->nid | 0ULL, ret);
> > > + goto out;
> > > + }
> > > +
> > > + entry = (struct erofs_xattr_entry *)buf;
> > > + entry_sz = erofs_xattr_entry_size(entry);
> > > + if (remaining < entry_sz) {
> > > + erofs_err("xattr on-disk corruption: xattr entry "
> > > + "beyond xattr_isize of nid(%llu)",
> > > + inode->nid | 0ULL);
> > > + ret = -EFSCORRUPTED;
> > > + goto out;
> > > + }
> > > + addr += entry_sz;
> > > + remaining -= entry_sz;
> > > + }
> > > +out:
> > > + free(buf);
> > > + return ret;
> > > +}
> > > +
> > > +static int erofs_verify_data_chunk(struct erofs_inode *inode)
> > > +{
> > > + int ret;
> > > +
> > > + erofs_dbg("verify data chunk of nid(%llu): type(%d)",
> > > + inode->nid | 0ULL, inode->datalayout);
> > > +
> > > + switch (inode->datalayout) {
> > > + case EROFS_INODE_FLAT_PLAIN:
> > > + case EROFS_INODE_FLAT_INLINE:
> > > + case EROFS_INODE_CHUNK_BASED:
> > > + ret = verify_raw_data_chunk(inode);
> > > + break;
> > > + case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
> > > + case EROFS_INODE_FLAT_COMPRESSION:
> > > + ret = verify_compressed_chunk(inode);
> > > + break;
> > > + default:
> > > + ret = -EINVAL;
> > > + break;
> > > + }
> > > +
> > > + if (ret == -EIO)
> > > + erofs_err("I/O error occurred when verifying "
> > > + "data chunk of nid(%llu)", inode->nid | 0ULL);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid)
> > > +{
> > > + int ret;
> > > + struct erofs_inode *inode;
> > > + char *buf;
> > > + erofs_off_t offset;
> > > +
> > > + erofs_dbg("check inode: nid(%llu)", nid | 0ULL);
> > > + inode = calloc(1, sizeof(struct erofs_inode));
> > > + BUG_ON(!inode);
> > > + buf = calloc(EROFS_BLKSIZ, 1);
> > > + BUG_ON(!buf);
> > > +
> > > + inode->nid = nid;
> > > + ret = erofs_read_inode_from_disk(inode);
> > > + if (ret) {
> > > + if (ret == -EIO)
> > > + erofs_err("I/O error occurred when reading nid(%llu)",
> > > + nid | 0ULL);
> > > + goto out;
> > > + }
> > > +
> > > + /* verify xattr field */
> > > + ret = erofs_verify_xattr(inode);
> > > + if (ret)
> > > + goto out;
> > > +
> > > + /* verify data chunk layout */
> > > + ret = erofs_verify_data_chunk(inode);
> > > + if (ret)
> > > + goto out;
> > > +
> > > + if ((inode->i_mode & S_IFMT) != S_IFDIR)
> > > + goto out;
> > > +
> > > + offset = 0;
> > > + while (offset < inode->i_size) {
> > > + erofs_off_t maxsize = min_t(erofs_off_t,
> > > + inode->i_size - offset, EROFS_BLKSIZ);
> > > + struct erofs_dirent *de = (void *)buf;
> > > + unsigned int nameoff;
> > > +
> > > + ret = erofs_pread(inode, buf, maxsize, offset);
> > > + if (ret) {
> > > + erofs_err("I/O error occurred when reading dirents: "
> > > + "nid(%llu), offset(%llu), size(%llu)",
> > > + nid | 0ULL, offset | 0ULL, maxsize | 0ULL);
> > > + goto out;
> > > + }
> > > +
> > > + nameoff = le16_to_cpu(de->nameoff);
> > > + if (nameoff < sizeof(struct erofs_dirent) ||
> > > + nameoff >= PAGE_SIZE) {
> > > + erofs_err("invalid de[0].nameoff %u @ nid(%llu), "
> > > + "offset(%llu)",
> > > + nameoff, nid | 0ULL, offset | 0ULL);
> > > + ret = -EFSCORRUPTED;
> > > + goto out;
> > > + }
> > > +
> > > + ret = traverse_dirents(pnid, nid, buf, offset,
> > > + nameoff, maxsize);
> > > + if (ret)
> > > + goto out;
> > > +
> > > + offset += maxsize;
> > > + }
> > > +out:
> > > + free(buf);
> > > + free(inode);
> > > + if (ret && ret != -EIO)
> > > + fsckcfg.corrupted = true;
> > > +}
> > > +
> > > +int main(int argc, char **argv)
> > > +{
> > > + int err;
> > > +
> > > + erofs_init_configure();
> > > +
> > > + fsckcfg.corrupted = false;
> > > + fsckcfg.print_comp_ratio = NO_PRINT_COMP_RATIO;
> > > + fsckcfg.logical_len = 0;
> > > + fsckcfg.ondisk_len = 0;
> > > +
> > > + err = erofsfsck_parse_options_cfg(argc, argv);
> > > + if (err) {
> > > + if (err == -EINVAL)
> > > + usage();
> > > + goto exit;
> > > + }
> > > +
> > > + err = dev_open_ro(cfg.c_img_path);
> > > + if (err) {
> > > + erofs_err("failed to open image file");
> > > + goto exit;
> > > + }
> > > +
> > > + err = erofs_read_superblock();
> > > + if (err) {
> > > + erofs_err("failed to read superblock");
> > > + goto exit;
> > > + }
> > > +
> > > + if (erofs_sb_has_sb_chksum() && erofs_check_sb_chksum()) {
> > > + erofs_err("failed to verify superblock checksum");
> > > + goto exit;
> > > + }
> > > +
> > > + erofs_check_inode(sbi.root_nid, sbi.root_nid);
> > > +
> > > + if (fsckcfg.corrupted) {
> > > + fprintf(stderr, "Found some filesystem corruption\n");
> > > + } else {
> > > + fprintf(stderr, "No error found\n");
> > > + if (fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO) {
> > > + double comp_ratio = (double)fsckcfg.ondisk_len * 100 /
> > > + (double)fsckcfg.logical_len;
> > > + fprintf(stderr, "Compression Ratio: %.2f(%%)\n",
> > > + comp_ratio);
> > > + }
> > > + }
> > > +
> > > +exit:
> > > + erofs_exit_configure();
> > > + return err;
> > > +}
> > > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > > index 8b154ed..80065b2 100644
> > > --- a/include/erofs/internal.h
> > > +++ b/include/erofs/internal.h
> > > @@ -82,6 +82,8 @@ struct erofs_sb_info {
> > > u16 available_compr_algs;
> > > u16 lz4_max_distance;
> > > +
> > > + u32 checksum;
> > > };
> > > /* global sbi */
> > > @@ -264,10 +266,13 @@ int erofs_read_superblock(void);
> > > /* namei.c */
> > > int erofs_ilookup(const char *path, struct erofs_inode *vi);
> > > +int erofs_read_inode_from_disk(struct erofs_inode *vi);
> > > /* data.c */
> > > int erofs_pread(struct erofs_inode *inode, char *buf,
> > > erofs_off_t count, erofs_off_t offset);
> > > +int erofs_map_blocks(struct erofs_inode *inode,
> > > + struct erofs_map_blocks *map, int flags);
> > > /* zmap.c */
> > > int z_erofs_fill_inode(struct erofs_inode *vi);
> > > int z_erofs_map_blocks_iter(struct erofs_inode *vi,
> > > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > > index 66a68e3..62e9981 100644
> > > --- a/include/erofs_fs.h
> > > +++ b/include/erofs_fs.h
> > > @@ -400,4 +400,17 @@ static inline void erofs_check_ondisk_layout_definitions(void)
> > > Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
> > > }
> > > +#define CRC32C_POLY_LE 0x82F63B78
> > > +static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
> > > +{
> > > + int i;
> > > +
> > > + while (len--) {
> > > + crc ^= *in++;
> > > + for (i = 0; i < 8; i++)
> > > + crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
> > > + }
> > > + return crc;
> > > +}
> > > +
> > > #endif
> > > diff --git a/lib/data.c b/lib/data.c
> > > index 641d840..6cb7eeb 100644
> > > --- a/lib/data.c
> > > +++ b/lib/data.c
> > > @@ -61,8 +61,8 @@ err_out:
> > > return err;
> > > }
> > > -static int erofs_map_blocks(struct erofs_inode *inode,
> > > - struct erofs_map_blocks *map, int flags)
> > > +int erofs_map_blocks(struct erofs_inode *inode,
> > > + struct erofs_map_blocks *map, int flags)
> > > {
> > > struct erofs_inode *vi = inode;
> > > struct erofs_inode_chunk_index *idx;
> > > diff --git a/lib/namei.c b/lib/namei.c
> > > index b4bdabf..56f199a 100644
> > > --- a/lib/namei.c
> > > +++ b/lib/namei.c
> > > @@ -22,7 +22,7 @@ static dev_t erofs_new_decode_dev(u32 dev)
> > > return makedev(major, minor);
> > > }
> > > -static int erofs_read_inode_from_disk(struct erofs_inode *vi)
> > > +int erofs_read_inode_from_disk(struct erofs_inode *vi)
> > > {
> > > int ret, ifmt;
> > > char buf[sizeof(struct erofs_inode_extended)];
> > > diff --git a/lib/super.c b/lib/super.c
> > > index 0fa69ab..0c30403 100644
> > > --- a/lib/super.c
> > > +++ b/lib/super.c
> > > @@ -62,6 +62,7 @@ int erofs_read_superblock(void)
> > > sbi.islotbits = EROFS_ISLOTBITS;
> > > sbi.root_nid = le16_to_cpu(dsb->root_nid);
> > > sbi.inos = le64_to_cpu(dsb->inos);
> > > + sbi.checksum = le32_to_cpu(dsb->checksum);
> > > sbi.build_time = le64_to_cpu(dsb->build_time);
> > > sbi.build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
> > > diff --git a/mkfs/main.c b/mkfs/main.c
> > > index 1c8dea5..b9b46f5 100644
> > > --- a/mkfs/main.c
> > > +++ b/mkfs/main.c
> > > @@ -424,19 +424,6 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
> > > return 0;
> > > }
> > > -#define CRC32C_POLY_LE 0x82F63B78
> > > -static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
> > > -{
> > > - int i;
> > > -
> > > - while (len--) {
> > > - crc ^= *in++;
> > > - for (i = 0; i < 8; i++)
> > > - crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
> > > - }
> > > - return crc;
> > > -}
> > > -
> > > static int erofs_mkfs_superblock_csum_set(void)
> > > {
> > > int ret;
> > > --
> > > 2.33.0.1079.g6e70778dc9-goog
> > >
> > .
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] erofs-utils: introduce fsck.erofs
2021-10-25 23:24 ` Gao Xiang
2021-10-26 2:42 ` Guo Xuenan
@ 2021-10-26 11:34 ` Guo Xuenan
2021-10-26 18:40 ` Daeho Jeong
2021-10-26 18:35 ` Daeho Jeong
2 siblings, 1 reply; 13+ messages in thread
From: Guo Xuenan @ 2021-10-26 11:34 UTC (permalink / raw)
To: Daeho Jeong, xiang; +Cc: Wang Qi, miaoxie, linux-erofs, Daeho Jeong
Hi Daeho,
在 2021/10/26 7:24, Gao Xiang 写道:
> (Sorry due to just mutt+vim edit by mistake. try to send properly
> again..)
>
> Hi Daeho,
>
> On Mon, Oct 25, 2021 at 12:48:09PM -0700, Daeho Jeong wrote:
>> From: Daeho Jeong <daehojeong@google.com>
>>
>> I made a fsck.erofs tool to check erofs filesystem image integrity
>> and calculate filesystem compression ratio.
>> Here are options to support now.
>>
>> fsck.erofs [options] IMAGE
>> -V print the version number of fsck.erofs and exit.
>> -d# set output message level to # (maximum 9)\n
>> -c print total compression ratio of all compressed files
>> -C print total compression ratio of all files
>>
>> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> Many thanks for the patch! I also think some fsck feature is useful in
> order to check the image integration.
>
> IMO, Compression ratio calculation is more like a dump fs feature.
> Wang Qi developed the erofsdump yet he told me that he is working for
> another urgent stuffs for now so he delayed the work:
> https://lore.kernel.org/r/20210915093537.2579575-1-guoxuenan@huawei.com
I agree with Xiang that -C/c options for collecting filesystem
statistics do repetitive
functional task comparing with dump tool. I think it's better to put it
in the dump tool.
And i have some further suggestions, In actual use scenairos of erofs on
mobile phone,
we found some accidental decompression failures. so, in my opinion, it
is better that
the fsck tool is able to unzip the overall data on disk or some specific
files.
Thanks.
> fsck and dump have the similar logic (and like f2fs-tools, we can also
> have one program for this 2 features). If fsck is actively developed,
> I'm pretty fine to use the fsck codebase and merge erofsdump into it
> later instead.
>
> Some comments as below:
>
>> ---
>> Makefile.am | 2 +-
>> configure.ac | 3 +-
>> fsck/Makefile.am | 9 +
>> fsck/main.c | 548 +++++++++++++++++++++++++++++++++++++++
>> include/erofs/internal.h | 5 +
>> include/erofs_fs.h | 13 +
>> lib/data.c | 4 +-
>> lib/namei.c | 2 +-
>> lib/super.c | 1 +
>> mkfs/main.c | 13 -
>> 10 files changed, 582 insertions(+), 18 deletions(-)
>> create mode 100644 fsck/Makefile.am
>> create mode 100644 fsck/main.c
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 24e1d38..fc464e8 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -2,7 +2,7 @@
>>
>> ACLOCAL_AMFLAGS = -I m4
>>
>> -SUBDIRS = man lib mkfs dump
>> +SUBDIRS = man lib mkfs dump fsck
>> if ENABLE_FUSE
>> SUBDIRS += fuse
>> endif
>> diff --git a/configure.ac b/configure.ac
>> index b2c3225..5698b2e 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -298,5 +298,6 @@ AC_CONFIG_FILES([Makefile
>> lib/Makefile
>> mkfs/Makefile
>> dump/Makefile
>> - fuse/Makefile])
>> + fuse/Makefile
>> + fsck/Makefile])
>> AC_OUTPUT
>> diff --git a/fsck/Makefile.am b/fsck/Makefile.am
>> new file mode 100644
>> index 0000000..82973ba
>> --- /dev/null
>> +++ b/fsck/Makefile.am
>> @@ -0,0 +1,9 @@
>> +# SPDX-License-Identifier: GPL-2.0+
>> +# Makefile.am
>> +
>> +AUTOMAKE_OPTIONS = foreign
>> +bin_PROGRAMS = fsck.erofs
>> +AM_CPPFLAGS = ${libuuid_CFLAGS}
>> +fsck_erofs_SOURCES = main.c
>> +fsck_erofs_CFLAGS = -Wall -Werror -I$(top_srcdir)/include
>> +fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libuuid_LIBS} ${liblz4_LIBS}
>> diff --git a/fsck/main.c b/fsck/main.c
>> new file mode 100644
>> index 0000000..c397d19
>> --- /dev/null
>> +++ b/fsck/main.c
>> @@ -0,0 +1,548 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright 2021 Google LLC
>> + * Author: Daeho Jeong <daehojeong@google.com>
>> + */
>> +#include <stdlib.h>
>> +#include <getopt.h>
>> +#include <time.h>
>> +#include "erofs/print.h"
>> +#include "erofs/io.h"
>> +
>> +enum {
>> + NO_PRINT_COMP_RATIO = 0,
>> + PRINT_COMP_RATIO_ALL = 1,
>> + PRINT_COMP_RATIO_COMP_FILE = 2,
>> +};
>> +
>> +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
>> +
>> +struct erofsfsck_cfg {
>> + bool corrupted;
>> + int print_comp_ratio;
>> + u64 ondisk_len;
>> + u64 logical_len;
>> +};
>> +static struct erofsfsck_cfg fsckcfg;
>> +
>> +static struct option long_options[] = {
>> + {"help", no_argument, 0, 1},
>> + {0, 0, 0, 0},
>> +};
>> +
>> +static void usage(void)
>> +{
>> + fputs("usage: [options] IMAGE\n\n"
>> + "Check erofs filesystem integrity of IMAGE, and [options] are:\n"
>> + " -V print the version number of fsck.erofs and exit.\n"
>> + " -d# set output message level to # (maximum 9)\n"
>> + " -c print total compression ratio of all compressed files\n"
>> + " -C print total compression ratio of all files\n"
> Apart from erofsdump consideration.. Can we merge these 2 options into one?
>
>> + " --help display this help and exit.\n",
>> + stderr);
>> +}
>> +
>> +static void erofsfsck_print_version(void)
>> +{
>> + fprintf(stderr, "fsck.erofs %s\n", cfg.c_version);
>> +}
>> +
>> +static int erofsfsck_parse_options_cfg(int argc, char **argv)
>> +{
>> + int opt, i;
>> +
>> + while ((opt = getopt_long(argc, argv, "Vd:Cc",
>> + long_options, NULL)) != -1) {
>> + switch (opt) {
>> + case 'V':
>> + erofsfsck_print_version();
>> + exit(0);
>> + case 'd':
>> + i = atoi(optarg);
>> + if (i < EROFS_MSG_MIN || i > EROFS_MSG_MAX) {
>> + erofs_err("invalid debug level %d", i);
>> + return -EINVAL;
>> + }
>> + cfg.c_dbg_lvl = i;
>> + break;
>> + case 'C':
>> + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_ALL;
>> + break;
>> + case 'c':
>> + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_COMP_FILE;
>> + break;
>> + case 1:
>> + usage();
>> + exit(0);
>> + default:
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + if (optind >= argc)
>> + return -EINVAL;
>> +
>> + cfg.c_img_path = strdup(argv[optind++]);
>> + if (!cfg.c_img_path)
>> + return -ENOMEM;
>> +
>> + if (optind < argc) {
>> + erofs_err("unexpected argument: %s\n", argv[optind]);
>> + return -EINVAL;
>> + }
>> + return 0;
>> +}
>> +
>> +static int erofs_check_sb_chksum(void)
>> +{
>> + int ret;
>> + u8 buf[EROFS_BLKSIZ];
>> + u32 crc;
>> + struct erofs_super_block *sb;
>> +
>> + ret = blk_read(buf, 0, 1);
>> + if (ret) {
>> + erofs_err("failed to read superblock to check checksum: "
>> + "errno(%d)", ret);
>> + return -1;
>> + }
>> +
>> + sb = (struct erofs_super_block *)(buf + EROFS_SUPER_OFFSET);
>> + sb->checksum = 0;
>> +
>> + crc = crc32c(~0, (u8 *)sb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
>> + if (crc != sbi.checksum) {
>> + erofs_err("superblock checksum doesn't match: saved(0x%08x) "
>> + "calculated(0x%08x)", sbi.checksum, crc);
>> + fsckcfg.corrupted = true;
>> + return -1;
>> + }
>> + return 0;
>> +}
>> +
>> +static int check_special_dentries(struct erofs_dirent *de, const char *de_name,
>> + unsigned int de_namelen, erofs_nid_t nid,
>> + erofs_nid_t pnid, bool is_curdir)
>> +{
>> + unsigned int dirname_len = is_curdir ? 1 : 2;
>> + const char *dirname = is_curdir ? "." : "..";
>> + erofs_nid_t correct_nid = is_curdir ? nid : pnid;
>> +
>> + if (de_namelen != dirname_len || memcmp(de_name, dirname, de_namelen)) {
>> + char *dbgname = strndup(de_name, de_namelen);
>> +
>> + BUG_ON(!dbgname);
>> + if (is_curdir)
>> + erofs_err("wrong current dir name(%s) @ nid(%llu)",
>> + dbgname, nid | 0ULL);
>> + else
>> + erofs_err("wrong parent dir name(%s) @ nid(%llu)",
>> + dbgname, nid | 0ULL);
>> + free(dbgname);
>> + return -1;
>> + }
>> +
>> + if (de->nid != correct_nid) {
>> + if (is_curdir)
>> + erofs_err("wrong current dir nid(%llu) @ nid(%llu)",
>> + de->nid | 0ULL, nid | 0ULL);
>> + else
>> + erofs_err("wrong parent dir nid(%llu): "
>> + "pnid(%llu) @ nid(%llu)",
>> + de->nid | 0ULL, pnid | 0ULL, nid | 0ULL);
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int traverse_dirents(erofs_nid_t pnid, erofs_nid_t nid,
>> + void *dentry_blk, erofs_off_t offset,
>> + unsigned int next_nameoff, unsigned int maxsize)
> Not quite sure if we could introduce some
> erofs_foreach_dirent() and customized callbacks to cleanup similar functions
> in fuse and fsck...
>
>> +{
>> + struct erofs_dirent *de = dentry_blk;
>> + const struct erofs_dirent *end = dentry_blk + next_nameoff;
>> + unsigned int idx = 0;
>> +
>> + erofs_dbg("traversing pnid(%llu), nid(%llu)", pnid | 0ULL, nid | 0ULL);
>> +
>> + if (offset == 0 && (next_nameoff < 2 * sizeof(struct erofs_dirent))) {
>> + erofs_err("too small dirents of size(%d) in nid(%llu)",
>> + next_nameoff, nid | 0ULL);
>> + return -EFSCORRUPTED;
>> + }
>> +
>> + while (de < end) {
>> + const char *de_name;
>> + unsigned int de_namelen;
>> + unsigned int nameoff;
>> + char *dbgname;
>> +
>> + nameoff = le16_to_cpu(de->nameoff);
>> + de_name = (char *)dentry_blk + nameoff;
>> +
>> + /* the last dirent check */
>> + if (de + 1 >= end)
>> + de_namelen = strnlen(de_name, maxsize - nameoff);
>> + else
>> + de_namelen = le16_to_cpu(de[1].nameoff) - nameoff;
>> +
>> + if (cfg.c_dbg_lvl >= EROFS_DBG) {
>> + dbgname = strndup(de_name, de_namelen);
>> + BUG_ON(!dbgname);
>> + erofs_dbg("traversed filename(%s)", dbgname);
>> + free(dbgname);
>> + }
>> +
>> + /* corrupted entry check */
>> + if (nameoff != next_nameoff) {
>> + erofs_err("bogus dirent with nameoff(%u): expected(%d) "
>> + "@ nid(%llu), offset(%llu), idx(%u)",
>> + nameoff, next_nameoff, nid | 0ULL,
>> + offset | 0ULL, idx);
>> + return -EFSCORRUPTED;
>> + }
>> +
>> + if (nameoff + de_namelen > maxsize ||
>> + de_namelen > EROFS_NAME_LEN) {
>> + erofs_err("bogus dirent with namelen(%u) @ nid(%llu), "
>> + "offset(%llu), idx(%u)",
>> + de_namelen, nid | 0ULL, offset | 0ULL, idx);
>> + return -EFSCORRUPTED;
>> + }
>> +
>> + if (offset == 0 && (idx == 0 || idx == 1)) {
> We may not assume "." and ".." special dirents as 0 or 1... Actually
> all dirents are sorted in the alphabetical order. I'm not sure if
> some dirents are smaller than "." and "..".
>
> We could also check if all dirents are in alphabetical order as well
> since it's used to do binary search runtimely.
>
>> + if (check_special_dentries(de, de_name, de_namelen, nid,
>> + pnid, idx == 0))
>> + return -EFSCORRUPTED;
>> + } else {
>> + erofs_check_inode(nid, de->nid);
>> + if (fsckcfg.corrupted)
>> + return -EFSCORRUPTED;
>> + }
>> +
>> + next_nameoff += de_namelen;
>> + ++de;
>> + ++idx;
>> + }
>> +
>> + erofs_dbg("traversing ... done nid(%llu)", nid | 0ULL);
>> + return 0;
>> +}
>> +
>> +static int verify_raw_data_chunk(struct erofs_inode *inode)
>> +{
>> + struct erofs_map_blocks map = {
>> + .index = UINT_MAX,
>> + };
>> + int ret;
>> + erofs_off_t ptr = 0;
>> +
>> + if (fsckcfg.print_comp_ratio == PRINT_COMP_RATIO_ALL) {
>> + fsckcfg.logical_len += inode->i_size;
>> + fsckcfg.ondisk_len += inode->i_size;
>> + }
>> +
>> + while (ptr < inode->i_size) {
>> + map.m_la = ptr;
>> + ret = erofs_map_blocks(inode, &map, 0);
>> + if (ret)
>> + return ret;
>> +
>> + if (map.m_plen != map.m_llen || ptr != map.m_la) {
>> + erofs_err("broken data chunk layout m_la %" PRIu64
>> + " ptr %" PRIu64 " m_llen %" PRIu64 " m_plen %"
>> + PRIu64, map.m_la, ptr, map.m_llen,
>> + map.m_plen);
>> + return -EFSCORRUPTED;
>> + }
>> +
>> + if (!(map.m_flags & EROFS_MAP_MAPPED) && !map.m_llen) {
>> + /* readched EOF */
>> + ptr = inode->i_size;
>> + continue;
>> + }
>> +
>> + ptr += map.m_llen;
>> + }
>> + return 0;
>> +}
>> +
>> +static int verify_compressed_chunk(struct erofs_inode *inode)
>> +{
>> + struct erofs_map_blocks map = {
>> + .index = UINT_MAX,
>> + };
>> + int ret = 0;
>> + bool count_pchunk = fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO;
>> + u64 pchunk_len = 0;
>> + erofs_off_t offset = 0, end = inode->i_size;
>> +
>> + while (end > offset) {
>> + map.m_la = end - 1;
>> +
>> + ret = z_erofs_map_blocks_iter(inode, &map);
>> + if (ret)
>> + return ret;
>> +
>> + if (end > map.m_la + map.m_llen) {
>> + erofs_err("broken compressed chunk layout m_la %" PRIu64
>> + " m_llen %" PRIu64 " end %" PRIu64,
>> + map.m_la, map.m_llen, end);
>> + return -EFSCORRUPTED;
>> + }
> IMO, A simple way to check this is to try to decompress by calling
> z_erofs_decompress() with .partial_decoding == 0 and see if any
> error when decompressing as well.
>
>> +
>> + if (count_pchunk)
>> + pchunk_len += map.m_plen;
>> +
>> + end = map.m_la;
>> + }
>> +
>> + if (count_pchunk) {
>> + fsckcfg.logical_len += inode->i_size;
>> + fsckcfg.ondisk_len += pchunk_len;
> we can also check another field called compressed_blocks in on-disk
> inode...
>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int erofs_verify_xattr(struct erofs_inode *inode)
>> +{
>> + unsigned int xattr_hdr_size = sizeof(struct erofs_xattr_ibody_header);
>> + unsigned int xattr_entry_size = sizeof(struct erofs_xattr_entry);
>> + erofs_off_t addr;
>> + unsigned int ofs, xattr_shared_count;
>> + struct erofs_xattr_ibody_header *ih;
>> + struct erofs_xattr_entry *entry;
>> + int i, remaining = inode->xattr_isize, ret = 0;
>> + char *buf = calloc(EROFS_BLKSIZ, 1);
>> +
>> + BUG_ON(!buf);
>> +
>> + if (inode->xattr_isize == xattr_hdr_size) {
>> + erofs_err("xattr_isize %d of nid %llu is not supported yet",
>> + inode->xattr_isize, inode->nid | 0ULL);
>> + ret = -EFSCORRUPTED;
>> + goto out;
>> + } else if (inode->xattr_isize < xattr_hdr_size) {
>> + if (inode->xattr_isize) {
>> + erofs_err("bogus xattr ibody @ nid %llu",
>> + inode->nid | 0ULL);
>> + ret = -EFSCORRUPTED;
>> + goto out;
>> + }
>> + }
>> +
>> + addr = iloc(inode->nid) + inode->inode_isize;
>> + ret = dev_read(buf, addr, xattr_hdr_size);
>> + if (ret < 0) {
>> + erofs_err("an error occurred when reading xattr header "
>> + "of nid(%llu): errno(%d)", inode->nid | 0ULL, ret);
>> + goto out;
>> + }
>> + ih = (struct erofs_xattr_ibody_header *)buf;
>> + xattr_shared_count = ih->h_shared_count;
>> +
>> + ofs = erofs_blkoff(addr) + xattr_hdr_size;
>> + addr += xattr_hdr_size;
>> + remaining -= xattr_hdr_size;
>> + for (i = 0; i < xattr_shared_count; ++i) {
>> + if (ofs >= EROFS_BLKSIZ) {
>> + if (ofs != EROFS_BLKSIZ) {
>> + erofs_err("unaligned xattr entry in "
>> + "xattr shared area of nid(%llu)",
>> + inode->nid | 0ULL);
>> + ret = -EFSCORRUPTED;
> IMO, it won't happen due to proper alignment.. but I'm fine as this too.
>
> Thanks,
> Gao Xiang
>
>> + goto out;
>> + }
>> + ofs = 0;
>> + }
>> + ofs += xattr_entry_size;
>> + addr += xattr_entry_size;
>> + remaining -= xattr_entry_size;
>> + }
>> +
>> + while (remaining > 0) {
>> + unsigned int entry_sz;
>> +
>> + ret = dev_read(buf, addr, xattr_entry_size);
>> + if (ret) {
>> + erofs_err("an error occurred when reading xattr entry "
>> + "of nid(%llu): errno(%d)",
>> + inode->nid | 0ULL, ret);
>> + goto out;
>> + }
>> +
>> + entry = (struct erofs_xattr_entry *)buf;
>> + entry_sz = erofs_xattr_entry_size(entry);
>> + if (remaining < entry_sz) {
>> + erofs_err("xattr on-disk corruption: xattr entry "
>> + "beyond xattr_isize of nid(%llu)",
>> + inode->nid | 0ULL);
>> + ret = -EFSCORRUPTED;
>> + goto out;
>> + }
>> + addr += entry_sz;
>> + remaining -= entry_sz;
>> + }
>> +out:
>> + free(buf);
>> + return ret;
>> +}
>> +
>> +static int erofs_verify_data_chunk(struct erofs_inode *inode)
>> +{
>> + int ret;
>> +
>> + erofs_dbg("verify data chunk of nid(%llu): type(%d)",
>> + inode->nid | 0ULL, inode->datalayout);
>> +
>> + switch (inode->datalayout) {
>> + case EROFS_INODE_FLAT_PLAIN:
>> + case EROFS_INODE_FLAT_INLINE:
>> + case EROFS_INODE_CHUNK_BASED:
>> + ret = verify_raw_data_chunk(inode);
>> + break;
>> + case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
>> + case EROFS_INODE_FLAT_COMPRESSION:
>> + ret = verify_compressed_chunk(inode);
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> +
>> + if (ret == -EIO)
>> + erofs_err("I/O error occurred when verifying "
>> + "data chunk of nid(%llu)", inode->nid | 0ULL);
>> +
>> + return ret;
>> +}
>> +
>> +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid)
>> +{
>> + int ret;
>> + struct erofs_inode *inode;
>> + char *buf;
>> + erofs_off_t offset;
>> +
>> + erofs_dbg("check inode: nid(%llu)", nid | 0ULL);
>> + inode = calloc(1, sizeof(struct erofs_inode));
>> + BUG_ON(!inode);
>> + buf = calloc(EROFS_BLKSIZ, 1);
>> + BUG_ON(!buf);
>> +
>> + inode->nid = nid;
>> + ret = erofs_read_inode_from_disk(inode);
>> + if (ret) {
>> + if (ret == -EIO)
>> + erofs_err("I/O error occurred when reading nid(%llu)",
>> + nid | 0ULL);
>> + goto out;
>> + }
>> +
>> + /* verify xattr field */
>> + ret = erofs_verify_xattr(inode);
>> + if (ret)
>> + goto out;
>> +
>> + /* verify data chunk layout */
>> + ret = erofs_verify_data_chunk(inode);
>> + if (ret)
>> + goto out;
>> +
>> + if ((inode->i_mode & S_IFMT) != S_IFDIR)
>> + goto out;
>> +
>> + offset = 0;
>> + while (offset < inode->i_size) {
>> + erofs_off_t maxsize = min_t(erofs_off_t,
>> + inode->i_size - offset, EROFS_BLKSIZ);
>> + struct erofs_dirent *de = (void *)buf;
>> + unsigned int nameoff;
>> +
>> + ret = erofs_pread(inode, buf, maxsize, offset);
>> + if (ret) {
>> + erofs_err("I/O error occurred when reading dirents: "
>> + "nid(%llu), offset(%llu), size(%llu)",
>> + nid | 0ULL, offset | 0ULL, maxsize | 0ULL);
>> + goto out;
>> + }
>> +
>> + nameoff = le16_to_cpu(de->nameoff);
>> + if (nameoff < sizeof(struct erofs_dirent) ||
>> + nameoff >= PAGE_SIZE) {
>> + erofs_err("invalid de[0].nameoff %u @ nid(%llu), "
>> + "offset(%llu)",
>> + nameoff, nid | 0ULL, offset | 0ULL);
>> + ret = -EFSCORRUPTED;
>> + goto out;
>> + }
>> +
>> + ret = traverse_dirents(pnid, nid, buf, offset,
>> + nameoff, maxsize);
>> + if (ret)
>> + goto out;
>> +
>> + offset += maxsize;
>> + }
>> +out:
>> + free(buf);
>> + free(inode);
>> + if (ret && ret != -EIO)
>> + fsckcfg.corrupted = true;
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> + int err;
>> +
>> + erofs_init_configure();
>> +
>> + fsckcfg.corrupted = false;
>> + fsckcfg.print_comp_ratio = NO_PRINT_COMP_RATIO;
>> + fsckcfg.logical_len = 0;
>> + fsckcfg.ondisk_len = 0;
>> +
>> + err = erofsfsck_parse_options_cfg(argc, argv);
>> + if (err) {
>> + if (err == -EINVAL)
>> + usage();
>> + goto exit;
>> + }
>> +
>> + err = dev_open_ro(cfg.c_img_path);
>> + if (err) {
>> + erofs_err("failed to open image file");
>> + goto exit;
>> + }
>> +
>> + err = erofs_read_superblock();
>> + if (err) {
>> + erofs_err("failed to read superblock");
>> + goto exit;
>> + }
>> +
>> + if (erofs_sb_has_sb_chksum() && erofs_check_sb_chksum()) {
>> + erofs_err("failed to verify superblock checksum");
>> + goto exit;
>> + }
>> +
>> + erofs_check_inode(sbi.root_nid, sbi.root_nid);
>> +
>> + if (fsckcfg.corrupted) {
>> + fprintf(stderr, "Found some filesystem corruption\n");
>> + } else {
>> + fprintf(stderr, "No error found\n");
>> + if (fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO) {
>> + double comp_ratio = (double)fsckcfg.ondisk_len * 100 /
>> + (double)fsckcfg.logical_len;
>> + fprintf(stderr, "Compression Ratio: %.2f(%%)\n",
>> + comp_ratio);
>> + }
>> + }
>> +
>> +exit:
>> + erofs_exit_configure();
>> + return err;
>> +}
>> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
>> index 8b154ed..80065b2 100644
>> --- a/include/erofs/internal.h
>> +++ b/include/erofs/internal.h
>> @@ -82,6 +82,8 @@ struct erofs_sb_info {
>>
>> u16 available_compr_algs;
>> u16 lz4_max_distance;
>> +
>> + u32 checksum;
>> };
>>
>> /* global sbi */
>> @@ -264,10 +266,13 @@ int erofs_read_superblock(void);
>>
>> /* namei.c */
>> int erofs_ilookup(const char *path, struct erofs_inode *vi);
>> +int erofs_read_inode_from_disk(struct erofs_inode *vi);
>>
>> /* data.c */
>> int erofs_pread(struct erofs_inode *inode, char *buf,
>> erofs_off_t count, erofs_off_t offset);
>> +int erofs_map_blocks(struct erofs_inode *inode,
>> + struct erofs_map_blocks *map, int flags);
>> /* zmap.c */
>> int z_erofs_fill_inode(struct erofs_inode *vi);
>> int z_erofs_map_blocks_iter(struct erofs_inode *vi,
>> diff --git a/include/erofs_fs.h b/include/erofs_fs.h
>> index 66a68e3..62e9981 100644
>> --- a/include/erofs_fs.h
>> +++ b/include/erofs_fs.h
>> @@ -400,4 +400,17 @@ static inline void erofs_check_ondisk_layout_definitions(void)
>> Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
>> }
>>
>> +#define CRC32C_POLY_LE 0x82F63B78
>> +static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
>> +{
>> + int i;
>> +
>> + while (len--) {
>> + crc ^= *in++;
>> + for (i = 0; i < 8; i++)
>> + crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
>> + }
>> + return crc;
>> +}
>> +
>> #endif
>> diff --git a/lib/data.c b/lib/data.c
>> index 641d840..6cb7eeb 100644
>> --- a/lib/data.c
>> +++ b/lib/data.c
>> @@ -61,8 +61,8 @@ err_out:
>> return err;
>> }
>>
>> -static int erofs_map_blocks(struct erofs_inode *inode,
>> - struct erofs_map_blocks *map, int flags)
>> +int erofs_map_blocks(struct erofs_inode *inode,
>> + struct erofs_map_blocks *map, int flags)
>> {
>> struct erofs_inode *vi = inode;
>> struct erofs_inode_chunk_index *idx;
>> diff --git a/lib/namei.c b/lib/namei.c
>> index b4bdabf..56f199a 100644
>> --- a/lib/namei.c
>> +++ b/lib/namei.c
>> @@ -22,7 +22,7 @@ static dev_t erofs_new_decode_dev(u32 dev)
>> return makedev(major, minor);
>> }
>>
>> -static int erofs_read_inode_from_disk(struct erofs_inode *vi)
>> +int erofs_read_inode_from_disk(struct erofs_inode *vi)
>> {
>> int ret, ifmt;
>> char buf[sizeof(struct erofs_inode_extended)];
>> diff --git a/lib/super.c b/lib/super.c
>> index 0fa69ab..0c30403 100644
>> --- a/lib/super.c
>> +++ b/lib/super.c
>> @@ -62,6 +62,7 @@ int erofs_read_superblock(void)
>> sbi.islotbits = EROFS_ISLOTBITS;
>> sbi.root_nid = le16_to_cpu(dsb->root_nid);
>> sbi.inos = le64_to_cpu(dsb->inos);
>> + sbi.checksum = le32_to_cpu(dsb->checksum);
>>
>> sbi.build_time = le64_to_cpu(dsb->build_time);
>> sbi.build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
>> diff --git a/mkfs/main.c b/mkfs/main.c
>> index 1c8dea5..b9b46f5 100644
>> --- a/mkfs/main.c
>> +++ b/mkfs/main.c
>> @@ -424,19 +424,6 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
>> return 0;
>> }
>>
>> -#define CRC32C_POLY_LE 0x82F63B78
>> -static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
>> -{
>> - int i;
>> -
>> - while (len--) {
>> - crc ^= *in++;
>> - for (i = 0; i < 8; i++)
>> - crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
>> - }
>> - return crc;
>> -}
>> -
>> static int erofs_mkfs_superblock_csum_set(void)
>> {
>> int ret;
>> --
>> 2.33.0.1079.g6e70778dc9-goog
>>
> .
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] erofs-utils: introduce fsck.erofs
2021-10-25 23:24 ` Gao Xiang
2021-10-26 2:42 ` Guo Xuenan
2021-10-26 11:34 ` Guo Xuenan
@ 2021-10-26 18:35 ` Daeho Jeong
2021-10-27 3:13 ` Gao Xiang
2 siblings, 1 reply; 13+ messages in thread
From: Daeho Jeong @ 2021-10-26 18:35 UTC (permalink / raw)
To: Daeho Jeong, linux-erofs, miaoxie, fangwei1, xiang, Daeho Jeong,
GuoXuenan, Wang Qi
Cc: Jaegeuk Kim
> Many thanks for the patch! I also think some fsck feature is useful in
> order to check the image integration.
>
> IMO, Compression ratio calculation is more like a dump fs feature.
> Wang Qi developed the erofsdump yet he told me that he is working for
> another urgent stuffs for now so he delayed the work:
> https://lore.kernel.org/r/20210915093537.2579575-1-guoxuenan@huawei.com
Right, it's better to move these feature to dump.erofs.
>
> fsck and dump have the similar logic (and like f2fs-tools, we can also
> have one program for this 2 features). If fsck is actively developed,
> I'm pretty fine to use the fsck codebase and merge erofsdump into it
> later instead.
Like f2fs tools, I think we can maintain two tools separately.
But, if you want, we can merge them into one tool.
> Some comments as below:
>
> > ---
> > Makefile.am | 2 +-
> > configure.ac | 3 +-
> > fsck/Makefile.am | 9 +
> > fsck/main.c | 548 +++++++++++++++++++++++++++++++++++++++
> > include/erofs/internal.h | 5 +
> > include/erofs_fs.h | 13 +
> > lib/data.c | 4 +-
> > lib/namei.c | 2 +-
> > lib/super.c | 1 +
> > mkfs/main.c | 13 -
> > 10 files changed, 582 insertions(+), 18 deletions(-)
> > create mode 100644 fsck/Makefile.am
> > create mode 100644 fsck/main.c
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 24e1d38..fc464e8 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -2,7 +2,7 @@
> >
> > ACLOCAL_AMFLAGS = -I m4
> >
> > -SUBDIRS = man lib mkfs dump
> > +SUBDIRS = man lib mkfs dump fsck
> > if ENABLE_FUSE
> > SUBDIRS += fuse
> > endif
> > diff --git a/configure.ac b/configure.ac
> > index b2c3225..5698b2e 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -298,5 +298,6 @@ AC_CONFIG_FILES([Makefile
> > lib/Makefile
> > mkfs/Makefile
> > dump/Makefile
> > - fuse/Makefile])
> > + fuse/Makefile
> > + fsck/Makefile])
> > AC_OUTPUT
> > diff --git a/fsck/Makefile.am b/fsck/Makefile.am
> > new file mode 100644
> > index 0000000..82973ba
> > --- /dev/null
> > +++ b/fsck/Makefile.am
> > @@ -0,0 +1,9 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +# Makefile.am
> > +
> > +AUTOMAKE_OPTIONS = foreign
> > +bin_PROGRAMS = fsck.erofs
> > +AM_CPPFLAGS = ${libuuid_CFLAGS}
> > +fsck_erofs_SOURCES = main.c
> > +fsck_erofs_CFLAGS = -Wall -Werror -I$(top_srcdir)/include
> > +fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libuuid_LIBS} ${liblz4_LIBS}
> > diff --git a/fsck/main.c b/fsck/main.c
> > new file mode 100644
> > index 0000000..c397d19
> > --- /dev/null
> > +++ b/fsck/main.c
> > @@ -0,0 +1,548 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2021 Google LLC
> > + * Author: Daeho Jeong <daehojeong@google.com>
> > + */
> > +#include <stdlib.h>
> > +#include <getopt.h>
> > +#include <time.h>
> > +#include "erofs/print.h"
> > +#include "erofs/io.h"
> > +
> > +enum {
> > + NO_PRINT_COMP_RATIO = 0,
> > + PRINT_COMP_RATIO_ALL = 1,
> > + PRINT_COMP_RATIO_COMP_FILE = 2,
> > +};
> > +
> > +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
> > +
> > +struct erofsfsck_cfg {
> > + bool corrupted;
> > + int print_comp_ratio;
> > + u64 ondisk_len;
> > + u64 logical_len;
> > +};
> > +static struct erofsfsck_cfg fsckcfg;
> > +
> > +static struct option long_options[] = {
> > + {"help", no_argument, 0, 1},
> > + {0, 0, 0, 0},
> > +};
> > +
> > +static void usage(void)
> > +{
> > + fputs("usage: [options] IMAGE\n\n"
> > + "Check erofs filesystem integrity of IMAGE, and [options] are:\n"
> > + " -V print the version number of fsck.erofs and exit.\n"
> > + " -d# set output message level to # (maximum 9)\n"
> > + " -c print total compression ratio of all compressed files\n"
> > + " -C print total compression ratio of all files\n"
>
> Apart from erofsdump consideration.. Can we merge these 2 options into one?
Sure~
>
> > + " --help display this help and exit.\n",
> > + stderr);
> > +}
> > +
> > +static void erofsfsck_print_version(void)
> > +{
> > + fprintf(stderr, "fsck.erofs %s\n", cfg.c_version);
> > +}
> > +
> > +static int erofsfsck_parse_options_cfg(int argc, char **argv)
> > +{
> > + int opt, i;
> > +
> > + while ((opt = getopt_long(argc, argv, "Vd:Cc",
> > + long_options, NULL)) != -1) {
> > + switch (opt) {
> > + case 'V':
> > + erofsfsck_print_version();
> > + exit(0);
> > + case 'd':
> > + i = atoi(optarg);
> > + if (i < EROFS_MSG_MIN || i > EROFS_MSG_MAX) {
> > + erofs_err("invalid debug level %d", i);
> > + return -EINVAL;
> > + }
> > + cfg.c_dbg_lvl = i;
> > + break;
> > + case 'C':
> > + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_ALL;
> > + break;
> > + case 'c':
> > + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_COMP_FILE;
> > + break;
> > + case 1:
> > + usage();
> > + exit(0);
> > + default:
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + if (optind >= argc)
> > + return -EINVAL;
> > +
> > + cfg.c_img_path = strdup(argv[optind++]);
> > + if (!cfg.c_img_path)
> > + return -ENOMEM;
> > +
> > + if (optind < argc) {
> > + erofs_err("unexpected argument: %s\n", argv[optind]);
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
> > +
> > +static int erofs_check_sb_chksum(void)
> > +{
> > + int ret;
> > + u8 buf[EROFS_BLKSIZ];
> > + u32 crc;
> > + struct erofs_super_block *sb;
> > +
> > + ret = blk_read(buf, 0, 1);
> > + if (ret) {
> > + erofs_err("failed to read superblock to check checksum: "
> > + "errno(%d)", ret);
> > + return -1;
> > + }
> > +
> > + sb = (struct erofs_super_block *)(buf + EROFS_SUPER_OFFSET);
> > + sb->checksum = 0;
> > +
> > + crc = crc32c(~0, (u8 *)sb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
> > + if (crc != sbi.checksum) {
> > + erofs_err("superblock checksum doesn't match: saved(0x%08x) "
> > + "calculated(0x%08x)", sbi.checksum, crc);
> > + fsckcfg.corrupted = true;
> > + return -1;
> > + }
> > + return 0;
> > +}
> > +
> > +static int check_special_dentries(struct erofs_dirent *de, const char *de_name,
> > + unsigned int de_namelen, erofs_nid_t nid,
> > + erofs_nid_t pnid, bool is_curdir)
> > +{
> > + unsigned int dirname_len = is_curdir ? 1 : 2;
> > + const char *dirname = is_curdir ? "." : "..";
> > + erofs_nid_t correct_nid = is_curdir ? nid : pnid;
> > +
> > + if (de_namelen != dirname_len || memcmp(de_name, dirname, de_namelen)) {
> > + char *dbgname = strndup(de_name, de_namelen);
> > +
> > + BUG_ON(!dbgname);
> > + if (is_curdir)
> > + erofs_err("wrong current dir name(%s) @ nid(%llu)",
> > + dbgname, nid | 0ULL);
> > + else
> > + erofs_err("wrong parent dir name(%s) @ nid(%llu)",
> > + dbgname, nid | 0ULL);
> > + free(dbgname);
> > + return -1;
> > + }
> > +
> > + if (de->nid != correct_nid) {
> > + if (is_curdir)
> > + erofs_err("wrong current dir nid(%llu) @ nid(%llu)",
> > + de->nid | 0ULL, nid | 0ULL);
> > + else
> > + erofs_err("wrong parent dir nid(%llu): "
> > + "pnid(%llu) @ nid(%llu)",
> > + de->nid | 0ULL, pnid | 0ULL, nid | 0ULL);
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int traverse_dirents(erofs_nid_t pnid, erofs_nid_t nid,
> > + void *dentry_blk, erofs_off_t offset,
> > + unsigned int next_nameoff, unsigned int maxsize)
>
> Not quite sure if we could introduce some
> erofs_foreach_dirent() and customized callbacks to cleanup similar functions
> in fuse and fsck...
Let me think about it more.
> > +{
> > + struct erofs_dirent *de = dentry_blk;
> > + const struct erofs_dirent *end = dentry_blk + next_nameoff;
> > + unsigned int idx = 0;
> > +
> > + erofs_dbg("traversing pnid(%llu), nid(%llu)", pnid | 0ULL, nid | 0ULL);
> > +
> > + if (offset == 0 && (next_nameoff < 2 * sizeof(struct erofs_dirent))) {
> > + erofs_err("too small dirents of size(%d) in nid(%llu)",
> > + next_nameoff, nid | 0ULL);
> > + return -EFSCORRUPTED;
> > + }
> > +
> > + while (de < end) {
> > + const char *de_name;
> > + unsigned int de_namelen;
> > + unsigned int nameoff;
> > + char *dbgname;
> > +
> > + nameoff = le16_to_cpu(de->nameoff);
> > + de_name = (char *)dentry_blk + nameoff;
> > +
> > + /* the last dirent check */
> > + if (de + 1 >= end)
> > + de_namelen = strnlen(de_name, maxsize - nameoff);
> > + else
> > + de_namelen = le16_to_cpu(de[1].nameoff) - nameoff;
> > +
> > + if (cfg.c_dbg_lvl >= EROFS_DBG) {
> > + dbgname = strndup(de_name, de_namelen);
> > + BUG_ON(!dbgname);
> > + erofs_dbg("traversed filename(%s)", dbgname);
> > + free(dbgname);
> > + }
> > +
> > + /* corrupted entry check */
> > + if (nameoff != next_nameoff) {
> > + erofs_err("bogus dirent with nameoff(%u): expected(%d) "
> > + "@ nid(%llu), offset(%llu), idx(%u)",
> > + nameoff, next_nameoff, nid | 0ULL,
> > + offset | 0ULL, idx);
> > + return -EFSCORRUPTED;
> > + }
> > +
> > + if (nameoff + de_namelen > maxsize ||
> > + de_namelen > EROFS_NAME_LEN) {
> > + erofs_err("bogus dirent with namelen(%u) @ nid(%llu), "
> > + "offset(%llu), idx(%u)",
> > + de_namelen, nid | 0ULL, offset | 0ULL, idx);
> > + return -EFSCORRUPTED;
> > + }
> > +
> > + if (offset == 0 && (idx == 0 || idx == 1)) {
>
> We may not assume "." and ".." special dirents as 0 or 1... Actually
> all dirents are sorted in the alphabetical order. I'm not sure if
> some dirents are smaller than "." and "..".
Ok, I will remove this check here.
> We could also check if all dirents are in alphabetical order as well
> since it's used to do binary search runtimely.
I will add a check routine to check all dirents are in alphabetical order.
>
> > + if (check_special_dentries(de, de_name, de_namelen, nid,
> > + pnid, idx == 0))
> > + return -EFSCORRUPTED;
> > + } else {
> > + erofs_check_inode(nid, de->nid);
> > + if (fsckcfg.corrupted)
> > + return -EFSCORRUPTED;
> > + }
> > +
> > + next_nameoff += de_namelen;
> > + ++de;
> > + ++idx;
> > + }
> > +
> > + erofs_dbg("traversing ... done nid(%llu)", nid | 0ULL);
> > + return 0;
> > +}
> > +
> > +static int verify_raw_data_chunk(struct erofs_inode *inode)
> > +{
> > + struct erofs_map_blocks map = {
> > + .index = UINT_MAX,
> > + };
> > + int ret;
> > + erofs_off_t ptr = 0;
> > +
> > + if (fsckcfg.print_comp_ratio == PRINT_COMP_RATIO_ALL) {
> > + fsckcfg.logical_len += inode->i_size;
> > + fsckcfg.ondisk_len += inode->i_size;
> > + }
> > +
> > + while (ptr < inode->i_size) {
> > + map.m_la = ptr;
> > + ret = erofs_map_blocks(inode, &map, 0);
> > + if (ret)
> > + return ret;
> > +
> > + if (map.m_plen != map.m_llen || ptr != map.m_la) {
> > + erofs_err("broken data chunk layout m_la %" PRIu64
> > + " ptr %" PRIu64 " m_llen %" PRIu64 " m_plen %"
> > + PRIu64, map.m_la, ptr, map.m_llen,
> > + map.m_plen);
> > + return -EFSCORRUPTED;
> > + }
> > +
> > + if (!(map.m_flags & EROFS_MAP_MAPPED) && !map.m_llen) {
> > + /* readched EOF */
> > + ptr = inode->i_size;
> > + continue;
> > + }
> > +
> > + ptr += map.m_llen;
> > + }
> > + return 0;
> > +}
> > +
> > +static int verify_compressed_chunk(struct erofs_inode *inode)
> > +{
> > + struct erofs_map_blocks map = {
> > + .index = UINT_MAX,
> > + };
> > + int ret = 0;
> > + bool count_pchunk = fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO;
> > + u64 pchunk_len = 0;
> > + erofs_off_t offset = 0, end = inode->i_size;
> > +
> > + while (end > offset) {
> > + map.m_la = end - 1;
> > +
> > + ret = z_erofs_map_blocks_iter(inode, &map);
> > + if (ret)
> > + return ret;
> > +
> > + if (end > map.m_la + map.m_llen) {
> > + erofs_err("broken compressed chunk layout m_la %" PRIu64
> > + " m_llen %" PRIu64 " end %" PRIu64,
> > + map.m_la, map.m_llen, end);
> > + return -EFSCORRUPTED;
> > + }
>
> IMO, A simple way to check this is to try to decompress by calling
> z_erofs_decompress() with .partial_decoding == 0 and see if any
> error when decompressing as well.
Got it~
> > +
> > + if (count_pchunk)
> > + pchunk_len += map.m_plen;
> > +
> > + end = map.m_la;
> > + }
> > +
> > + if (count_pchunk) {
> > + fsckcfg.logical_len += inode->i_size;
> > + fsckcfg.ondisk_len += pchunk_len;
>
> we can also check another field called compressed_blocks in on-disk
> inode...
ditto
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int erofs_verify_xattr(struct erofs_inode *inode)
> > +{
> > + unsigned int xattr_hdr_size = sizeof(struct erofs_xattr_ibody_header);
> > + unsigned int xattr_entry_size = sizeof(struct erofs_xattr_entry);
> > + erofs_off_t addr;
> > + unsigned int ofs, xattr_shared_count;
> > + struct erofs_xattr_ibody_header *ih;
> > + struct erofs_xattr_entry *entry;
> > + int i, remaining = inode->xattr_isize, ret = 0;
> > + char *buf = calloc(EROFS_BLKSIZ, 1);
> > +
> > + BUG_ON(!buf);
> > +
> > + if (inode->xattr_isize == xattr_hdr_size) {
> > + erofs_err("xattr_isize %d of nid %llu is not supported yet",
> > + inode->xattr_isize, inode->nid | 0ULL);
> > + ret = -EFSCORRUPTED;
> > + goto out;
> > + } else if (inode->xattr_isize < xattr_hdr_size) {
> > + if (inode->xattr_isize) {
> > + erofs_err("bogus xattr ibody @ nid %llu",
> > + inode->nid | 0ULL);
> > + ret = -EFSCORRUPTED;
> > + goto out;
> > + }
> > + }
> > +
> > + addr = iloc(inode->nid) + inode->inode_isize;
> > + ret = dev_read(buf, addr, xattr_hdr_size);
> > + if (ret < 0) {
> > + erofs_err("an error occurred when reading xattr header "
> > + "of nid(%llu): errno(%d)", inode->nid | 0ULL, ret);
> > + goto out;
> > + }
> > + ih = (struct erofs_xattr_ibody_header *)buf;
> > + xattr_shared_count = ih->h_shared_count;
> > +
> > + ofs = erofs_blkoff(addr) + xattr_hdr_size;
> > + addr += xattr_hdr_size;
> > + remaining -= xattr_hdr_size;
> > + for (i = 0; i < xattr_shared_count; ++i) {
> > + if (ofs >= EROFS_BLKSIZ) {
> > + if (ofs != EROFS_BLKSIZ) {
> > + erofs_err("unaligned xattr entry in "
> > + "xattr shared area of nid(%llu)",
> > + inode->nid | 0ULL);
> > + ret = -EFSCORRUPTED;
>
> IMO, it won't happen due to proper alignment.. but I'm fine as this too.
>
> Thanks,
> Gao Xiang
>
> > + goto out;
> > + }
> > + ofs = 0;
> > + }
> > + ofs += xattr_entry_size;
> > + addr += xattr_entry_size;
> > + remaining -= xattr_entry_size;
> > + }
> > +
> > + while (remaining > 0) {
> > + unsigned int entry_sz;
> > +
> > + ret = dev_read(buf, addr, xattr_entry_size);
> > + if (ret) {
> > + erofs_err("an error occurred when reading xattr entry "
> > + "of nid(%llu): errno(%d)",
> > + inode->nid | 0ULL, ret);
> > + goto out;
> > + }
> > +
> > + entry = (struct erofs_xattr_entry *)buf;
> > + entry_sz = erofs_xattr_entry_size(entry);
> > + if (remaining < entry_sz) {
> > + erofs_err("xattr on-disk corruption: xattr entry "
> > + "beyond xattr_isize of nid(%llu)",
> > + inode->nid | 0ULL);
> > + ret = -EFSCORRUPTED;
> > + goto out;
> > + }
> > + addr += entry_sz;
> > + remaining -= entry_sz;
> > + }
> > +out:
> > + free(buf);
> > + return ret;
> > +}
> > +
> > +static int erofs_verify_data_chunk(struct erofs_inode *inode)
> > +{
> > + int ret;
> > +
> > + erofs_dbg("verify data chunk of nid(%llu): type(%d)",
> > + inode->nid | 0ULL, inode->datalayout);
> > +
> > + switch (inode->datalayout) {
> > + case EROFS_INODE_FLAT_PLAIN:
> > + case EROFS_INODE_FLAT_INLINE:
> > + case EROFS_INODE_CHUNK_BASED:
> > + ret = verify_raw_data_chunk(inode);
> > + break;
> > + case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
> > + case EROFS_INODE_FLAT_COMPRESSION:
> > + ret = verify_compressed_chunk(inode);
> > + break;
> > + default:
> > + ret = -EINVAL;
> > + break;
> > + }
> > +
> > + if (ret == -EIO)
> > + erofs_err("I/O error occurred when verifying "
> > + "data chunk of nid(%llu)", inode->nid | 0ULL);
> > +
> > + return ret;
> > +}
> > +
> > +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid)
> > +{
> > + int ret;
> > + struct erofs_inode *inode;
> > + char *buf;
> > + erofs_off_t offset;
> > +
> > + erofs_dbg("check inode: nid(%llu)", nid | 0ULL);
> > + inode = calloc(1, sizeof(struct erofs_inode));
> > + BUG_ON(!inode);
> > + buf = calloc(EROFS_BLKSIZ, 1);
> > + BUG_ON(!buf);
> > +
> > + inode->nid = nid;
> > + ret = erofs_read_inode_from_disk(inode);
> > + if (ret) {
> > + if (ret == -EIO)
> > + erofs_err("I/O error occurred when reading nid(%llu)",
> > + nid | 0ULL);
> > + goto out;
> > + }
> > +
> > + /* verify xattr field */
> > + ret = erofs_verify_xattr(inode);
> > + if (ret)
> > + goto out;
> > +
> > + /* verify data chunk layout */
> > + ret = erofs_verify_data_chunk(inode);
> > + if (ret)
> > + goto out;
> > +
> > + if ((inode->i_mode & S_IFMT) != S_IFDIR)
> > + goto out;
> > +
> > + offset = 0;
> > + while (offset < inode->i_size) {
> > + erofs_off_t maxsize = min_t(erofs_off_t,
> > + inode->i_size - offset, EROFS_BLKSIZ);
> > + struct erofs_dirent *de = (void *)buf;
> > + unsigned int nameoff;
> > +
> > + ret = erofs_pread(inode, buf, maxsize, offset);
> > + if (ret) {
> > + erofs_err("I/O error occurred when reading dirents: "
> > + "nid(%llu), offset(%llu), size(%llu)",
> > + nid | 0ULL, offset | 0ULL, maxsize | 0ULL);
> > + goto out;
> > + }
> > +
> > + nameoff = le16_to_cpu(de->nameoff);
> > + if (nameoff < sizeof(struct erofs_dirent) ||
> > + nameoff >= PAGE_SIZE) {
> > + erofs_err("invalid de[0].nameoff %u @ nid(%llu), "
> > + "offset(%llu)",
> > + nameoff, nid | 0ULL, offset | 0ULL);
> > + ret = -EFSCORRUPTED;
> > + goto out;
> > + }
> > +
> > + ret = traverse_dirents(pnid, nid, buf, offset,
> > + nameoff, maxsize);
> > + if (ret)
> > + goto out;
> > +
> > + offset += maxsize;
> > + }
> > +out:
> > + free(buf);
> > + free(inode);
> > + if (ret && ret != -EIO)
> > + fsckcfg.corrupted = true;
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > + int err;
> > +
> > + erofs_init_configure();
> > +
> > + fsckcfg.corrupted = false;
> > + fsckcfg.print_comp_ratio = NO_PRINT_COMP_RATIO;
> > + fsckcfg.logical_len = 0;
> > + fsckcfg.ondisk_len = 0;
> > +
> > + err = erofsfsck_parse_options_cfg(argc, argv);
> > + if (err) {
> > + if (err == -EINVAL)
> > + usage();
> > + goto exit;
> > + }
> > +
> > + err = dev_open_ro(cfg.c_img_path);
> > + if (err) {
> > + erofs_err("failed to open image file");
> > + goto exit;
> > + }
> > +
> > + err = erofs_read_superblock();
> > + if (err) {
> > + erofs_err("failed to read superblock");
> > + goto exit;
> > + }
> > +
> > + if (erofs_sb_has_sb_chksum() && erofs_check_sb_chksum()) {
> > + erofs_err("failed to verify superblock checksum");
> > + goto exit;
> > + }
> > +
> > + erofs_check_inode(sbi.root_nid, sbi.root_nid);
> > +
> > + if (fsckcfg.corrupted) {
> > + fprintf(stderr, "Found some filesystem corruption\n");
> > + } else {
> > + fprintf(stderr, "No error found\n");
> > + if (fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO) {
> > + double comp_ratio = (double)fsckcfg.ondisk_len * 100 /
> > + (double)fsckcfg.logical_len;
> > + fprintf(stderr, "Compression Ratio: %.2f(%%)\n",
> > + comp_ratio);
> > + }
> > + }
> > +
> > +exit:
> > + erofs_exit_configure();
> > + return err;
> > +}
> > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > index 8b154ed..80065b2 100644
> > --- a/include/erofs/internal.h
> > +++ b/include/erofs/internal.h
> > @@ -82,6 +82,8 @@ struct erofs_sb_info {
> >
> > u16 available_compr_algs;
> > u16 lz4_max_distance;
> > +
> > + u32 checksum;
> > };
> >
> > /* global sbi */
> > @@ -264,10 +266,13 @@ int erofs_read_superblock(void);
> >
> > /* namei.c */
> > int erofs_ilookup(const char *path, struct erofs_inode *vi);
> > +int erofs_read_inode_from_disk(struct erofs_inode *vi);
> >
> > /* data.c */
> > int erofs_pread(struct erofs_inode *inode, char *buf,
> > erofs_off_t count, erofs_off_t offset);
> > +int erofs_map_blocks(struct erofs_inode *inode,
> > + struct erofs_map_blocks *map, int flags);
> > /* zmap.c */
> > int z_erofs_fill_inode(struct erofs_inode *vi);
> > int z_erofs_map_blocks_iter(struct erofs_inode *vi,
> > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > index 66a68e3..62e9981 100644
> > --- a/include/erofs_fs.h
> > +++ b/include/erofs_fs.h
> > @@ -400,4 +400,17 @@ static inline void erofs_check_ondisk_layout_definitions(void)
> > Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
> > }
> >
> > +#define CRC32C_POLY_LE 0x82F63B78
> > +static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
> > +{
> > + int i;
> > +
> > + while (len--) {
> > + crc ^= *in++;
> > + for (i = 0; i < 8; i++)
> > + crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
> > + }
> > + return crc;
> > +}
> > +
> > #endif
> > diff --git a/lib/data.c b/lib/data.c
> > index 641d840..6cb7eeb 100644
> > --- a/lib/data.c
> > +++ b/lib/data.c
> > @@ -61,8 +61,8 @@ err_out:
> > return err;
> > }
> >
> > -static int erofs_map_blocks(struct erofs_inode *inode,
> > - struct erofs_map_blocks *map, int flags)
> > +int erofs_map_blocks(struct erofs_inode *inode,
> > + struct erofs_map_blocks *map, int flags)
> > {
> > struct erofs_inode *vi = inode;
> > struct erofs_inode_chunk_index *idx;
> > diff --git a/lib/namei.c b/lib/namei.c
> > index b4bdabf..56f199a 100644
> > --- a/lib/namei.c
> > +++ b/lib/namei.c
> > @@ -22,7 +22,7 @@ static dev_t erofs_new_decode_dev(u32 dev)
> > return makedev(major, minor);
> > }
> >
> > -static int erofs_read_inode_from_disk(struct erofs_inode *vi)
> > +int erofs_read_inode_from_disk(struct erofs_inode *vi)
> > {
> > int ret, ifmt;
> > char buf[sizeof(struct erofs_inode_extended)];
> > diff --git a/lib/super.c b/lib/super.c
> > index 0fa69ab..0c30403 100644
> > --- a/lib/super.c
> > +++ b/lib/super.c
> > @@ -62,6 +62,7 @@ int erofs_read_superblock(void)
> > sbi.islotbits = EROFS_ISLOTBITS;
> > sbi.root_nid = le16_to_cpu(dsb->root_nid);
> > sbi.inos = le64_to_cpu(dsb->inos);
> > + sbi.checksum = le32_to_cpu(dsb->checksum);
> >
> > sbi.build_time = le64_to_cpu(dsb->build_time);
> > sbi.build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
> > diff --git a/mkfs/main.c b/mkfs/main.c
> > index 1c8dea5..b9b46f5 100644
> > --- a/mkfs/main.c
> > +++ b/mkfs/main.c
> > @@ -424,19 +424,6 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
> > return 0;
> > }
> >
> > -#define CRC32C_POLY_LE 0x82F63B78
> > -static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
> > -{
> > - int i;
> > -
> > - while (len--) {
> > - crc ^= *in++;
> > - for (i = 0; i < 8; i++)
> > - crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
> > - }
> > - return crc;
> > -}
> > -
> > static int erofs_mkfs_superblock_csum_set(void)
> > {
> > int ret;
> > --
> > 2.33.0.1079.g6e70778dc9-goog
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] erofs-utils: introduce fsck.erofs
2021-10-26 11:34 ` Guo Xuenan
@ 2021-10-26 18:40 ` Daeho Jeong
0 siblings, 0 replies; 13+ messages in thread
From: Daeho Jeong @ 2021-10-26 18:40 UTC (permalink / raw)
To: Guo Xuenan; +Cc: Daeho Jeong, linux-erofs, Wang Qi, Jaegeuk Kim, miaoxie
On Tue, Oct 26, 2021 at 4:34 AM Guo Xuenan <guoxuenan@huawei.com> wrote:
>
> Hi Daeho,
>
> I agree with Xiang that -C/c options for collecting filesystem
> statistics do repetitive
>
> functional task comparing with dump tool. I think it's better to put it
> in the dump tool.
>
> And i have some further suggestions, In actual use scenairos of erofs on
> mobile phone,
>
> we found some accidental decompression failures. so, in my opinion, it
> is better that
>
> the fsck tool is able to unzip the overall data on disk or some specific
> files.
>
> Thanks.
Hi Guo,
I will move the compression rate calculation feature into the dump
tool as one option after you've done your work.
Plus, I think we can add the unzip feature into dump tool. It's more
like a dump feature. :)
Thanks,
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] erofs-utils: introduce fsck.erofs
2021-10-26 18:35 ` Daeho Jeong
@ 2021-10-27 3:13 ` Gao Xiang
2021-10-28 0:50 ` Daeho Jeong
0 siblings, 1 reply; 13+ messages in thread
From: Gao Xiang @ 2021-10-27 3:13 UTC (permalink / raw)
To: Daeho Jeong; +Cc: Daeho Jeong, linux-erofs, Jaegeuk Kim, miaoxie, Wang Qi
Hi Daeho,
On Tue, Oct 26, 2021 at 11:35:44AM -0700, Daeho Jeong wrote:
> > Many thanks for the patch! I also think some fsck feature is useful in
> > order to check the image integration.
> >
> > IMO, Compression ratio calculation is more like a dump fs feature.
> > Wang Qi developed the erofsdump yet he told me that he is working for
> > another urgent stuffs for now so he delayed the work:
> > https://lore.kernel.org/r/20210915093537.2579575-1-guoxuenan@huawei.com
>
> Right, it's better to move these feature to dump.erofs.
I'm fine to add "Compression ratio calculation" temporarily in
fsck.erofs as long as active development.
Let's see if fsck feature can match erofs-utils 1.4 timepoint.
>
> >
> > fsck and dump have the similar logic (and like f2fs-tools, we can also
> > have one program for this 2 features). If fsck is actively developed,
> > I'm pretty fine to use the fsck codebase and merge erofsdump into it
> > later instead.
>
> Like f2fs tools, I think we can maintain two tools separately.
Yeah, my point is two tools share one common logic in the final shape,
but I'm fine with the current status as well :)
> But, if you want, we can merge them into one tool.
>
> > Some comments as below:
> >
> > > ---
> > > Makefile.am | 2 +-
> > > configure.ac | 3 +-
> > > fsck/Makefile.am | 9 +
> > > fsck/main.c | 548 +++++++++++++++++++++++++++++++++++++++
> > > include/erofs/internal.h | 5 +
> > > include/erofs_fs.h | 13 +
> > > lib/data.c | 4 +-
> > > lib/namei.c | 2 +-
> > > lib/super.c | 1 +
> > > mkfs/main.c | 13 -
> > > 10 files changed, 582 insertions(+), 18 deletions(-)
> > > create mode 100644 fsck/Makefile.am
> > > create mode 100644 fsck/main.c
> > >
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 24e1d38..fc464e8 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -2,7 +2,7 @@
> > >
> > > ACLOCAL_AMFLAGS = -I m4
> > >
> > > -SUBDIRS = man lib mkfs dump
> > > +SUBDIRS = man lib mkfs dump fsck
> > > if ENABLE_FUSE
> > > SUBDIRS += fuse
> > > endif
> > > diff --git a/configure.ac b/configure.ac
> > > index b2c3225..5698b2e 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -298,5 +298,6 @@ AC_CONFIG_FILES([Makefile
> > > lib/Makefile
> > > mkfs/Makefile
> > > dump/Makefile
> > > - fuse/Makefile])
> > > + fuse/Makefile
> > > + fsck/Makefile])
> > > AC_OUTPUT
> > > diff --git a/fsck/Makefile.am b/fsck/Makefile.am
> > > new file mode 100644
> > > index 0000000..82973ba
> > > --- /dev/null
> > > +++ b/fsck/Makefile.am
> > > @@ -0,0 +1,9 @@
> > > +# SPDX-License-Identifier: GPL-2.0+
> > > +# Makefile.am
> > > +
> > > +AUTOMAKE_OPTIONS = foreign
> > > +bin_PROGRAMS = fsck.erofs
> > > +AM_CPPFLAGS = ${libuuid_CFLAGS}
> > > +fsck_erofs_SOURCES = main.c
> > > +fsck_erofs_CFLAGS = -Wall -Werror -I$(top_srcdir)/include
> > > +fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libuuid_LIBS} ${liblz4_LIBS}
> > > diff --git a/fsck/main.c b/fsck/main.c
> > > new file mode 100644
> > > index 0000000..c397d19
> > > --- /dev/null
> > > +++ b/fsck/main.c
> > > @@ -0,0 +1,548 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright 2021 Google LLC
> > > + * Author: Daeho Jeong <daehojeong@google.com>
> > > + */
> > > +#include <stdlib.h>
> > > +#include <getopt.h>
> > > +#include <time.h>
> > > +#include "erofs/print.h"
> > > +#include "erofs/io.h"
> > > +
> > > +enum {
> > > + NO_PRINT_COMP_RATIO = 0,
> > > + PRINT_COMP_RATIO_ALL = 1,
> > > + PRINT_COMP_RATIO_COMP_FILE = 2,
> > > +};
> > > +
> > > +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
> > > +
> > > +struct erofsfsck_cfg {
> > > + bool corrupted;
> > > + int print_comp_ratio;
> > > + u64 ondisk_len;
> > > + u64 logical_len;
> > > +};
> > > +static struct erofsfsck_cfg fsckcfg;
> > > +
> > > +static struct option long_options[] = {
> > > + {"help", no_argument, 0, 1},
> > > + {0, 0, 0, 0},
> > > +};
> > > +
> > > +static void usage(void)
> > > +{
> > > + fputs("usage: [options] IMAGE\n\n"
> > > + "Check erofs filesystem integrity of IMAGE, and [options] are:\n"
> > > + " -V print the version number of fsck.erofs and exit.\n"
> > > + " -d# set output message level to # (maximum 9)\n"
> > > + " -c print total compression ratio of all compressed files\n"
> > > + " -C print total compression ratio of all files\n"
> >
> > Apart from erofsdump consideration.. Can we merge these 2 options into one?
>
> Sure~
>
> >
> > > + " --help display this help and exit.\n",
> > > + stderr);
> > > +}
> > > +
> > > +static void erofsfsck_print_version(void)
> > > +{
> > > + fprintf(stderr, "fsck.erofs %s\n", cfg.c_version);
> > > +}
> > > +
> > > +static int erofsfsck_parse_options_cfg(int argc, char **argv)
> > > +{
> > > + int opt, i;
> > > +
> > > + while ((opt = getopt_long(argc, argv, "Vd:Cc",
> > > + long_options, NULL)) != -1) {
> > > + switch (opt) {
> > > + case 'V':
> > > + erofsfsck_print_version();
> > > + exit(0);
> > > + case 'd':
> > > + i = atoi(optarg);
> > > + if (i < EROFS_MSG_MIN || i > EROFS_MSG_MAX) {
> > > + erofs_err("invalid debug level %d", i);
> > > + return -EINVAL;
> > > + }
> > > + cfg.c_dbg_lvl = i;
> > > + break;
> > > + case 'C':
> > > + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_ALL;
> > > + break;
> > > + case 'c':
> > > + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_COMP_FILE;
> > > + break;
> > > + case 1:
> > > + usage();
> > > + exit(0);
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > + }
> > > +
> > > + if (optind >= argc)
> > > + return -EINVAL;
> > > +
> > > + cfg.c_img_path = strdup(argv[optind++]);
> > > + if (!cfg.c_img_path)
> > > + return -ENOMEM;
> > > +
> > > + if (optind < argc) {
> > > + erofs_err("unexpected argument: %s\n", argv[optind]);
> > > + return -EINVAL;
> > > + }
> > > + return 0;
> > > +}
> > > +
> > > +static int erofs_check_sb_chksum(void)
> > > +{
> > > + int ret;
> > > + u8 buf[EROFS_BLKSIZ];
> > > + u32 crc;
> > > + struct erofs_super_block *sb;
> > > +
> > > + ret = blk_read(buf, 0, 1);
> > > + if (ret) {
> > > + erofs_err("failed to read superblock to check checksum: "
> > > + "errno(%d)", ret);
> > > + return -1;
> > > + }
> > > +
> > > + sb = (struct erofs_super_block *)(buf + EROFS_SUPER_OFFSET);
> > > + sb->checksum = 0;
> > > +
> > > + crc = crc32c(~0, (u8 *)sb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
> > > + if (crc != sbi.checksum) {
> > > + erofs_err("superblock checksum doesn't match: saved(0x%08x) "
> > > + "calculated(0x%08x)", sbi.checksum, crc);
> > > + fsckcfg.corrupted = true;
> > > + return -1;
> > > + }
> > > + return 0;
> > > +}
> > > +
> > > +static int check_special_dentries(struct erofs_dirent *de, const char *de_name,
> > > + unsigned int de_namelen, erofs_nid_t nid,
> > > + erofs_nid_t pnid, bool is_curdir)
> > > +{
> > > + unsigned int dirname_len = is_curdir ? 1 : 2;
> > > + const char *dirname = is_curdir ? "." : "..";
> > > + erofs_nid_t correct_nid = is_curdir ? nid : pnid;
> > > +
> > > + if (de_namelen != dirname_len || memcmp(de_name, dirname, de_namelen)) {
> > > + char *dbgname = strndup(de_name, de_namelen);
> > > +
> > > + BUG_ON(!dbgname);
> > > + if (is_curdir)
> > > + erofs_err("wrong current dir name(%s) @ nid(%llu)",
> > > + dbgname, nid | 0ULL);
> > > + else
> > > + erofs_err("wrong parent dir name(%s) @ nid(%llu)",
> > > + dbgname, nid | 0ULL);
> > > + free(dbgname);
> > > + return -1;
> > > + }
> > > +
> > > + if (de->nid != correct_nid) {
> > > + if (is_curdir)
> > > + erofs_err("wrong current dir nid(%llu) @ nid(%llu)",
> > > + de->nid | 0ULL, nid | 0ULL);
> > > + else
> > > + erofs_err("wrong parent dir nid(%llu): "
> > > + "pnid(%llu) @ nid(%llu)",
> > > + de->nid | 0ULL, pnid | 0ULL, nid | 0ULL);
> > > + return -1;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int traverse_dirents(erofs_nid_t pnid, erofs_nid_t nid,
> > > + void *dentry_blk, erofs_off_t offset,
> > > + unsigned int next_nameoff, unsigned int maxsize)
> >
> > Not quite sure if we could introduce some
> > erofs_foreach_dirent() and customized callbacks to cleanup similar functions
> > in fuse and fsck...
>
> Let me think about it more.
That is not quite urgent for now, we can stay as this.
>
> > > +{
> > > + struct erofs_dirent *de = dentry_blk;
> > > + const struct erofs_dirent *end = dentry_blk + next_nameoff;
> > > + unsigned int idx = 0;
> > > +
> > > + erofs_dbg("traversing pnid(%llu), nid(%llu)", pnid | 0ULL, nid | 0ULL);
> > > +
> > > + if (offset == 0 && (next_nameoff < 2 * sizeof(struct erofs_dirent))) {
> > > + erofs_err("too small dirents of size(%d) in nid(%llu)",
> > > + next_nameoff, nid | 0ULL);
> > > + return -EFSCORRUPTED;
> > > + }
> > > +
> > > + while (de < end) {
> > > + const char *de_name;
> > > + unsigned int de_namelen;
> > > + unsigned int nameoff;
> > > + char *dbgname;
> > > +
> > > + nameoff = le16_to_cpu(de->nameoff);
> > > + de_name = (char *)dentry_blk + nameoff;
> > > +
> > > + /* the last dirent check */
> > > + if (de + 1 >= end)
> > > + de_namelen = strnlen(de_name, maxsize - nameoff);
> > > + else
> > > + de_namelen = le16_to_cpu(de[1].nameoff) - nameoff;
> > > +
> > > + if (cfg.c_dbg_lvl >= EROFS_DBG) {
> > > + dbgname = strndup(de_name, de_namelen);
> > > + BUG_ON(!dbgname);
> > > + erofs_dbg("traversed filename(%s)", dbgname);
> > > + free(dbgname);
> > > + }
> > > +
> > > + /* corrupted entry check */
> > > + if (nameoff != next_nameoff) {
> > > + erofs_err("bogus dirent with nameoff(%u): expected(%d) "
> > > + "@ nid(%llu), offset(%llu), idx(%u)",
> > > + nameoff, next_nameoff, nid | 0ULL,
> > > + offset | 0ULL, idx);
> > > + return -EFSCORRUPTED;
> > > + }
> > > +
> > > + if (nameoff + de_namelen > maxsize ||
> > > + de_namelen > EROFS_NAME_LEN) {
> > > + erofs_err("bogus dirent with namelen(%u) @ nid(%llu), "
> > > + "offset(%llu), idx(%u)",
> > > + de_namelen, nid | 0ULL, offset | 0ULL, idx);
> > > + return -EFSCORRUPTED;
> > > + }
> > > +
> > > + if (offset == 0 && (idx == 0 || idx == 1)) {
> >
> > We may not assume "." and ".." special dirents as 0 or 1... Actually
> > all dirents are sorted in the alphabetical order. I'm not sure if
> > some dirents are smaller than "." and "..".
>
> Ok, I will remove this check here.
Ok.
>
> > We could also check if all dirents are in alphabetical order as well
> > since it's used to do binary search runtimely.
>
> I will add a check routine to check all dirents are in alphabetical order.
Ok.
>
> >
> > > + if (check_special_dentries(de, de_name, de_namelen, nid,
> > > + pnid, idx == 0))
> > > + return -EFSCORRUPTED;
> > > + } else {
> > > + erofs_check_inode(nid, de->nid);
> > > + if (fsckcfg.corrupted)
> > > + return -EFSCORRUPTED;
> > > + }
> > > +
> > > + next_nameoff += de_namelen;
> > > + ++de;
> > > + ++idx;
> > > + }
> > > +
> > > + erofs_dbg("traversing ... done nid(%llu)", nid | 0ULL);
> > > + return 0;
> > > +}
> > > +
> > > +static int verify_raw_data_chunk(struct erofs_inode *inode)
> > > +{
> > > + struct erofs_map_blocks map = {
> > > + .index = UINT_MAX,
> > > + };
> > > + int ret;
> > > + erofs_off_t ptr = 0;
> > > +
> > > + if (fsckcfg.print_comp_ratio == PRINT_COMP_RATIO_ALL) {
> > > + fsckcfg.logical_len += inode->i_size;
> > > + fsckcfg.ondisk_len += inode->i_size;
> > > + }
> > > +
> > > + while (ptr < inode->i_size) {
> > > + map.m_la = ptr;
> > > + ret = erofs_map_blocks(inode, &map, 0);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + if (map.m_plen != map.m_llen || ptr != map.m_la) {
> > > + erofs_err("broken data chunk layout m_la %" PRIu64
> > > + " ptr %" PRIu64 " m_llen %" PRIu64 " m_plen %"
> > > + PRIu64, map.m_la, ptr, map.m_llen,
> > > + map.m_plen);
> > > + return -EFSCORRUPTED;
> > > + }
> > > +
> > > + if (!(map.m_flags & EROFS_MAP_MAPPED) && !map.m_llen) {
> > > + /* readched EOF */
> > > + ptr = inode->i_size;
> > > + continue;
> > > + }
> > > +
> > > + ptr += map.m_llen;
> > > + }
> > > + return 0;
> > > +}
> > > +
> > > +static int verify_compressed_chunk(struct erofs_inode *inode)
> > > +{
> > > + struct erofs_map_blocks map = {
> > > + .index = UINT_MAX,
> > > + };
> > > + int ret = 0;
> > > + bool count_pchunk = fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO;
> > > + u64 pchunk_len = 0;
> > > + erofs_off_t offset = 0, end = inode->i_size;
> > > +
> > > + while (end > offset) {
> > > + map.m_la = end - 1;
> > > +
> > > + ret = z_erofs_map_blocks_iter(inode, &map);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + if (end > map.m_la + map.m_llen) {
> > > + erofs_err("broken compressed chunk layout m_la %" PRIu64
> > > + " m_llen %" PRIu64 " end %" PRIu64,
> > > + map.m_la, map.m_llen, end);
> > > + return -EFSCORRUPTED;
> > > + }
> >
> > IMO, A simple way to check this is to try to decompress by calling
> > z_erofs_decompress() with .partial_decoding == 0 and see if any
> > error when decompressing as well.
>
> Got it~
Yeah, thanks for doing this again!
Thanks,
Gao Xiang
>
> > > +
> > > + if (count_pchunk)
> > > + pchunk_len += map.m_plen;
> > > +
> > > + end = map.m_la;
> > > + }
> > > +
> > > + if (count_pchunk) {
> > > + fsckcfg.logical_len += inode->i_size;
> > > + fsckcfg.ondisk_len += pchunk_len;
> >
> > we can also check another field called compressed_blocks in on-disk
> > inode...
>
> ditto
>
>
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int erofs_verify_xattr(struct erofs_inode *inode)
> > > +{
> > > + unsigned int xattr_hdr_size = sizeof(struct erofs_xattr_ibody_header);
> > > + unsigned int xattr_entry_size = sizeof(struct erofs_xattr_entry);
> > > + erofs_off_t addr;
> > > + unsigned int ofs, xattr_shared_count;
> > > + struct erofs_xattr_ibody_header *ih;
> > > + struct erofs_xattr_entry *entry;
> > > + int i, remaining = inode->xattr_isize, ret = 0;
> > > + char *buf = calloc(EROFS_BLKSIZ, 1);
> > > +
> > > + BUG_ON(!buf);
> > > +
> > > + if (inode->xattr_isize == xattr_hdr_size) {
> > > + erofs_err("xattr_isize %d of nid %llu is not supported yet",
> > > + inode->xattr_isize, inode->nid | 0ULL);
> > > + ret = -EFSCORRUPTED;
> > > + goto out;
> > > + } else if (inode->xattr_isize < xattr_hdr_size) {
> > > + if (inode->xattr_isize) {
> > > + erofs_err("bogus xattr ibody @ nid %llu",
> > > + inode->nid | 0ULL);
> > > + ret = -EFSCORRUPTED;
> > > + goto out;
> > > + }
> > > + }
> > > +
> > > + addr = iloc(inode->nid) + inode->inode_isize;
> > > + ret = dev_read(buf, addr, xattr_hdr_size);
> > > + if (ret < 0) {
> > > + erofs_err("an error occurred when reading xattr header "
> > > + "of nid(%llu): errno(%d)", inode->nid | 0ULL, ret);
> > > + goto out;
> > > + }
> > > + ih = (struct erofs_xattr_ibody_header *)buf;
> > > + xattr_shared_count = ih->h_shared_count;
> > > +
> > > + ofs = erofs_blkoff(addr) + xattr_hdr_size;
> > > + addr += xattr_hdr_size;
> > > + remaining -= xattr_hdr_size;
> > > + for (i = 0; i < xattr_shared_count; ++i) {
> > > + if (ofs >= EROFS_BLKSIZ) {
> > > + if (ofs != EROFS_BLKSIZ) {
> > > + erofs_err("unaligned xattr entry in "
> > > + "xattr shared area of nid(%llu)",
> > > + inode->nid | 0ULL);
> > > + ret = -EFSCORRUPTED;
> >
> > IMO, it won't happen due to proper alignment.. but I'm fine as this too.
> >
> > Thanks,
> > Gao Xiang
> >
> > > + goto out;
> > > + }
> > > + ofs = 0;
> > > + }
> > > + ofs += xattr_entry_size;
> > > + addr += xattr_entry_size;
> > > + remaining -= xattr_entry_size;
> > > + }
> > > +
> > > + while (remaining > 0) {
> > > + unsigned int entry_sz;
> > > +
> > > + ret = dev_read(buf, addr, xattr_entry_size);
> > > + if (ret) {
> > > + erofs_err("an error occurred when reading xattr entry "
> > > + "of nid(%llu): errno(%d)",
> > > + inode->nid | 0ULL, ret);
> > > + goto out;
> > > + }
> > > +
> > > + entry = (struct erofs_xattr_entry *)buf;
> > > + entry_sz = erofs_xattr_entry_size(entry);
> > > + if (remaining < entry_sz) {
> > > + erofs_err("xattr on-disk corruption: xattr entry "
> > > + "beyond xattr_isize of nid(%llu)",
> > > + inode->nid | 0ULL);
> > > + ret = -EFSCORRUPTED;
> > > + goto out;
> > > + }
> > > + addr += entry_sz;
> > > + remaining -= entry_sz;
> > > + }
> > > +out:
> > > + free(buf);
> > > + return ret;
> > > +}
> > > +
> > > +static int erofs_verify_data_chunk(struct erofs_inode *inode)
> > > +{
> > > + int ret;
> > > +
> > > + erofs_dbg("verify data chunk of nid(%llu): type(%d)",
> > > + inode->nid | 0ULL, inode->datalayout);
> > > +
> > > + switch (inode->datalayout) {
> > > + case EROFS_INODE_FLAT_PLAIN:
> > > + case EROFS_INODE_FLAT_INLINE:
> > > + case EROFS_INODE_CHUNK_BASED:
> > > + ret = verify_raw_data_chunk(inode);
> > > + break;
> > > + case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
> > > + case EROFS_INODE_FLAT_COMPRESSION:
> > > + ret = verify_compressed_chunk(inode);
> > > + break;
> > > + default:
> > > + ret = -EINVAL;
> > > + break;
> > > + }
> > > +
> > > + if (ret == -EIO)
> > > + erofs_err("I/O error occurred when verifying "
> > > + "data chunk of nid(%llu)", inode->nid | 0ULL);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid)
> > > +{
> > > + int ret;
> > > + struct erofs_inode *inode;
> > > + char *buf;
> > > + erofs_off_t offset;
> > > +
> > > + erofs_dbg("check inode: nid(%llu)", nid | 0ULL);
> > > + inode = calloc(1, sizeof(struct erofs_inode));
> > > + BUG_ON(!inode);
> > > + buf = calloc(EROFS_BLKSIZ, 1);
> > > + BUG_ON(!buf);
> > > +
> > > + inode->nid = nid;
> > > + ret = erofs_read_inode_from_disk(inode);
> > > + if (ret) {
> > > + if (ret == -EIO)
> > > + erofs_err("I/O error occurred when reading nid(%llu)",
> > > + nid | 0ULL);
> > > + goto out;
> > > + }
> > > +
> > > + /* verify xattr field */
> > > + ret = erofs_verify_xattr(inode);
> > > + if (ret)
> > > + goto out;
> > > +
> > > + /* verify data chunk layout */
> > > + ret = erofs_verify_data_chunk(inode);
> > > + if (ret)
> > > + goto out;
> > > +
> > > + if ((inode->i_mode & S_IFMT) != S_IFDIR)
> > > + goto out;
> > > +
> > > + offset = 0;
> > > + while (offset < inode->i_size) {
> > > + erofs_off_t maxsize = min_t(erofs_off_t,
> > > + inode->i_size - offset, EROFS_BLKSIZ);
> > > + struct erofs_dirent *de = (void *)buf;
> > > + unsigned int nameoff;
> > > +
> > > + ret = erofs_pread(inode, buf, maxsize, offset);
> > > + if (ret) {
> > > + erofs_err("I/O error occurred when reading dirents: "
> > > + "nid(%llu), offset(%llu), size(%llu)",
> > > + nid | 0ULL, offset | 0ULL, maxsize | 0ULL);
> > > + goto out;
> > > + }
> > > +
> > > + nameoff = le16_to_cpu(de->nameoff);
> > > + if (nameoff < sizeof(struct erofs_dirent) ||
> > > + nameoff >= PAGE_SIZE) {
> > > + erofs_err("invalid de[0].nameoff %u @ nid(%llu), "
> > > + "offset(%llu)",
> > > + nameoff, nid | 0ULL, offset | 0ULL);
> > > + ret = -EFSCORRUPTED;
> > > + goto out;
> > > + }
> > > +
> > > + ret = traverse_dirents(pnid, nid, buf, offset,
> > > + nameoff, maxsize);
> > > + if (ret)
> > > + goto out;
> > > +
> > > + offset += maxsize;
> > > + }
> > > +out:
> > > + free(buf);
> > > + free(inode);
> > > + if (ret && ret != -EIO)
> > > + fsckcfg.corrupted = true;
> > > +}
> > > +
> > > +int main(int argc, char **argv)
> > > +{
> > > + int err;
> > > +
> > > + erofs_init_configure();
> > > +
> > > + fsckcfg.corrupted = false;
> > > + fsckcfg.print_comp_ratio = NO_PRINT_COMP_RATIO;
> > > + fsckcfg.logical_len = 0;
> > > + fsckcfg.ondisk_len = 0;
> > > +
> > > + err = erofsfsck_parse_options_cfg(argc, argv);
> > > + if (err) {
> > > + if (err == -EINVAL)
> > > + usage();
> > > + goto exit;
> > > + }
> > > +
> > > + err = dev_open_ro(cfg.c_img_path);
> > > + if (err) {
> > > + erofs_err("failed to open image file");
> > > + goto exit;
> > > + }
> > > +
> > > + err = erofs_read_superblock();
> > > + if (err) {
> > > + erofs_err("failed to read superblock");
> > > + goto exit;
> > > + }
> > > +
> > > + if (erofs_sb_has_sb_chksum() && erofs_check_sb_chksum()) {
> > > + erofs_err("failed to verify superblock checksum");
> > > + goto exit;
> > > + }
> > > +
> > > + erofs_check_inode(sbi.root_nid, sbi.root_nid);
> > > +
> > > + if (fsckcfg.corrupted) {
> > > + fprintf(stderr, "Found some filesystem corruption\n");
> > > + } else {
> > > + fprintf(stderr, "No error found\n");
> > > + if (fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO) {
> > > + double comp_ratio = (double)fsckcfg.ondisk_len * 100 /
> > > + (double)fsckcfg.logical_len;
> > > + fprintf(stderr, "Compression Ratio: %.2f(%%)\n",
> > > + comp_ratio);
> > > + }
> > > + }
> > > +
> > > +exit:
> > > + erofs_exit_configure();
> > > + return err;
> > > +}
> > > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > > index 8b154ed..80065b2 100644
> > > --- a/include/erofs/internal.h
> > > +++ b/include/erofs/internal.h
> > > @@ -82,6 +82,8 @@ struct erofs_sb_info {
> > >
> > > u16 available_compr_algs;
> > > u16 lz4_max_distance;
> > > +
> > > + u32 checksum;
> > > };
> > >
> > > /* global sbi */
> > > @@ -264,10 +266,13 @@ int erofs_read_superblock(void);
> > >
> > > /* namei.c */
> > > int erofs_ilookup(const char *path, struct erofs_inode *vi);
> > > +int erofs_read_inode_from_disk(struct erofs_inode *vi);
> > >
> > > /* data.c */
> > > int erofs_pread(struct erofs_inode *inode, char *buf,
> > > erofs_off_t count, erofs_off_t offset);
> > > +int erofs_map_blocks(struct erofs_inode *inode,
> > > + struct erofs_map_blocks *map, int flags);
> > > /* zmap.c */
> > > int z_erofs_fill_inode(struct erofs_inode *vi);
> > > int z_erofs_map_blocks_iter(struct erofs_inode *vi,
> > > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > > index 66a68e3..62e9981 100644
> > > --- a/include/erofs_fs.h
> > > +++ b/include/erofs_fs.h
> > > @@ -400,4 +400,17 @@ static inline void erofs_check_ondisk_layout_definitions(void)
> > > Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
> > > }
> > >
> > > +#define CRC32C_POLY_LE 0x82F63B78
> > > +static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
> > > +{
> > > + int i;
> > > +
> > > + while (len--) {
> > > + crc ^= *in++;
> > > + for (i = 0; i < 8; i++)
> > > + crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
> > > + }
> > > + return crc;
> > > +}
> > > +
> > > #endif
> > > diff --git a/lib/data.c b/lib/data.c
> > > index 641d840..6cb7eeb 100644
> > > --- a/lib/data.c
> > > +++ b/lib/data.c
> > > @@ -61,8 +61,8 @@ err_out:
> > > return err;
> > > }
> > >
> > > -static int erofs_map_blocks(struct erofs_inode *inode,
> > > - struct erofs_map_blocks *map, int flags)
> > > +int erofs_map_blocks(struct erofs_inode *inode,
> > > + struct erofs_map_blocks *map, int flags)
> > > {
> > > struct erofs_inode *vi = inode;
> > > struct erofs_inode_chunk_index *idx;
> > > diff --git a/lib/namei.c b/lib/namei.c
> > > index b4bdabf..56f199a 100644
> > > --- a/lib/namei.c
> > > +++ b/lib/namei.c
> > > @@ -22,7 +22,7 @@ static dev_t erofs_new_decode_dev(u32 dev)
> > > return makedev(major, minor);
> > > }
> > >
> > > -static int erofs_read_inode_from_disk(struct erofs_inode *vi)
> > > +int erofs_read_inode_from_disk(struct erofs_inode *vi)
> > > {
> > > int ret, ifmt;
> > > char buf[sizeof(struct erofs_inode_extended)];
> > > diff --git a/lib/super.c b/lib/super.c
> > > index 0fa69ab..0c30403 100644
> > > --- a/lib/super.c
> > > +++ b/lib/super.c
> > > @@ -62,6 +62,7 @@ int erofs_read_superblock(void)
> > > sbi.islotbits = EROFS_ISLOTBITS;
> > > sbi.root_nid = le16_to_cpu(dsb->root_nid);
> > > sbi.inos = le64_to_cpu(dsb->inos);
> > > + sbi.checksum = le32_to_cpu(dsb->checksum);
> > >
> > > sbi.build_time = le64_to_cpu(dsb->build_time);
> > > sbi.build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
> > > diff --git a/mkfs/main.c b/mkfs/main.c
> > > index 1c8dea5..b9b46f5 100644
> > > --- a/mkfs/main.c
> > > +++ b/mkfs/main.c
> > > @@ -424,19 +424,6 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
> > > return 0;
> > > }
> > >
> > > -#define CRC32C_POLY_LE 0x82F63B78
> > > -static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
> > > -{
> > > - int i;
> > > -
> > > - while (len--) {
> > > - crc ^= *in++;
> > > - for (i = 0; i < 8; i++)
> > > - crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
> > > - }
> > > - return crc;
> > > -}
> > > -
> > > static int erofs_mkfs_superblock_csum_set(void)
> > > {
> > > int ret;
> > > --
> > > 2.33.0.1079.g6e70778dc9-goog
> > >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] erofs-utils: introduce fsck.erofs
2021-10-27 3:13 ` Gao Xiang
@ 2021-10-28 0:50 ` Daeho Jeong
2021-10-28 1:00 ` Gao Xiang
0 siblings, 1 reply; 13+ messages in thread
From: Daeho Jeong @ 2021-10-28 0:50 UTC (permalink / raw)
To: Gao Xiang; +Cc: Daeho Jeong, linux-erofs, Jaegeuk Kim, miaoxie, Wang Qi
Hi Gao,
On Tue, Oct 26, 2021 at 8:14 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>
> Hi Daeho,
>
> On Tue, Oct 26, 2021 at 11:35:44AM -0700, Daeho Jeong wrote:
> > > Many thanks for the patch! I also think some fsck feature is useful in
> > > order to check the image integration.
> > >
> > > IMO, Compression ratio calculation is more like a dump fs feature.
> > > Wang Qi developed the erofsdump yet he told me that he is working for
> > > another urgent stuffs for now so he delayed the work:
> > > https://lore.kernel.org/r/20210915093537.2579575-1-guoxuenan@huawei.com
> >
> > Right, it's better to move these feature to dump.erofs.
>
> I'm fine to add "Compression ratio calculation" temporarily in
> fsck.erofs as long as active development.
>
> Let's see if fsck feature can match erofs-utils 1.4 timepoint.
How is the timeline for erofs-utils 1.4?
>
> >
> > >
> > > fsck and dump have the similar logic (and like f2fs-tools, we can also
> > > have one program for this 2 features). If fsck is actively developed,
> > > I'm pretty fine to use the fsck codebase and merge erofsdump into it
> > > later instead.
> >
> > Like f2fs tools, I think we can maintain two tools separately.
>
> Yeah, my point is two tools share one common logic in the final shape,
> but I'm fine with the current status as well :)
>
> > But, if you want, we can merge them into one tool.
> >
> > > Some comments as below:
> > >
> > > > ---
> > > > Makefile.am | 2 +-
> > > > configure.ac | 3 +-
> > > > fsck/Makefile.am | 9 +
> > > > fsck/main.c | 548 +++++++++++++++++++++++++++++++++++++++
> > > > include/erofs/internal.h | 5 +
> > > > include/erofs_fs.h | 13 +
> > > > lib/data.c | 4 +-
> > > > lib/namei.c | 2 +-
> > > > lib/super.c | 1 +
> > > > mkfs/main.c | 13 -
> > > > 10 files changed, 582 insertions(+), 18 deletions(-)
> > > > create mode 100644 fsck/Makefile.am
> > > > create mode 100644 fsck/main.c
> > > >
> > > > diff --git a/Makefile.am b/Makefile.am
> > > > index 24e1d38..fc464e8 100644
> > > > --- a/Makefile.am
> > > > +++ b/Makefile.am
> > > > @@ -2,7 +2,7 @@
> > > >
> > > > ACLOCAL_AMFLAGS = -I m4
> > > >
> > > > -SUBDIRS = man lib mkfs dump
> > > > +SUBDIRS = man lib mkfs dump fsck
> > > > if ENABLE_FUSE
> > > > SUBDIRS += fuse
> > > > endif
> > > > diff --git a/configure.ac b/configure.ac
> > > > index b2c3225..5698b2e 100644
> > > > --- a/configure.ac
> > > > +++ b/configure.ac
> > > > @@ -298,5 +298,6 @@ AC_CONFIG_FILES([Makefile
> > > > lib/Makefile
> > > > mkfs/Makefile
> > > > dump/Makefile
> > > > - fuse/Makefile])
> > > > + fuse/Makefile
> > > > + fsck/Makefile])
> > > > AC_OUTPUT
> > > > diff --git a/fsck/Makefile.am b/fsck/Makefile.am
> > > > new file mode 100644
> > > > index 0000000..82973ba
> > > > --- /dev/null
> > > > +++ b/fsck/Makefile.am
> > > > @@ -0,0 +1,9 @@
> > > > +# SPDX-License-Identifier: GPL-2.0+
> > > > +# Makefile.am
> > > > +
> > > > +AUTOMAKE_OPTIONS = foreign
> > > > +bin_PROGRAMS = fsck.erofs
> > > > +AM_CPPFLAGS = ${libuuid_CFLAGS}
> > > > +fsck_erofs_SOURCES = main.c
> > > > +fsck_erofs_CFLAGS = -Wall -Werror -I$(top_srcdir)/include
> > > > +fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libuuid_LIBS} ${liblz4_LIBS}
> > > > diff --git a/fsck/main.c b/fsck/main.c
> > > > new file mode 100644
> > > > index 0000000..c397d19
> > > > --- /dev/null
> > > > +++ b/fsck/main.c
> > > > @@ -0,0 +1,548 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * Copyright 2021 Google LLC
> > > > + * Author: Daeho Jeong <daehojeong@google.com>
> > > > + */
> > > > +#include <stdlib.h>
> > > > +#include <getopt.h>
> > > > +#include <time.h>
> > > > +#include "erofs/print.h"
> > > > +#include "erofs/io.h"
> > > > +
> > > > +enum {
> > > > + NO_PRINT_COMP_RATIO = 0,
> > > > + PRINT_COMP_RATIO_ALL = 1,
> > > > + PRINT_COMP_RATIO_COMP_FILE = 2,
> > > > +};
> > > > +
> > > > +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
> > > > +
> > > > +struct erofsfsck_cfg {
> > > > + bool corrupted;
> > > > + int print_comp_ratio;
> > > > + u64 ondisk_len;
> > > > + u64 logical_len;
> > > > +};
> > > > +static struct erofsfsck_cfg fsckcfg;
> > > > +
> > > > +static struct option long_options[] = {
> > > > + {"help", no_argument, 0, 1},
> > > > + {0, 0, 0, 0},
> > > > +};
> > > > +
> > > > +static void usage(void)
> > > > +{
> > > > + fputs("usage: [options] IMAGE\n\n"
> > > > + "Check erofs filesystem integrity of IMAGE, and [options] are:\n"
> > > > + " -V print the version number of fsck.erofs and exit.\n"
> > > > + " -d# set output message level to # (maximum 9)\n"
> > > > + " -c print total compression ratio of all compressed files\n"
> > > > + " -C print total compression ratio of all files\n"
> > >
> > > Apart from erofsdump consideration.. Can we merge these 2 options into one?
> >
> > Sure~
> >
> > >
> > > > + " --help display this help and exit.\n",
> > > > + stderr);
> > > > +}
> > > > +
> > > > +static void erofsfsck_print_version(void)
> > > > +{
> > > > + fprintf(stderr, "fsck.erofs %s\n", cfg.c_version);
> > > > +}
> > > > +
> > > > +static int erofsfsck_parse_options_cfg(int argc, char **argv)
> > > > +{
> > > > + int opt, i;
> > > > +
> > > > + while ((opt = getopt_long(argc, argv, "Vd:Cc",
> > > > + long_options, NULL)) != -1) {
> > > > + switch (opt) {
> > > > + case 'V':
> > > > + erofsfsck_print_version();
> > > > + exit(0);
> > > > + case 'd':
> > > > + i = atoi(optarg);
> > > > + if (i < EROFS_MSG_MIN || i > EROFS_MSG_MAX) {
> > > > + erofs_err("invalid debug level %d", i);
> > > > + return -EINVAL;
> > > > + }
> > > > + cfg.c_dbg_lvl = i;
> > > > + break;
> > > > + case 'C':
> > > > + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_ALL;
> > > > + break;
> > > > + case 'c':
> > > > + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_COMP_FILE;
> > > > + break;
> > > > + case 1:
> > > > + usage();
> > > > + exit(0);
> > > > + default:
> > > > + return -EINVAL;
> > > > + }
> > > > + }
> > > > +
> > > > + if (optind >= argc)
> > > > + return -EINVAL;
> > > > +
> > > > + cfg.c_img_path = strdup(argv[optind++]);
> > > > + if (!cfg.c_img_path)
> > > > + return -ENOMEM;
> > > > +
> > > > + if (optind < argc) {
> > > > + erofs_err("unexpected argument: %s\n", argv[optind]);
> > > > + return -EINVAL;
> > > > + }
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int erofs_check_sb_chksum(void)
> > > > +{
> > > > + int ret;
> > > > + u8 buf[EROFS_BLKSIZ];
> > > > + u32 crc;
> > > > + struct erofs_super_block *sb;
> > > > +
> > > > + ret = blk_read(buf, 0, 1);
> > > > + if (ret) {
> > > > + erofs_err("failed to read superblock to check checksum: "
> > > > + "errno(%d)", ret);
> > > > + return -1;
> > > > + }
> > > > +
> > > > + sb = (struct erofs_super_block *)(buf + EROFS_SUPER_OFFSET);
> > > > + sb->checksum = 0;
> > > > +
> > > > + crc = crc32c(~0, (u8 *)sb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
> > > > + if (crc != sbi.checksum) {
> > > > + erofs_err("superblock checksum doesn't match: saved(0x%08x) "
> > > > + "calculated(0x%08x)", sbi.checksum, crc);
> > > > + fsckcfg.corrupted = true;
> > > > + return -1;
> > > > + }
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int check_special_dentries(struct erofs_dirent *de, const char *de_name,
> > > > + unsigned int de_namelen, erofs_nid_t nid,
> > > > + erofs_nid_t pnid, bool is_curdir)
> > > > +{
> > > > + unsigned int dirname_len = is_curdir ? 1 : 2;
> > > > + const char *dirname = is_curdir ? "." : "..";
> > > > + erofs_nid_t correct_nid = is_curdir ? nid : pnid;
> > > > +
> > > > + if (de_namelen != dirname_len || memcmp(de_name, dirname, de_namelen)) {
> > > > + char *dbgname = strndup(de_name, de_namelen);
> > > > +
> > > > + BUG_ON(!dbgname);
> > > > + if (is_curdir)
> > > > + erofs_err("wrong current dir name(%s) @ nid(%llu)",
> > > > + dbgname, nid | 0ULL);
> > > > + else
> > > > + erofs_err("wrong parent dir name(%s) @ nid(%llu)",
> > > > + dbgname, nid | 0ULL);
> > > > + free(dbgname);
> > > > + return -1;
> > > > + }
> > > > +
> > > > + if (de->nid != correct_nid) {
> > > > + if (is_curdir)
> > > > + erofs_err("wrong current dir nid(%llu) @ nid(%llu)",
> > > > + de->nid | 0ULL, nid | 0ULL);
> > > > + else
> > > > + erofs_err("wrong parent dir nid(%llu): "
> > > > + "pnid(%llu) @ nid(%llu)",
> > > > + de->nid | 0ULL, pnid | 0ULL, nid | 0ULL);
> > > > + return -1;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int traverse_dirents(erofs_nid_t pnid, erofs_nid_t nid,
> > > > + void *dentry_blk, erofs_off_t offset,
> > > > + unsigned int next_nameoff, unsigned int maxsize)
> > >
> > > Not quite sure if we could introduce some
> > > erofs_foreach_dirent() and customized callbacks to cleanup similar functions
> > > in fuse and fsck...
> >
> > Let me think about it more.
>
> That is not quite urgent for now, we can stay as this.
>
> >
> > > > +{
> > > > + struct erofs_dirent *de = dentry_blk;
> > > > + const struct erofs_dirent *end = dentry_blk + next_nameoff;
> > > > + unsigned int idx = 0;
> > > > +
> > > > + erofs_dbg("traversing pnid(%llu), nid(%llu)", pnid | 0ULL, nid | 0ULL);
> > > > +
> > > > + if (offset == 0 && (next_nameoff < 2 * sizeof(struct erofs_dirent))) {
> > > > + erofs_err("too small dirents of size(%d) in nid(%llu)",
> > > > + next_nameoff, nid | 0ULL);
> > > > + return -EFSCORRUPTED;
> > > > + }
> > > > +
> > > > + while (de < end) {
> > > > + const char *de_name;
> > > > + unsigned int de_namelen;
> > > > + unsigned int nameoff;
> > > > + char *dbgname;
> > > > +
> > > > + nameoff = le16_to_cpu(de->nameoff);
> > > > + de_name = (char *)dentry_blk + nameoff;
> > > > +
> > > > + /* the last dirent check */
> > > > + if (de + 1 >= end)
> > > > + de_namelen = strnlen(de_name, maxsize - nameoff);
> > > > + else
> > > > + de_namelen = le16_to_cpu(de[1].nameoff) - nameoff;
> > > > +
> > > > + if (cfg.c_dbg_lvl >= EROFS_DBG) {
> > > > + dbgname = strndup(de_name, de_namelen);
> > > > + BUG_ON(!dbgname);
> > > > + erofs_dbg("traversed filename(%s)", dbgname);
> > > > + free(dbgname);
> > > > + }
> > > > +
> > > > + /* corrupted entry check */
> > > > + if (nameoff != next_nameoff) {
> > > > + erofs_err("bogus dirent with nameoff(%u): expected(%d) "
> > > > + "@ nid(%llu), offset(%llu), idx(%u)",
> > > > + nameoff, next_nameoff, nid | 0ULL,
> > > > + offset | 0ULL, idx);
> > > > + return -EFSCORRUPTED;
> > > > + }
> > > > +
> > > > + if (nameoff + de_namelen > maxsize ||
> > > > + de_namelen > EROFS_NAME_LEN) {
> > > > + erofs_err("bogus dirent with namelen(%u) @ nid(%llu), "
> > > > + "offset(%llu), idx(%u)",
> > > > + de_namelen, nid | 0ULL, offset | 0ULL, idx);
> > > > + return -EFSCORRUPTED;
> > > > + }
> > > > +
> > > > + if (offset == 0 && (idx == 0 || idx == 1)) {
> > >
> > > We may not assume "." and ".." special dirents as 0 or 1... Actually
> > > all dirents are sorted in the alphabetical order. I'm not sure if
> > > some dirents are smaller than "." and "..".
> >
> > Ok, I will remove this check here.
>
> Ok.
>
> >
> > > We could also check if all dirents are in alphabetical order as well
> > > since it's used to do binary search runtimely.
> >
> > I will add a check routine to check all dirents are in alphabetical order.
>
> Ok.
>
> >
> > >
> > > > + if (check_special_dentries(de, de_name, de_namelen, nid,
> > > > + pnid, idx == 0))
> > > > + return -EFSCORRUPTED;
> > > > + } else {
> > > > + erofs_check_inode(nid, de->nid);
> > > > + if (fsckcfg.corrupted)
> > > > + return -EFSCORRUPTED;
> > > > + }
> > > > +
> > > > + next_nameoff += de_namelen;
> > > > + ++de;
> > > > + ++idx;
> > > > + }
> > > > +
> > > > + erofs_dbg("traversing ... done nid(%llu)", nid | 0ULL);
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int verify_raw_data_chunk(struct erofs_inode *inode)
> > > > +{
> > > > + struct erofs_map_blocks map = {
> > > > + .index = UINT_MAX,
> > > > + };
> > > > + int ret;
> > > > + erofs_off_t ptr = 0;
> > > > +
> > > > + if (fsckcfg.print_comp_ratio == PRINT_COMP_RATIO_ALL) {
> > > > + fsckcfg.logical_len += inode->i_size;
> > > > + fsckcfg.ondisk_len += inode->i_size;
> > > > + }
> > > > +
> > > > + while (ptr < inode->i_size) {
> > > > + map.m_la = ptr;
> > > > + ret = erofs_map_blocks(inode, &map, 0);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + if (map.m_plen != map.m_llen || ptr != map.m_la) {
> > > > + erofs_err("broken data chunk layout m_la %" PRIu64
> > > > + " ptr %" PRIu64 " m_llen %" PRIu64 " m_plen %"
> > > > + PRIu64, map.m_la, ptr, map.m_llen,
> > > > + map.m_plen);
> > > > + return -EFSCORRUPTED;
> > > > + }
> > > > +
> > > > + if (!(map.m_flags & EROFS_MAP_MAPPED) && !map.m_llen) {
> > > > + /* readched EOF */
> > > > + ptr = inode->i_size;
> > > > + continue;
> > > > + }
> > > > +
> > > > + ptr += map.m_llen;
> > > > + }
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int verify_compressed_chunk(struct erofs_inode *inode)
> > > > +{
> > > > + struct erofs_map_blocks map = {
> > > > + .index = UINT_MAX,
> > > > + };
> > > > + int ret = 0;
> > > > + bool count_pchunk = fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO;
> > > > + u64 pchunk_len = 0;
> > > > + erofs_off_t offset = 0, end = inode->i_size;
> > > > +
> > > > + while (end > offset) {
> > > > + map.m_la = end - 1;
> > > > +
> > > > + ret = z_erofs_map_blocks_iter(inode, &map);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + if (end > map.m_la + map.m_llen) {
> > > > + erofs_err("broken compressed chunk layout m_la %" PRIu64
> > > > + " m_llen %" PRIu64 " end %" PRIu64,
> > > > + map.m_la, map.m_llen, end);
> > > > + return -EFSCORRUPTED;
> > > > + }
> > >
> > > IMO, A simple way to check this is to try to decompress by calling
> > > z_erofs_decompress() with .partial_decoding == 0 and see if any
> > > error when decompressing as well.
> >
> > Got it~
>
> Yeah, thanks for doing this again!
In fact, I wanted to decompress the whole data here. We can't check
the data integrity,
so I just wanted to check the layout of the file and that is the
reason why I used z_erofs_map_blocks_iter() directly.
>
> Thanks,
> Gao Xiang
>
> >
> > > > +
> > > > + if (count_pchunk)
> > > > + pchunk_len += map.m_plen;
> > > > +
> > > > + end = map.m_la;
> > > > + }
> > > > +
> > > > + if (count_pchunk) {
> > > > + fsckcfg.logical_len += inode->i_size;
> > > > + fsckcfg.ondisk_len += pchunk_len;
> > >
> > > we can also check another field called compressed_blocks in on-disk
> > > inode...
> >
> > ditto
When we use compressed_blocks, we need to calculate the ratio based on
block count.
Then, I need to make the i_size aligned to BLK_SIZE. Which one do you
prefer, bytes or blocks?
Thanks~!
> >
> >
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int erofs_verify_xattr(struct erofs_inode *inode)
> > > > +{
> > > > + unsigned int xattr_hdr_size = sizeof(struct erofs_xattr_ibody_header);
> > > > + unsigned int xattr_entry_size = sizeof(struct erofs_xattr_entry);
> > > > + erofs_off_t addr;
> > > > + unsigned int ofs, xattr_shared_count;
> > > > + struct erofs_xattr_ibody_header *ih;
> > > > + struct erofs_xattr_entry *entry;
> > > > + int i, remaining = inode->xattr_isize, ret = 0;
> > > > + char *buf = calloc(EROFS_BLKSIZ, 1);
> > > > +
> > > > + BUG_ON(!buf);
> > > > +
> > > > + if (inode->xattr_isize == xattr_hdr_size) {
> > > > + erofs_err("xattr_isize %d of nid %llu is not supported yet",
> > > > + inode->xattr_isize, inode->nid | 0ULL);
> > > > + ret = -EFSCORRUPTED;
> > > > + goto out;
> > > > + } else if (inode->xattr_isize < xattr_hdr_size) {
> > > > + if (inode->xattr_isize) {
> > > > + erofs_err("bogus xattr ibody @ nid %llu",
> > > > + inode->nid | 0ULL);
> > > > + ret = -EFSCORRUPTED;
> > > > + goto out;
> > > > + }
> > > > + }
> > > > +
> > > > + addr = iloc(inode->nid) + inode->inode_isize;
> > > > + ret = dev_read(buf, addr, xattr_hdr_size);
> > > > + if (ret < 0) {
> > > > + erofs_err("an error occurred when reading xattr header "
> > > > + "of nid(%llu): errno(%d)", inode->nid | 0ULL, ret);
> > > > + goto out;
> > > > + }
> > > > + ih = (struct erofs_xattr_ibody_header *)buf;
> > > > + xattr_shared_count = ih->h_shared_count;
> > > > +
> > > > + ofs = erofs_blkoff(addr) + xattr_hdr_size;
> > > > + addr += xattr_hdr_size;
> > > > + remaining -= xattr_hdr_size;
> > > > + for (i = 0; i < xattr_shared_count; ++i) {
> > > > + if (ofs >= EROFS_BLKSIZ) {
> > > > + if (ofs != EROFS_BLKSIZ) {
> > > > + erofs_err("unaligned xattr entry in "
> > > > + "xattr shared area of nid(%llu)",
> > > > + inode->nid | 0ULL);
> > > > + ret = -EFSCORRUPTED;
> > >
> > > IMO, it won't happen due to proper alignment.. but I'm fine as this too.
> > >
> > > Thanks,
> > > Gao Xiang
> > >
> > > > + goto out;
> > > > + }
> > > > + ofs = 0;
> > > > + }
> > > > + ofs += xattr_entry_size;
> > > > + addr += xattr_entry_size;
> > > > + remaining -= xattr_entry_size;
> > > > + }
> > > > +
> > > > + while (remaining > 0) {
> > > > + unsigned int entry_sz;
> > > > +
> > > > + ret = dev_read(buf, addr, xattr_entry_size);
> > > > + if (ret) {
> > > > + erofs_err("an error occurred when reading xattr entry "
> > > > + "of nid(%llu): errno(%d)",
> > > > + inode->nid | 0ULL, ret);
> > > > + goto out;
> > > > + }
> > > > +
> > > > + entry = (struct erofs_xattr_entry *)buf;
> > > > + entry_sz = erofs_xattr_entry_size(entry);
> > > > + if (remaining < entry_sz) {
> > > > + erofs_err("xattr on-disk corruption: xattr entry "
> > > > + "beyond xattr_isize of nid(%llu)",
> > > > + inode->nid | 0ULL);
> > > > + ret = -EFSCORRUPTED;
> > > > + goto out;
> > > > + }
> > > > + addr += entry_sz;
> > > > + remaining -= entry_sz;
> > > > + }
> > > > +out:
> > > > + free(buf);
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int erofs_verify_data_chunk(struct erofs_inode *inode)
> > > > +{
> > > > + int ret;
> > > > +
> > > > + erofs_dbg("verify data chunk of nid(%llu): type(%d)",
> > > > + inode->nid | 0ULL, inode->datalayout);
> > > > +
> > > > + switch (inode->datalayout) {
> > > > + case EROFS_INODE_FLAT_PLAIN:
> > > > + case EROFS_INODE_FLAT_INLINE:
> > > > + case EROFS_INODE_CHUNK_BASED:
> > > > + ret = verify_raw_data_chunk(inode);
> > > > + break;
> > > > + case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
> > > > + case EROFS_INODE_FLAT_COMPRESSION:
> > > > + ret = verify_compressed_chunk(inode);
> > > > + break;
> > > > + default:
> > > > + ret = -EINVAL;
> > > > + break;
> > > > + }
> > > > +
> > > > + if (ret == -EIO)
> > > > + erofs_err("I/O error occurred when verifying "
> > > > + "data chunk of nid(%llu)", inode->nid | 0ULL);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid)
> > > > +{
> > > > + int ret;
> > > > + struct erofs_inode *inode;
> > > > + char *buf;
> > > > + erofs_off_t offset;
> > > > +
> > > > + erofs_dbg("check inode: nid(%llu)", nid | 0ULL);
> > > > + inode = calloc(1, sizeof(struct erofs_inode));
> > > > + BUG_ON(!inode);
> > > > + buf = calloc(EROFS_BLKSIZ, 1);
> > > > + BUG_ON(!buf);
> > > > +
> > > > + inode->nid = nid;
> > > > + ret = erofs_read_inode_from_disk(inode);
> > > > + if (ret) {
> > > > + if (ret == -EIO)
> > > > + erofs_err("I/O error occurred when reading nid(%llu)",
> > > > + nid | 0ULL);
> > > > + goto out;
> > > > + }
> > > > +
> > > > + /* verify xattr field */
> > > > + ret = erofs_verify_xattr(inode);
> > > > + if (ret)
> > > > + goto out;
> > > > +
> > > > + /* verify data chunk layout */
> > > > + ret = erofs_verify_data_chunk(inode);
> > > > + if (ret)
> > > > + goto out;
> > > > +
> > > > + if ((inode->i_mode & S_IFMT) != S_IFDIR)
> > > > + goto out;
> > > > +
> > > > + offset = 0;
> > > > + while (offset < inode->i_size) {
> > > > + erofs_off_t maxsize = min_t(erofs_off_t,
> > > > + inode->i_size - offset, EROFS_BLKSIZ);
> > > > + struct erofs_dirent *de = (void *)buf;
> > > > + unsigned int nameoff;
> > > > +
> > > > + ret = erofs_pread(inode, buf, maxsize, offset);
> > > > + if (ret) {
> > > > + erofs_err("I/O error occurred when reading dirents: "
> > > > + "nid(%llu), offset(%llu), size(%llu)",
> > > > + nid | 0ULL, offset | 0ULL, maxsize | 0ULL);
> > > > + goto out;
> > > > + }
> > > > +
> > > > + nameoff = le16_to_cpu(de->nameoff);
> > > > + if (nameoff < sizeof(struct erofs_dirent) ||
> > > > + nameoff >= PAGE_SIZE) {
> > > > + erofs_err("invalid de[0].nameoff %u @ nid(%llu), "
> > > > + "offset(%llu)",
> > > > + nameoff, nid | 0ULL, offset | 0ULL);
> > > > + ret = -EFSCORRUPTED;
> > > > + goto out;
> > > > + }
> > > > +
> > > > + ret = traverse_dirents(pnid, nid, buf, offset,
> > > > + nameoff, maxsize);
> > > > + if (ret)
> > > > + goto out;
> > > > +
> > > > + offset += maxsize;
> > > > + }
> > > > +out:
> > > > + free(buf);
> > > > + free(inode);
> > > > + if (ret && ret != -EIO)
> > > > + fsckcfg.corrupted = true;
> > > > +}
> > > > +
> > > > +int main(int argc, char **argv)
> > > > +{
> > > > + int err;
> > > > +
> > > > + erofs_init_configure();
> > > > +
> > > > + fsckcfg.corrupted = false;
> > > > + fsckcfg.print_comp_ratio = NO_PRINT_COMP_RATIO;
> > > > + fsckcfg.logical_len = 0;
> > > > + fsckcfg.ondisk_len = 0;
> > > > +
> > > > + err = erofsfsck_parse_options_cfg(argc, argv);
> > > > + if (err) {
> > > > + if (err == -EINVAL)
> > > > + usage();
> > > > + goto exit;
> > > > + }
> > > > +
> > > > + err = dev_open_ro(cfg.c_img_path);
> > > > + if (err) {
> > > > + erofs_err("failed to open image file");
> > > > + goto exit;
> > > > + }
> > > > +
> > > > + err = erofs_read_superblock();
> > > > + if (err) {
> > > > + erofs_err("failed to read superblock");
> > > > + goto exit;
> > > > + }
> > > > +
> > > > + if (erofs_sb_has_sb_chksum() && erofs_check_sb_chksum()) {
> > > > + erofs_err("failed to verify superblock checksum");
> > > > + goto exit;
> > > > + }
> > > > +
> > > > + erofs_check_inode(sbi.root_nid, sbi.root_nid);
> > > > +
> > > > + if (fsckcfg.corrupted) {
> > > > + fprintf(stderr, "Found some filesystem corruption\n");
> > > > + } else {
> > > > + fprintf(stderr, "No error found\n");
> > > > + if (fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO) {
> > > > + double comp_ratio = (double)fsckcfg.ondisk_len * 100 /
> > > > + (double)fsckcfg.logical_len;
> > > > + fprintf(stderr, "Compression Ratio: %.2f(%%)\n",
> > > > + comp_ratio);
> > > > + }
> > > > + }
> > > > +
> > > > +exit:
> > > > + erofs_exit_configure();
> > > > + return err;
> > > > +}
> > > > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > > > index 8b154ed..80065b2 100644
> > > > --- a/include/erofs/internal.h
> > > > +++ b/include/erofs/internal.h
> > > > @@ -82,6 +82,8 @@ struct erofs_sb_info {
> > > >
> > > > u16 available_compr_algs;
> > > > u16 lz4_max_distance;
> > > > +
> > > > + u32 checksum;
> > > > };
> > > >
> > > > /* global sbi */
> > > > @@ -264,10 +266,13 @@ int erofs_read_superblock(void);
> > > >
> > > > /* namei.c */
> > > > int erofs_ilookup(const char *path, struct erofs_inode *vi);
> > > > +int erofs_read_inode_from_disk(struct erofs_inode *vi);
> > > >
> > > > /* data.c */
> > > > int erofs_pread(struct erofs_inode *inode, char *buf,
> > > > erofs_off_t count, erofs_off_t offset);
> > > > +int erofs_map_blocks(struct erofs_inode *inode,
> > > > + struct erofs_map_blocks *map, int flags);
> > > > /* zmap.c */
> > > > int z_erofs_fill_inode(struct erofs_inode *vi);
> > > > int z_erofs_map_blocks_iter(struct erofs_inode *vi,
> > > > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > > > index 66a68e3..62e9981 100644
> > > > --- a/include/erofs_fs.h
> > > > +++ b/include/erofs_fs.h
> > > > @@ -400,4 +400,17 @@ static inline void erofs_check_ondisk_layout_definitions(void)
> > > > Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
> > > > }
> > > >
> > > > +#define CRC32C_POLY_LE 0x82F63B78
> > > > +static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
> > > > +{
> > > > + int i;
> > > > +
> > > > + while (len--) {
> > > > + crc ^= *in++;
> > > > + for (i = 0; i < 8; i++)
> > > > + crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
> > > > + }
> > > > + return crc;
> > > > +}
> > > > +
> > > > #endif
> > > > diff --git a/lib/data.c b/lib/data.c
> > > > index 641d840..6cb7eeb 100644
> > > > --- a/lib/data.c
> > > > +++ b/lib/data.c
> > > > @@ -61,8 +61,8 @@ err_out:
> > > > return err;
> > > > }
> > > >
> > > > -static int erofs_map_blocks(struct erofs_inode *inode,
> > > > - struct erofs_map_blocks *map, int flags)
> > > > +int erofs_map_blocks(struct erofs_inode *inode,
> > > > + struct erofs_map_blocks *map, int flags)
> > > > {
> > > > struct erofs_inode *vi = inode;
> > > > struct erofs_inode_chunk_index *idx;
> > > > diff --git a/lib/namei.c b/lib/namei.c
> > > > index b4bdabf..56f199a 100644
> > > > --- a/lib/namei.c
> > > > +++ b/lib/namei.c
> > > > @@ -22,7 +22,7 @@ static dev_t erofs_new_decode_dev(u32 dev)
> > > > return makedev(major, minor);
> > > > }
> > > >
> > > > -static int erofs_read_inode_from_disk(struct erofs_inode *vi)
> > > > +int erofs_read_inode_from_disk(struct erofs_inode *vi)
> > > > {
> > > > int ret, ifmt;
> > > > char buf[sizeof(struct erofs_inode_extended)];
> > > > diff --git a/lib/super.c b/lib/super.c
> > > > index 0fa69ab..0c30403 100644
> > > > --- a/lib/super.c
> > > > +++ b/lib/super.c
> > > > @@ -62,6 +62,7 @@ int erofs_read_superblock(void)
> > > > sbi.islotbits = EROFS_ISLOTBITS;
> > > > sbi.root_nid = le16_to_cpu(dsb->root_nid);
> > > > sbi.inos = le64_to_cpu(dsb->inos);
> > > > + sbi.checksum = le32_to_cpu(dsb->checksum);
> > > >
> > > > sbi.build_time = le64_to_cpu(dsb->build_time);
> > > > sbi.build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
> > > > diff --git a/mkfs/main.c b/mkfs/main.c
> > > > index 1c8dea5..b9b46f5 100644
> > > > --- a/mkfs/main.c
> > > > +++ b/mkfs/main.c
> > > > @@ -424,19 +424,6 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
> > > > return 0;
> > > > }
> > > >
> > > > -#define CRC32C_POLY_LE 0x82F63B78
> > > > -static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
> > > > -{
> > > > - int i;
> > > > -
> > > > - while (len--) {
> > > > - crc ^= *in++;
> > > > - for (i = 0; i < 8; i++)
> > > > - crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
> > > > - }
> > > > - return crc;
> > > > -}
> > > > -
> > > > static int erofs_mkfs_superblock_csum_set(void)
> > > > {
> > > > int ret;
> > > > --
> > > > 2.33.0.1079.g6e70778dc9-goog
> > > >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] erofs-utils: introduce fsck.erofs
2021-10-28 0:50 ` Daeho Jeong
@ 2021-10-28 1:00 ` Gao Xiang
2021-10-28 17:10 ` Daeho Jeong
0 siblings, 1 reply; 13+ messages in thread
From: Gao Xiang @ 2021-10-28 1:00 UTC (permalink / raw)
To: Daeho Jeong; +Cc: Daeho Jeong, linux-erofs, Jaegeuk Kim, miaoxie, Wang Qi
On Wed, Oct 27, 2021 at 05:50:22PM -0700, Daeho Jeong wrote:
> Hi Gao,
>
> On Tue, Oct 26, 2021 at 8:14 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> >
> > Hi Daeho,
> >
> > On Tue, Oct 26, 2021 at 11:35:44AM -0700, Daeho Jeong wrote:
> > > > Many thanks for the patch! I also think some fsck feature is useful in
> > > > order to check the image integration.
> > > >
> > > > IMO, Compression ratio calculation is more like a dump fs feature.
> > > > Wang Qi developed the erofsdump yet he told me that he is working for
> > > > another urgent stuffs for now so he delayed the work:
> > > > https://lore.kernel.org/r/20210915093537.2579575-1-guoxuenan@huawei.com
> > >
> > > Right, it's better to move these feature to dump.erofs.
> >
> > I'm fine to add "Compression ratio calculation" temporarily in
> > fsck.erofs as long as active development.
> >
> > Let's see if fsck feature can match erofs-utils 1.4 timepoint.
>
> How is the timeline for erofs-utils 1.4?
I roughly release a new utils version every 6 months.
For v1.4, I'd like to release it after 5.16-rc1 is out (between
5.16-rc1 ~ 5.16-rc2...)
>
> >
> > >
> > > >
> > > > fsck and dump have the similar logic (and like f2fs-tools, we can also
> > > > have one program for this 2 features). If fsck is actively developed,
> > > > I'm pretty fine to use the fsck codebase and merge erofsdump into it
> > > > later instead.
> > >
> > > Like f2fs tools, I think we can maintain two tools separately.
> >
> > Yeah, my point is two tools share one common logic in the final shape,
> > but I'm fine with the current status as well :)
> >
> > > But, if you want, we can merge them into one tool.
> > >
> > > > Some comments as below:
> > > >
> > > > > ---
> > > > > Makefile.am | 2 +-
> > > > > configure.ac | 3 +-
> > > > > fsck/Makefile.am | 9 +
> > > > > fsck/main.c | 548 +++++++++++++++++++++++++++++++++++++++
> > > > > include/erofs/internal.h | 5 +
> > > > > include/erofs_fs.h | 13 +
> > > > > lib/data.c | 4 +-
> > > > > lib/namei.c | 2 +-
> > > > > lib/super.c | 1 +
> > > > > mkfs/main.c | 13 -
> > > > > 10 files changed, 582 insertions(+), 18 deletions(-)
> > > > > create mode 100644 fsck/Makefile.am
> > > > > create mode 100644 fsck/main.c
> > > > >
> > > > > diff --git a/Makefile.am b/Makefile.am
> > > > > index 24e1d38..fc464e8 100644
> > > > > --- a/Makefile.am
> > > > > +++ b/Makefile.am
> > > > > @@ -2,7 +2,7 @@
> > > > >
> > > > > ACLOCAL_AMFLAGS = -I m4
> > > > >
> > > > > -SUBDIRS = man lib mkfs dump
> > > > > +SUBDIRS = man lib mkfs dump fsck
> > > > > if ENABLE_FUSE
> > > > > SUBDIRS += fuse
> > > > > endif
> > > > > diff --git a/configure.ac b/configure.ac
> > > > > index b2c3225..5698b2e 100644
> > > > > --- a/configure.ac
> > > > > +++ b/configure.ac
> > > > > @@ -298,5 +298,6 @@ AC_CONFIG_FILES([Makefile
> > > > > lib/Makefile
> > > > > mkfs/Makefile
> > > > > dump/Makefile
> > > > > - fuse/Makefile])
> > > > > + fuse/Makefile
> > > > > + fsck/Makefile])
> > > > > AC_OUTPUT
> > > > > diff --git a/fsck/Makefile.am b/fsck/Makefile.am
> > > > > new file mode 100644
> > > > > index 0000000..82973ba
> > > > > --- /dev/null
> > > > > +++ b/fsck/Makefile.am
> > > > > @@ -0,0 +1,9 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0+
> > > > > +# Makefile.am
> > > > > +
> > > > > +AUTOMAKE_OPTIONS = foreign
> > > > > +bin_PROGRAMS = fsck.erofs
> > > > > +AM_CPPFLAGS = ${libuuid_CFLAGS}
> > > > > +fsck_erofs_SOURCES = main.c
> > > > > +fsck_erofs_CFLAGS = -Wall -Werror -I$(top_srcdir)/include
> > > > > +fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libuuid_LIBS} ${liblz4_LIBS}
> > > > > diff --git a/fsck/main.c b/fsck/main.c
> > > > > new file mode 100644
> > > > > index 0000000..c397d19
> > > > > --- /dev/null
> > > > > +++ b/fsck/main.c
> > > > > @@ -0,0 +1,548 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > +/*
> > > > > + * Copyright 2021 Google LLC
> > > > > + * Author: Daeho Jeong <daehojeong@google.com>
> > > > > + */
> > > > > +#include <stdlib.h>
> > > > > +#include <getopt.h>
> > > > > +#include <time.h>
> > > > > +#include "erofs/print.h"
> > > > > +#include "erofs/io.h"
> > > > > +
> > > > > +enum {
> > > > > + NO_PRINT_COMP_RATIO = 0,
> > > > > + PRINT_COMP_RATIO_ALL = 1,
> > > > > + PRINT_COMP_RATIO_COMP_FILE = 2,
> > > > > +};
> > > > > +
> > > > > +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
> > > > > +
> > > > > +struct erofsfsck_cfg {
> > > > > + bool corrupted;
> > > > > + int print_comp_ratio;
> > > > > + u64 ondisk_len;
> > > > > + u64 logical_len;
> > > > > +};
> > > > > +static struct erofsfsck_cfg fsckcfg;
> > > > > +
> > > > > +static struct option long_options[] = {
> > > > > + {"help", no_argument, 0, 1},
> > > > > + {0, 0, 0, 0},
> > > > > +};
> > > > > +
> > > > > +static void usage(void)
> > > > > +{
> > > > > + fputs("usage: [options] IMAGE\n\n"
> > > > > + "Check erofs filesystem integrity of IMAGE, and [options] are:\n"
> > > > > + " -V print the version number of fsck.erofs and exit.\n"
> > > > > + " -d# set output message level to # (maximum 9)\n"
> > > > > + " -c print total compression ratio of all compressed files\n"
> > > > > + " -C print total compression ratio of all files\n"
> > > >
> > > > Apart from erofsdump consideration.. Can we merge these 2 options into one?
> > >
> > > Sure~
> > >
> > > >
> > > > > + " --help display this help and exit.\n",
> > > > > + stderr);
> > > > > +}
> > > > > +
> > > > > +static void erofsfsck_print_version(void)
> > > > > +{
> > > > > + fprintf(stderr, "fsck.erofs %s\n", cfg.c_version);
> > > > > +}
> > > > > +
> > > > > +static int erofsfsck_parse_options_cfg(int argc, char **argv)
> > > > > +{
> > > > > + int opt, i;
> > > > > +
> > > > > + while ((opt = getopt_long(argc, argv, "Vd:Cc",
> > > > > + long_options, NULL)) != -1) {
> > > > > + switch (opt) {
> > > > > + case 'V':
> > > > > + erofsfsck_print_version();
> > > > > + exit(0);
> > > > > + case 'd':
> > > > > + i = atoi(optarg);
> > > > > + if (i < EROFS_MSG_MIN || i > EROFS_MSG_MAX) {
> > > > > + erofs_err("invalid debug level %d", i);
> > > > > + return -EINVAL;
> > > > > + }
> > > > > + cfg.c_dbg_lvl = i;
> > > > > + break;
> > > > > + case 'C':
> > > > > + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_ALL;
> > > > > + break;
> > > > > + case 'c':
> > > > > + fsckcfg.print_comp_ratio = PRINT_COMP_RATIO_COMP_FILE;
> > > > > + break;
> > > > > + case 1:
> > > > > + usage();
> > > > > + exit(0);
> > > > > + default:
> > > > > + return -EINVAL;
> > > > > + }
> > > > > + }
> > > > > +
> > > > > + if (optind >= argc)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + cfg.c_img_path = strdup(argv[optind++]);
> > > > > + if (!cfg.c_img_path)
> > > > > + return -ENOMEM;
> > > > > +
> > > > > + if (optind < argc) {
> > > > > + erofs_err("unexpected argument: %s\n", argv[optind]);
> > > > > + return -EINVAL;
> > > > > + }
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int erofs_check_sb_chksum(void)
> > > > > +{
> > > > > + int ret;
> > > > > + u8 buf[EROFS_BLKSIZ];
> > > > > + u32 crc;
> > > > > + struct erofs_super_block *sb;
> > > > > +
> > > > > + ret = blk_read(buf, 0, 1);
> > > > > + if (ret) {
> > > > > + erofs_err("failed to read superblock to check checksum: "
> > > > > + "errno(%d)", ret);
> > > > > + return -1;
> > > > > + }
> > > > > +
> > > > > + sb = (struct erofs_super_block *)(buf + EROFS_SUPER_OFFSET);
> > > > > + sb->checksum = 0;
> > > > > +
> > > > > + crc = crc32c(~0, (u8 *)sb, EROFS_BLKSIZ - EROFS_SUPER_OFFSET);
> > > > > + if (crc != sbi.checksum) {
> > > > > + erofs_err("superblock checksum doesn't match: saved(0x%08x) "
> > > > > + "calculated(0x%08x)", sbi.checksum, crc);
> > > > > + fsckcfg.corrupted = true;
> > > > > + return -1;
> > > > > + }
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int check_special_dentries(struct erofs_dirent *de, const char *de_name,
> > > > > + unsigned int de_namelen, erofs_nid_t nid,
> > > > > + erofs_nid_t pnid, bool is_curdir)
> > > > > +{
> > > > > + unsigned int dirname_len = is_curdir ? 1 : 2;
> > > > > + const char *dirname = is_curdir ? "." : "..";
> > > > > + erofs_nid_t correct_nid = is_curdir ? nid : pnid;
> > > > > +
> > > > > + if (de_namelen != dirname_len || memcmp(de_name, dirname, de_namelen)) {
> > > > > + char *dbgname = strndup(de_name, de_namelen);
> > > > > +
> > > > > + BUG_ON(!dbgname);
> > > > > + if (is_curdir)
> > > > > + erofs_err("wrong current dir name(%s) @ nid(%llu)",
> > > > > + dbgname, nid | 0ULL);
> > > > > + else
> > > > > + erofs_err("wrong parent dir name(%s) @ nid(%llu)",
> > > > > + dbgname, nid | 0ULL);
> > > > > + free(dbgname);
> > > > > + return -1;
> > > > > + }
> > > > > +
> > > > > + if (de->nid != correct_nid) {
> > > > > + if (is_curdir)
> > > > > + erofs_err("wrong current dir nid(%llu) @ nid(%llu)",
> > > > > + de->nid | 0ULL, nid | 0ULL);
> > > > > + else
> > > > > + erofs_err("wrong parent dir nid(%llu): "
> > > > > + "pnid(%llu) @ nid(%llu)",
> > > > > + de->nid | 0ULL, pnid | 0ULL, nid | 0ULL);
> > > > > + return -1;
> > > > > + }
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int traverse_dirents(erofs_nid_t pnid, erofs_nid_t nid,
> > > > > + void *dentry_blk, erofs_off_t offset,
> > > > > + unsigned int next_nameoff, unsigned int maxsize)
> > > >
> > > > Not quite sure if we could introduce some
> > > > erofs_foreach_dirent() and customized callbacks to cleanup similar functions
> > > > in fuse and fsck...
> > >
> > > Let me think about it more.
> >
> > That is not quite urgent for now, we can stay as this.
> >
> > >
> > > > > +{
> > > > > + struct erofs_dirent *de = dentry_blk;
> > > > > + const struct erofs_dirent *end = dentry_blk + next_nameoff;
> > > > > + unsigned int idx = 0;
> > > > > +
> > > > > + erofs_dbg("traversing pnid(%llu), nid(%llu)", pnid | 0ULL, nid | 0ULL);
> > > > > +
> > > > > + if (offset == 0 && (next_nameoff < 2 * sizeof(struct erofs_dirent))) {
> > > > > + erofs_err("too small dirents of size(%d) in nid(%llu)",
> > > > > + next_nameoff, nid | 0ULL);
> > > > > + return -EFSCORRUPTED;
> > > > > + }
> > > > > +
> > > > > + while (de < end) {
> > > > > + const char *de_name;
> > > > > + unsigned int de_namelen;
> > > > > + unsigned int nameoff;
> > > > > + char *dbgname;
> > > > > +
> > > > > + nameoff = le16_to_cpu(de->nameoff);
> > > > > + de_name = (char *)dentry_blk + nameoff;
> > > > > +
> > > > > + /* the last dirent check */
> > > > > + if (de + 1 >= end)
> > > > > + de_namelen = strnlen(de_name, maxsize - nameoff);
> > > > > + else
> > > > > + de_namelen = le16_to_cpu(de[1].nameoff) - nameoff;
> > > > > +
> > > > > + if (cfg.c_dbg_lvl >= EROFS_DBG) {
> > > > > + dbgname = strndup(de_name, de_namelen);
> > > > > + BUG_ON(!dbgname);
> > > > > + erofs_dbg("traversed filename(%s)", dbgname);
> > > > > + free(dbgname);
> > > > > + }
> > > > > +
> > > > > + /* corrupted entry check */
> > > > > + if (nameoff != next_nameoff) {
> > > > > + erofs_err("bogus dirent with nameoff(%u): expected(%d) "
> > > > > + "@ nid(%llu), offset(%llu), idx(%u)",
> > > > > + nameoff, next_nameoff, nid | 0ULL,
> > > > > + offset | 0ULL, idx);
> > > > > + return -EFSCORRUPTED;
> > > > > + }
> > > > > +
> > > > > + if (nameoff + de_namelen > maxsize ||
> > > > > + de_namelen > EROFS_NAME_LEN) {
> > > > > + erofs_err("bogus dirent with namelen(%u) @ nid(%llu), "
> > > > > + "offset(%llu), idx(%u)",
> > > > > + de_namelen, nid | 0ULL, offset | 0ULL, idx);
> > > > > + return -EFSCORRUPTED;
> > > > > + }
> > > > > +
> > > > > + if (offset == 0 && (idx == 0 || idx == 1)) {
> > > >
> > > > We may not assume "." and ".." special dirents as 0 or 1... Actually
> > > > all dirents are sorted in the alphabetical order. I'm not sure if
> > > > some dirents are smaller than "." and "..".
> > >
> > > Ok, I will remove this check here.
> >
> > Ok.
> >
> > >
> > > > We could also check if all dirents are in alphabetical order as well
> > > > since it's used to do binary search runtimely.
> > >
> > > I will add a check routine to check all dirents are in alphabetical order.
> >
> > Ok.
> >
> > >
> > > >
> > > > > + if (check_special_dentries(de, de_name, de_namelen, nid,
> > > > > + pnid, idx == 0))
> > > > > + return -EFSCORRUPTED;
> > > > > + } else {
> > > > > + erofs_check_inode(nid, de->nid);
> > > > > + if (fsckcfg.corrupted)
> > > > > + return -EFSCORRUPTED;
> > > > > + }
> > > > > +
> > > > > + next_nameoff += de_namelen;
> > > > > + ++de;
> > > > > + ++idx;
> > > > > + }
> > > > > +
> > > > > + erofs_dbg("traversing ... done nid(%llu)", nid | 0ULL);
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int verify_raw_data_chunk(struct erofs_inode *inode)
> > > > > +{
> > > > > + struct erofs_map_blocks map = {
> > > > > + .index = UINT_MAX,
> > > > > + };
> > > > > + int ret;
> > > > > + erofs_off_t ptr = 0;
> > > > > +
> > > > > + if (fsckcfg.print_comp_ratio == PRINT_COMP_RATIO_ALL) {
> > > > > + fsckcfg.logical_len += inode->i_size;
> > > > > + fsckcfg.ondisk_len += inode->i_size;
> > > > > + }
> > > > > +
> > > > > + while (ptr < inode->i_size) {
> > > > > + map.m_la = ptr;
> > > > > + ret = erofs_map_blocks(inode, &map, 0);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + if (map.m_plen != map.m_llen || ptr != map.m_la) {
> > > > > + erofs_err("broken data chunk layout m_la %" PRIu64
> > > > > + " ptr %" PRIu64 " m_llen %" PRIu64 " m_plen %"
> > > > > + PRIu64, map.m_la, ptr, map.m_llen,
> > > > > + map.m_plen);
> > > > > + return -EFSCORRUPTED;
> > > > > + }
> > > > > +
> > > > > + if (!(map.m_flags & EROFS_MAP_MAPPED) && !map.m_llen) {
> > > > > + /* readched EOF */
> > > > > + ptr = inode->i_size;
> > > > > + continue;
> > > > > + }
> > > > > +
> > > > > + ptr += map.m_llen;
> > > > > + }
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int verify_compressed_chunk(struct erofs_inode *inode)
> > > > > +{
> > > > > + struct erofs_map_blocks map = {
> > > > > + .index = UINT_MAX,
> > > > > + };
> > > > > + int ret = 0;
> > > > > + bool count_pchunk = fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO;
> > > > > + u64 pchunk_len = 0;
> > > > > + erofs_off_t offset = 0, end = inode->i_size;
> > > > > +
> > > > > + while (end > offset) {
> > > > > + map.m_la = end - 1;
> > > > > +
> > > > > + ret = z_erofs_map_blocks_iter(inode, &map);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + if (end > map.m_la + map.m_llen) {
> > > > > + erofs_err("broken compressed chunk layout m_la %" PRIu64
> > > > > + " m_llen %" PRIu64 " end %" PRIu64,
> > > > > + map.m_la, map.m_llen, end);
> > > > > + return -EFSCORRUPTED;
> > > > > + }
> > > >
> > > > IMO, A simple way to check this is to try to decompress by calling
> > > > z_erofs_decompress() with .partial_decoding == 0 and see if any
> > > > error when decompressing as well.
> > >
> > > Got it~
> >
> > Yeah, thanks for doing this again!
>
> In fact, I wanted to decompress the whole data here. We can't check
> the data integrity,
> so I just wanted to check the layout of the file and that is the
> reason why I used z_erofs_map_blocks_iter() directly.
Yeah, z_erofs_map_blocks_iter() here is good, yet I think we could
add a follow-up z_erofs_decompress() as well, at least it can verify
obvious compressed data corruption.
>
> >
> > Thanks,
> > Gao Xiang
> >
> > >
> > > > > +
> > > > > + if (count_pchunk)
> > > > > + pchunk_len += map.m_plen;
> > > > > +
> > > > > + end = map.m_la;
> > > > > + }
> > > > > +
> > > > > + if (count_pchunk) {
> > > > > + fsckcfg.logical_len += inode->i_size;
> > > > > + fsckcfg.ondisk_len += pchunk_len;
> > > >
> > > > we can also check another field called compressed_blocks in on-disk
> > > > inode...
> > >
> > > ditto
>
> When we use compressed_blocks, we need to calculate the ratio based on
> block count.
> Then, I need to make the i_size aligned to BLK_SIZE. Which one do you
> prefer, bytes or blocks?
I think it depends,
maybe for uncompressed files, if it's already tail-packing, we could use
i_size. Otherwise, i_blocks makes sense for me since it already takes
such blocks and easier to deal with.
Thanks,
Gao Xiang
>
> Thanks~!
>
> > >
> > >
> > > > > + }
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int erofs_verify_xattr(struct erofs_inode *inode)
> > > > > +{
> > > > > + unsigned int xattr_hdr_size = sizeof(struct erofs_xattr_ibody_header);
> > > > > + unsigned int xattr_entry_size = sizeof(struct erofs_xattr_entry);
> > > > > + erofs_off_t addr;
> > > > > + unsigned int ofs, xattr_shared_count;
> > > > > + struct erofs_xattr_ibody_header *ih;
> > > > > + struct erofs_xattr_entry *entry;
> > > > > + int i, remaining = inode->xattr_isize, ret = 0;
> > > > > + char *buf = calloc(EROFS_BLKSIZ, 1);
> > > > > +
> > > > > + BUG_ON(!buf);
> > > > > +
> > > > > + if (inode->xattr_isize == xattr_hdr_size) {
> > > > > + erofs_err("xattr_isize %d of nid %llu is not supported yet",
> > > > > + inode->xattr_isize, inode->nid | 0ULL);
> > > > > + ret = -EFSCORRUPTED;
> > > > > + goto out;
> > > > > + } else if (inode->xattr_isize < xattr_hdr_size) {
> > > > > + if (inode->xattr_isize) {
> > > > > + erofs_err("bogus xattr ibody @ nid %llu",
> > > > > + inode->nid | 0ULL);
> > > > > + ret = -EFSCORRUPTED;
> > > > > + goto out;
> > > > > + }
> > > > > + }
> > > > > +
> > > > > + addr = iloc(inode->nid) + inode->inode_isize;
> > > > > + ret = dev_read(buf, addr, xattr_hdr_size);
> > > > > + if (ret < 0) {
> > > > > + erofs_err("an error occurred when reading xattr header "
> > > > > + "of nid(%llu): errno(%d)", inode->nid | 0ULL, ret);
> > > > > + goto out;
> > > > > + }
> > > > > + ih = (struct erofs_xattr_ibody_header *)buf;
> > > > > + xattr_shared_count = ih->h_shared_count;
> > > > > +
> > > > > + ofs = erofs_blkoff(addr) + xattr_hdr_size;
> > > > > + addr += xattr_hdr_size;
> > > > > + remaining -= xattr_hdr_size;
> > > > > + for (i = 0; i < xattr_shared_count; ++i) {
> > > > > + if (ofs >= EROFS_BLKSIZ) {
> > > > > + if (ofs != EROFS_BLKSIZ) {
> > > > > + erofs_err("unaligned xattr entry in "
> > > > > + "xattr shared area of nid(%llu)",
> > > > > + inode->nid | 0ULL);
> > > > > + ret = -EFSCORRUPTED;
> > > >
> > > > IMO, it won't happen due to proper alignment.. but I'm fine as this too.
> > > >
> > > > Thanks,
> > > > Gao Xiang
> > > >
> > > > > + goto out;
> > > > > + }
> > > > > + ofs = 0;
> > > > > + }
> > > > > + ofs += xattr_entry_size;
> > > > > + addr += xattr_entry_size;
> > > > > + remaining -= xattr_entry_size;
> > > > > + }
> > > > > +
> > > > > + while (remaining > 0) {
> > > > > + unsigned int entry_sz;
> > > > > +
> > > > > + ret = dev_read(buf, addr, xattr_entry_size);
> > > > > + if (ret) {
> > > > > + erofs_err("an error occurred when reading xattr entry "
> > > > > + "of nid(%llu): errno(%d)",
> > > > > + inode->nid | 0ULL, ret);
> > > > > + goto out;
> > > > > + }
> > > > > +
> > > > > + entry = (struct erofs_xattr_entry *)buf;
> > > > > + entry_sz = erofs_xattr_entry_size(entry);
> > > > > + if (remaining < entry_sz) {
> > > > > + erofs_err("xattr on-disk corruption: xattr entry "
> > > > > + "beyond xattr_isize of nid(%llu)",
> > > > > + inode->nid | 0ULL);
> > > > > + ret = -EFSCORRUPTED;
> > > > > + goto out;
> > > > > + }
> > > > > + addr += entry_sz;
> > > > > + remaining -= entry_sz;
> > > > > + }
> > > > > +out:
> > > > > + free(buf);
> > > > > + return ret;
> > > > > +}
> > > > > +
> > > > > +static int erofs_verify_data_chunk(struct erofs_inode *inode)
> > > > > +{
> > > > > + int ret;
> > > > > +
> > > > > + erofs_dbg("verify data chunk of nid(%llu): type(%d)",
> > > > > + inode->nid | 0ULL, inode->datalayout);
> > > > > +
> > > > > + switch (inode->datalayout) {
> > > > > + case EROFS_INODE_FLAT_PLAIN:
> > > > > + case EROFS_INODE_FLAT_INLINE:
> > > > > + case EROFS_INODE_CHUNK_BASED:
> > > > > + ret = verify_raw_data_chunk(inode);
> > > > > + break;
> > > > > + case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
> > > > > + case EROFS_INODE_FLAT_COMPRESSION:
> > > > > + ret = verify_compressed_chunk(inode);
> > > > > + break;
> > > > > + default:
> > > > > + ret = -EINVAL;
> > > > > + break;
> > > > > + }
> > > > > +
> > > > > + if (ret == -EIO)
> > > > > + erofs_err("I/O error occurred when verifying "
> > > > > + "data chunk of nid(%llu)", inode->nid | 0ULL);
> > > > > +
> > > > > + return ret;
> > > > > +}
> > > > > +
> > > > > +static void erofs_check_inode(erofs_nid_t pnid, erofs_nid_t nid)
> > > > > +{
> > > > > + int ret;
> > > > > + struct erofs_inode *inode;
> > > > > + char *buf;
> > > > > + erofs_off_t offset;
> > > > > +
> > > > > + erofs_dbg("check inode: nid(%llu)", nid | 0ULL);
> > > > > + inode = calloc(1, sizeof(struct erofs_inode));
> > > > > + BUG_ON(!inode);
> > > > > + buf = calloc(EROFS_BLKSIZ, 1);
> > > > > + BUG_ON(!buf);
> > > > > +
> > > > > + inode->nid = nid;
> > > > > + ret = erofs_read_inode_from_disk(inode);
> > > > > + if (ret) {
> > > > > + if (ret == -EIO)
> > > > > + erofs_err("I/O error occurred when reading nid(%llu)",
> > > > > + nid | 0ULL);
> > > > > + goto out;
> > > > > + }
> > > > > +
> > > > > + /* verify xattr field */
> > > > > + ret = erofs_verify_xattr(inode);
> > > > > + if (ret)
> > > > > + goto out;
> > > > > +
> > > > > + /* verify data chunk layout */
> > > > > + ret = erofs_verify_data_chunk(inode);
> > > > > + if (ret)
> > > > > + goto out;
> > > > > +
> > > > > + if ((inode->i_mode & S_IFMT) != S_IFDIR)
> > > > > + goto out;
> > > > > +
> > > > > + offset = 0;
> > > > > + while (offset < inode->i_size) {
> > > > > + erofs_off_t maxsize = min_t(erofs_off_t,
> > > > > + inode->i_size - offset, EROFS_BLKSIZ);
> > > > > + struct erofs_dirent *de = (void *)buf;
> > > > > + unsigned int nameoff;
> > > > > +
> > > > > + ret = erofs_pread(inode, buf, maxsize, offset);
> > > > > + if (ret) {
> > > > > + erofs_err("I/O error occurred when reading dirents: "
> > > > > + "nid(%llu), offset(%llu), size(%llu)",
> > > > > + nid | 0ULL, offset | 0ULL, maxsize | 0ULL);
> > > > > + goto out;
> > > > > + }
> > > > > +
> > > > > + nameoff = le16_to_cpu(de->nameoff);
> > > > > + if (nameoff < sizeof(struct erofs_dirent) ||
> > > > > + nameoff >= PAGE_SIZE) {
> > > > > + erofs_err("invalid de[0].nameoff %u @ nid(%llu), "
> > > > > + "offset(%llu)",
> > > > > + nameoff, nid | 0ULL, offset | 0ULL);
> > > > > + ret = -EFSCORRUPTED;
> > > > > + goto out;
> > > > > + }
> > > > > +
> > > > > + ret = traverse_dirents(pnid, nid, buf, offset,
> > > > > + nameoff, maxsize);
> > > > > + if (ret)
> > > > > + goto out;
> > > > > +
> > > > > + offset += maxsize;
> > > > > + }
> > > > > +out:
> > > > > + free(buf);
> > > > > + free(inode);
> > > > > + if (ret && ret != -EIO)
> > > > > + fsckcfg.corrupted = true;
> > > > > +}
> > > > > +
> > > > > +int main(int argc, char **argv)
> > > > > +{
> > > > > + int err;
> > > > > +
> > > > > + erofs_init_configure();
> > > > > +
> > > > > + fsckcfg.corrupted = false;
> > > > > + fsckcfg.print_comp_ratio = NO_PRINT_COMP_RATIO;
> > > > > + fsckcfg.logical_len = 0;
> > > > > + fsckcfg.ondisk_len = 0;
> > > > > +
> > > > > + err = erofsfsck_parse_options_cfg(argc, argv);
> > > > > + if (err) {
> > > > > + if (err == -EINVAL)
> > > > > + usage();
> > > > > + goto exit;
> > > > > + }
> > > > > +
> > > > > + err = dev_open_ro(cfg.c_img_path);
> > > > > + if (err) {
> > > > > + erofs_err("failed to open image file");
> > > > > + goto exit;
> > > > > + }
> > > > > +
> > > > > + err = erofs_read_superblock();
> > > > > + if (err) {
> > > > > + erofs_err("failed to read superblock");
> > > > > + goto exit;
> > > > > + }
> > > > > +
> > > > > + if (erofs_sb_has_sb_chksum() && erofs_check_sb_chksum()) {
> > > > > + erofs_err("failed to verify superblock checksum");
> > > > > + goto exit;
> > > > > + }
> > > > > +
> > > > > + erofs_check_inode(sbi.root_nid, sbi.root_nid);
> > > > > +
> > > > > + if (fsckcfg.corrupted) {
> > > > > + fprintf(stderr, "Found some filesystem corruption\n");
> > > > > + } else {
> > > > > + fprintf(stderr, "No error found\n");
> > > > > + if (fsckcfg.print_comp_ratio != NO_PRINT_COMP_RATIO) {
> > > > > + double comp_ratio = (double)fsckcfg.ondisk_len * 100 /
> > > > > + (double)fsckcfg.logical_len;
> > > > > + fprintf(stderr, "Compression Ratio: %.2f(%%)\n",
> > > > > + comp_ratio);
> > > > > + }
> > > > > + }
> > > > > +
> > > > > +exit:
> > > > > + erofs_exit_configure();
> > > > > + return err;
> > > > > +}
> > > > > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > > > > index 8b154ed..80065b2 100644
> > > > > --- a/include/erofs/internal.h
> > > > > +++ b/include/erofs/internal.h
> > > > > @@ -82,6 +82,8 @@ struct erofs_sb_info {
> > > > >
> > > > > u16 available_compr_algs;
> > > > > u16 lz4_max_distance;
> > > > > +
> > > > > + u32 checksum;
> > > > > };
> > > > >
> > > > > /* global sbi */
> > > > > @@ -264,10 +266,13 @@ int erofs_read_superblock(void);
> > > > >
> > > > > /* namei.c */
> > > > > int erofs_ilookup(const char *path, struct erofs_inode *vi);
> > > > > +int erofs_read_inode_from_disk(struct erofs_inode *vi);
> > > > >
> > > > > /* data.c */
> > > > > int erofs_pread(struct erofs_inode *inode, char *buf,
> > > > > erofs_off_t count, erofs_off_t offset);
> > > > > +int erofs_map_blocks(struct erofs_inode *inode,
> > > > > + struct erofs_map_blocks *map, int flags);
> > > > > /* zmap.c */
> > > > > int z_erofs_fill_inode(struct erofs_inode *vi);
> > > > > int z_erofs_map_blocks_iter(struct erofs_inode *vi,
> > > > > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > > > > index 66a68e3..62e9981 100644
> > > > > --- a/include/erofs_fs.h
> > > > > +++ b/include/erofs_fs.h
> > > > > @@ -400,4 +400,17 @@ static inline void erofs_check_ondisk_layout_definitions(void)
> > > > > Z_EROFS_VLE_CLUSTER_TYPE_MAX - 1);
> > > > > }
> > > > >
> > > > > +#define CRC32C_POLY_LE 0x82F63B78
> > > > > +static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
> > > > > +{
> > > > > + int i;
> > > > > +
> > > > > + while (len--) {
> > > > > + crc ^= *in++;
> > > > > + for (i = 0; i < 8; i++)
> > > > > + crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
> > > > > + }
> > > > > + return crc;
> > > > > +}
> > > > > +
> > > > > #endif
> > > > > diff --git a/lib/data.c b/lib/data.c
> > > > > index 641d840..6cb7eeb 100644
> > > > > --- a/lib/data.c
> > > > > +++ b/lib/data.c
> > > > > @@ -61,8 +61,8 @@ err_out:
> > > > > return err;
> > > > > }
> > > > >
> > > > > -static int erofs_map_blocks(struct erofs_inode *inode,
> > > > > - struct erofs_map_blocks *map, int flags)
> > > > > +int erofs_map_blocks(struct erofs_inode *inode,
> > > > > + struct erofs_map_blocks *map, int flags)
> > > > > {
> > > > > struct erofs_inode *vi = inode;
> > > > > struct erofs_inode_chunk_index *idx;
> > > > > diff --git a/lib/namei.c b/lib/namei.c
> > > > > index b4bdabf..56f199a 100644
> > > > > --- a/lib/namei.c
> > > > > +++ b/lib/namei.c
> > > > > @@ -22,7 +22,7 @@ static dev_t erofs_new_decode_dev(u32 dev)
> > > > > return makedev(major, minor);
> > > > > }
> > > > >
> > > > > -static int erofs_read_inode_from_disk(struct erofs_inode *vi)
> > > > > +int erofs_read_inode_from_disk(struct erofs_inode *vi)
> > > > > {
> > > > > int ret, ifmt;
> > > > > char buf[sizeof(struct erofs_inode_extended)];
> > > > > diff --git a/lib/super.c b/lib/super.c
> > > > > index 0fa69ab..0c30403 100644
> > > > > --- a/lib/super.c
> > > > > +++ b/lib/super.c
> > > > > @@ -62,6 +62,7 @@ int erofs_read_superblock(void)
> > > > > sbi.islotbits = EROFS_ISLOTBITS;
> > > > > sbi.root_nid = le16_to_cpu(dsb->root_nid);
> > > > > sbi.inos = le64_to_cpu(dsb->inos);
> > > > > + sbi.checksum = le32_to_cpu(dsb->checksum);
> > > > >
> > > > > sbi.build_time = le64_to_cpu(dsb->build_time);
> > > > > sbi.build_time_nsec = le32_to_cpu(dsb->build_time_nsec);
> > > > > diff --git a/mkfs/main.c b/mkfs/main.c
> > > > > index 1c8dea5..b9b46f5 100644
> > > > > --- a/mkfs/main.c
> > > > > +++ b/mkfs/main.c
> > > > > @@ -424,19 +424,6 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
> > > > > return 0;
> > > > > }
> > > > >
> > > > > -#define CRC32C_POLY_LE 0x82F63B78
> > > > > -static inline u32 crc32c(u32 crc, const u8 *in, size_t len)
> > > > > -{
> > > > > - int i;
> > > > > -
> > > > > - while (len--) {
> > > > > - crc ^= *in++;
> > > > > - for (i = 0; i < 8; i++)
> > > > > - crc = (crc >> 1) ^ ((crc & 1) ? CRC32C_POLY_LE : 0);
> > > > > - }
> > > > > - return crc;
> > > > > -}
> > > > > -
> > > > > static int erofs_mkfs_superblock_csum_set(void)
> > > > > {
> > > > > int ret;
> > > > > --
> > > > > 2.33.0.1079.g6e70778dc9-goog
> > > > >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] erofs-utils: introduce fsck.erofs
2021-10-28 1:00 ` Gao Xiang
@ 2021-10-28 17:10 ` Daeho Jeong
2021-10-28 22:20 ` Gao Xiang
0 siblings, 1 reply; 13+ messages in thread
From: Daeho Jeong @ 2021-10-28 17:10 UTC (permalink / raw)
To: Gao Xiang; +Cc: Daeho Jeong, linux-erofs, Jaegeuk Kim, miaoxie, Wang Qi
> > In fact, I wanted to decompress the whole data here. We can't check
> > the data integrity,
> > so I just wanted to check the layout of the file and that is the
> > reason why I used z_erofs_map_blocks_iter() directly.
>
> Yeah, z_erofs_map_blocks_iter() here is good, yet I think we could
> add a follow-up z_erofs_decompress() as well, at least it can verify
> obvious compressed data corruption.
Could you enlighten me what is wrong with the below flow?
z_erofs_decompress fails with -EIO or -EUCLEAN.
raw = malloc(pchunk_len);
BUG_ON(!raw);
buffer = malloc(inode->i_size);
BUG_ON(!buffer);
ret = dev_read(raw, 0, pchunk_len);
if (ret < 0) {
erofs_err("an error occurred when reading compressed data "
"of nid(%llu): errno(%d)", inode->nid | 0ULL, ret);
goto out;
}
ret = z_erofs_decompress(&(struct z_erofs_decompress_req) {
.in = raw,
.out = buffer,
.decodedskip = 0,
.inputsize = pchunk_len,
.decodedlength = inode->i_size,
.alg = algorithmformat,
.partial_decoding = 0
});
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] erofs-utils: introduce fsck.erofs
2021-10-28 17:10 ` Daeho Jeong
@ 2021-10-28 22:20 ` Gao Xiang
2021-10-28 22:25 ` Daeho Jeong
0 siblings, 1 reply; 13+ messages in thread
From: Gao Xiang @ 2021-10-28 22:20 UTC (permalink / raw)
To: Daeho Jeong
Cc: Daeho Jeong, linux-erofs, Jaegeuk Kim, Gao Xiang, miaoxie, Wang Qi
On Thu, Oct 28, 2021 at 10:10:03AM -0700, Daeho Jeong wrote:
> > > In fact, I wanted to decompress the whole data here. We can't check
> > > the data integrity,
> > > so I just wanted to check the layout of the file and that is the
> > > reason why I used z_erofs_map_blocks_iter() directly.
> >
> > Yeah, z_erofs_map_blocks_iter() here is good, yet I think we could
> > add a follow-up z_erofs_decompress() as well, at least it can verify
> > obvious compressed data corruption.
>
> Could you enlighten me what is wrong with the below flow?
> z_erofs_decompress fails with -EIO or -EUCLEAN.
>
> raw = malloc(pchunk_len);
> BUG_ON(!raw);
> buffer = malloc(inode->i_size);
> BUG_ON(!buffer);
>
> ret = dev_read(raw, 0, pchunk_len);
> if (ret < 0) {
> erofs_err("an error occurred when reading compressed data "
> "of nid(%llu): errno(%d)", inode->nid | 0ULL, ret);
> goto out;
> }
>
> ret = z_erofs_decompress(&(struct z_erofs_decompress_req) {
> .in = raw,
> .out = buffer,
> .decodedskip = 0,
> .inputsize = pchunk_len,
> .decodedlength = inode->i_size,
I guess try to pass map.m_llen here? since we need to decode pcluster
one-by-one....
Thanks,
Gao Xiang
> .alg = algorithmformat,
> .partial_decoding = 0
> });
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] erofs-utils: introduce fsck.erofs
2021-10-28 22:20 ` Gao Xiang
@ 2021-10-28 22:25 ` Daeho Jeong
0 siblings, 0 replies; 13+ messages in thread
From: Daeho Jeong @ 2021-10-28 22:25 UTC (permalink / raw)
To: Daeho Jeong, Gao Xiang, linux-erofs, miaoxie, fangwei1, xiang,
Daeho Jeong, GuoXuenan, Wang Qi, Jaegeuk Kim
On Thu, Oct 28, 2021 at 3:20 PM Gao Xiang <xiang@kernel.org> wrote:
>
> On Thu, Oct 28, 2021 at 10:10:03AM -0700, Daeho Jeong wrote:
> > > > In fact, I wanted to decompress the whole data here. We can't check
> > > > the data integrity,
> > > > so I just wanted to check the layout of the file and that is the
> > > > reason why I used z_erofs_map_blocks_iter() directly.
> > >
> > > Yeah, z_erofs_map_blocks_iter() here is good, yet I think we could
> > > add a follow-up z_erofs_decompress() as well, at least it can verify
> > > obvious compressed data corruption.
> >
> > Could you enlighten me what is wrong with the below flow?
> > z_erofs_decompress fails with -EIO or -EUCLEAN.
> >
> > raw = malloc(pchunk_len);
> > BUG_ON(!raw);
> > buffer = malloc(inode->i_size);
> > BUG_ON(!buffer);
> >
> > ret = dev_read(raw, 0, pchunk_len);
> > if (ret < 0) {
> > erofs_err("an error occurred when reading compressed data "
> > "of nid(%llu): errno(%d)", inode->nid | 0ULL, ret);
> > goto out;
> > }
> >
> > ret = z_erofs_decompress(&(struct z_erofs_decompress_req) {
> > .in = raw,
> > .out = buffer,
> > .decodedskip = 0,
> > .inputsize = pchunk_len,
> > .decodedlength = inode->i_size,
>
> I guess try to pass map.m_llen here? since we need to decode pcluster
> one-by-one....
Oh, I had to decompress them by cluster, not the whole file. Got it.
Thanks,
>
> Thanks,
> Gao Xiang
>
> > .alg = algorithmformat,
> > .partial_decoding = 0
> > });
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-10-28 22:26 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-25 19:48 [PATCH] erofs-utils: introduce fsck.erofs Daeho Jeong
2021-10-25 23:24 ` Gao Xiang
2021-10-26 2:42 ` Guo Xuenan
2021-10-26 3:12 ` Gao Xiang
2021-10-26 11:34 ` Guo Xuenan
2021-10-26 18:40 ` Daeho Jeong
2021-10-26 18:35 ` Daeho Jeong
2021-10-27 3:13 ` Gao Xiang
2021-10-28 0:50 ` Daeho Jeong
2021-10-28 1:00 ` Gao Xiang
2021-10-28 17:10 ` Daeho Jeong
2021-10-28 22:20 ` Gao Xiang
2021-10-28 22:25 ` Daeho Jeong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).