[RFC PATCH 1/8] fsck.f2fs: do not set fix_on directly

[RFC PATCH 1/8] fsck.f2fs: do not set fix_on directly

[Sheng Yong]

Do not set fix_on if it is allowed by user.

Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
 fsck/main.c  | 2 +-
 fsck/mount.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fsck/main.c b/fsck/main.c
index c9411eb..93037e1 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -429,7 +429,7 @@ static void do_fsck(struct f2fs_sb_info *sbi)
 				c.fix_on = 1;
 			break;
 		}
-	} else {
+	} else if (c.preen_mode) {
 		/*
 		 * we can hit this in 3 situations:
 		 *  1. fsck -f, fix_on has already been set to 1 when
diff --git a/fsck/mount.c b/fsck/mount.c
index 29af3b7..faa9bc8 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -1879,6 +1879,7 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
 
 	memset(&nat_block->entries[entry_off], 0,
 					sizeof(struct f2fs_nat_entry));
+	FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
 
 	ret = dev_write_block(nat_block, block_addr);
 	ASSERT(ret >= 0);
@@ -2031,7 +2032,6 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
 				 */
 				ASSERT_MSG("Invalid nat entry[0]: "
 					"blk_addr[0x%x]\n", ni.blk_addr);
-				c.fix_on = 1;
 				fsck->chk.valid_nat_entry_cnt--;
 			}
 
-- 
2.11.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[RFC PATCH 2/8] fsck.f2fs: do not fix corrupted nat entries in build_nat_area_bitmap

[Sheng Yong]

Fixing corrupted data depends on c.fix_on. If it's not set, we should not
force fixing corrupted data.  So if nat entries are found invalid when
building nat_area_bitmap, we should just set c.bug_on, and fix it later.

Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
 fsck/mount.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/fsck/mount.c b/fsck/mount.c
index faa9bc8..2d51678 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -1877,9 +1877,15 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
 	ret = dev_read_block(nat_block, block_addr);
 	ASSERT(ret >= 0);
 
-	memset(&nat_block->entries[entry_off], 0,
+	if (nid == F2FS_NODE_INO(sbi) || nid == F2FS_META_INO(sbi)) {
+		FIX_MSG("nid [0x%x] block_addr= 0x%x -> 0x1", nid,
+			le32_to_cpu(nat_block->entries[entry_off].block_addr));
+		nat_block->entries[entry_off].block_addr = cpu_to_le32(0x1);
+	} else {
+		memset(&nat_block->entries[entry_off], 0,
 					sizeof(struct f2fs_nat_entry));
-	FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
+		FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
+	}
 
 	ret = dev_write_block(nat_block, block_addr);
 	ASSERT(ret >= 0);
@@ -2001,13 +2007,14 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
 
 			if ((nid + i) == F2FS_NODE_INO(sbi) ||
 					(nid + i) == F2FS_META_INO(sbi)) {
-				/* block_addr of node/meta inode should be 0x1 */
+				/*
+				 * block_addr of node/meta inode should be 0x1.
+				 * Set this bit, and fsck_verify will fix it.
+				 */
 				if (le32_to_cpu(nat_block->entries[i].block_addr) != 0x1) {
-					FIX_MSG("ino: 0x%x node/meta inode, block_addr= 0x%x -> 0x1",
+					ASSERT_MSG("\tError: ino[0x%x] block_addr[0x%x] is invalid\n",
 							nid + i, le32_to_cpu(nat_block->entries[i].block_addr));
-					nat_block->entries[i].block_addr = cpu_to_le32(0x1);
-					ret = dev_write_block(nat_block, block_addr);
-					ASSERT(ret >= 0);
+					f2fs_set_bit(nid + i, fsck->nat_area_bitmap);
 				}
 				continue;
 			}
-- 
2.11.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[RFC PATCH 3/8] dump/fsck: introduce print_xattr_entry

[Sheng Yong]

This patch exports read_all_xattrs to allow dump/fsck to get all xattrs,
and introduces print_xattr_entry which tries to parse an xattr entry
accroding to its xattr index.

Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
 fsck/dump.c  |  18 ++++-----
 fsck/fsck.h  |   7 +++-
 fsck/mount.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 fsck/xattr.c |   2 +-
 fsck/xattr.h |  54 ++++++++++++++++++++++++-
 5 files changed, 192 insertions(+), 16 deletions(-)

diff --git a/fsck/dump.c b/fsck/dump.c
index 128dc53..90fd073 100644
--- a/fsck/dump.c
+++ b/fsck/dump.c
@@ -419,17 +419,17 @@ void dump_node(struct f2fs_sb_info *sbi, nid_t nid, int force)
 	if (le32_to_cpu(node_blk->footer.ino) == ni.ino &&
 			le32_to_cpu(node_blk->footer.nid) == ni.nid &&
 			ni.ino == ni.nid) {
-		print_node_info(node_blk, force);
+		print_node_info(sbi, node_blk, force);
 		dump_file(sbi, &ni, node_blk, force);
 	} else {
-		print_node_info(node_blk, force);
+		print_node_info(sbi, node_blk, force);
 		MSG(force, "Invalid (i)node block\n\n");
 	}
 
 	free(node_blk);
 }
 
-static void dump_node_from_blkaddr(u32 blk_addr)
+static void dump_node_from_blkaddr(struct f2fs_sb_info *sbi, u32 blk_addr)
 {
 	struct f2fs_node *node_blk;
 	int ret;
@@ -441,9 +441,9 @@ static void dump_node_from_blkaddr(u32 blk_addr)
 	ASSERT(ret >= 0);
 
 	if (c.dbg_lv > 0)
-		print_node_info(node_blk, 0);
+		print_node_info(sbi, node_blk, 0);
 	else
-		print_inode_info(&node_blk->i, 1);
+		print_inode_info(sbi, node_blk, 1);
 
 	free(node_blk);
 }
@@ -567,7 +567,7 @@ int dump_info_from_blkaddr(struct f2fs_sb_info *sbi, u32 blk_addr)
 
 	/* print inode */
 	if (c.dbg_lv > 0)
-		dump_node_from_blkaddr(ino_ni.blk_addr);
+		dump_node_from_blkaddr(sbi, ino_ni.blk_addr);
 
 	if (type == SEG_TYPE_CUR_DATA || type == SEG_TYPE_DATA) {
 		MSG(0, "FS Userdata Area: Data block from 0x%x\n", blk_addr);
@@ -575,7 +575,7 @@ int dump_info_from_blkaddr(struct f2fs_sb_info *sbi, u32 blk_addr)
 					nid, ni.blk_addr);
 		MSG(0, " - Inode block       : id = 0x%x from 0x%x\n",
 					ni.ino, ino_ni.blk_addr);
-		dump_node_from_blkaddr(ino_ni.blk_addr);
+		dump_node_from_blkaddr(sbi, ino_ni.blk_addr);
 		dump_data_offset(ni.blk_addr,
 			le16_to_cpu(sum_entry.ofs_in_node));
 	} else {
@@ -583,13 +583,13 @@ int dump_info_from_blkaddr(struct f2fs_sb_info *sbi, u32 blk_addr)
 		if (ni.ino == ni.nid) {
 			MSG(0, " - Inode block       : id = 0x%x from 0x%x\n",
 					ni.ino, ino_ni.blk_addr);
-			dump_node_from_blkaddr(ino_ni.blk_addr);
+			dump_node_from_blkaddr(sbi, ino_ni.blk_addr);
 		} else {
 			MSG(0, " - Node block        : id = 0x%x from 0x%x\n",
 					nid, ni.blk_addr);
 			MSG(0, " - Inode block       : id = 0x%x from 0x%x\n",
 					ni.ino, ino_ni.blk_addr);
-			dump_node_from_blkaddr(ino_ni.blk_addr);
+			dump_node_from_blkaddr(sbi, ino_ni.blk_addr);
 			dump_node_offset(ni.blk_addr);
 		}
 	}
diff --git a/fsck/fsck.h b/fsck/fsck.h
index 1e8ed0b..f8caa46 100644
--- a/fsck/fsck.h
+++ b/fsck/fsck.h
@@ -141,8 +141,8 @@ int convert_encrypted_name(unsigned char *, int, unsigned char *, int);
 
 extern void update_free_segments(struct f2fs_sb_info *);
 void print_cp_state(u32);
-extern void print_node_info(struct f2fs_node *, int);
-extern void print_inode_info(struct f2fs_inode *, int);
+extern void print_node_info(struct f2fs_sb_info *, struct f2fs_node *, int);
+extern void print_inode_info(struct f2fs_sb_info *, struct f2fs_node *, int);
 extern struct seg_entry *get_seg_entry(struct f2fs_sb_info *, unsigned int);
 extern struct f2fs_summary_block *get_sum_block(struct f2fs_sb_info *,
 				unsigned int, int *);
@@ -228,4 +228,7 @@ int f2fs_symlink(struct f2fs_sb_info *, struct dentry *);
 int inode_set_selinux(struct f2fs_sb_info *, u32, const char *);
 int f2fs_find_path(struct f2fs_sb_info *, char *, nid_t *);
 
+/* xattr.c */
+void *read_all_xattrs(struct f2fs_sb_info *, struct f2fs_node *);
+
 #endif /* _FSCK_H_ */
diff --git a/fsck/mount.c b/fsck/mount.c
index 2d51678..a9cd581 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -9,7 +9,9 @@
  * published by the Free Software Foundation.
  */
 #include "fsck.h"
+#include "xattr.h"
 #include <locale.h>
+#include <linux/posix_acl.h>
 
 u32 get_free_segments(struct f2fs_sb_info *sbi)
 {
@@ -35,8 +37,120 @@ void update_free_segments(struct f2fs_sb_info *sbi)
 	i++;
 }
 
-void print_inode_info(struct f2fs_inode *inode, int name)
+void print_acl(char *value, int size)
 {
+	struct f2fs_acl_header *hdr = (struct f2fs_acl_header *)value;
+	struct f2fs_acl_entry *entry = (struct f2fs_acl_entry *)(hdr + 1);
+	const char *end = value + size;
+	int i, count;
+
+	if (hdr->a_version != cpu_to_le32(F2FS_ACL_VERSION)) {
+		MSG(0, "Invalid ACL version [0x%x : 0x%x]\n",
+				le32_to_cpu(hdr->a_version), F2FS_ACL_VERSION);
+		return;
+	}
+
+	count = f2fs_acl_count(size);
+	if (count <= 0) {
+		MSG(0, "Invalid ACL value size %d\n", size);
+		return;
+	}
+
+	for (i = 0; i < count; i++) {
+		if ((char *)entry > end) {
+			MSG(0, "Invalid ACL entries count %d\n", count);
+			return;
+		}
+
+		switch (le16_to_cpu(entry->e_tag)) {
+		case ACL_USER_OBJ:
+		case ACL_GROUP_OBJ:
+		case ACL_MASK:
+		case ACL_OTHER:
+			MSG(0, "tag:0x%x perm:0x%x\n",
+					le16_to_cpu(entry->e_tag),
+					le16_to_cpu(entry->e_perm));
+			entry = (struct f2fs_acl_entry *)((char *)entry +
+					sizeof(struct f2fs_acl_entry_short));
+			break;
+		case ACL_USER:
+			MSG(0, "tag:0x%x perm:0x%x uid:%u\n",
+					le16_to_cpu(entry->e_tag),
+					le16_to_cpu(entry->e_perm),
+					le32_to_cpu(entry->e_id));
+			entry = (struct f2fs_acl_entry *)((char *)entry +
+					sizeof(struct f2fs_acl_entry));
+			break;
+		case ACL_GROUP:
+			MSG(0, "tag:0x%x perm:0x%x gid:%u\n",
+					le16_to_cpu(entry->e_tag),
+					le16_to_cpu(entry->e_perm),
+					le32_to_cpu(entry->e_id));
+			entry = (struct f2fs_acl_entry *)((char *)entry +
+					sizeof(struct f2fs_acl_entry));
+			break;
+		default:
+			MSG(0, "Unknown ACL tag 0x%x\n",
+					le16_to_cpu(entry->e_tag));
+			return;
+		}
+	}
+}
+
+void print_xattr_entry(struct f2fs_xattr_entry *ent)
+{
+	char *value = (char *)(ent->e_name + le16_to_cpu(ent->e_name_len));
+	struct fscrypt_context *ctx;
+	int i;
+
+	MSG(0, "\nxattr: e_name_index:%d e_name:", ent->e_name_index);
+	for (i = 0; i < le16_to_cpu(ent->e_name_len); i++)
+		MSG(0, "%c", ent->e_name[i]);
+	MSG(0, " e_name_len:%d e_value_size:%d e_value:\n",
+			ent->e_name_len, le16_to_cpu(ent->e_value_size));
+
+	switch (ent->e_name_index) {
+	case F2FS_XATTR_INDEX_USER:
+	case F2FS_XATTR_INDEX_SECURITY:
+		for (i = 0; i < le16_to_cpu(ent->e_value_size); i++)
+			MSG(0, "%c", value[i]);
+		MSG(0, "\n");
+		break;
+	case F2FS_XATTR_INDEX_POSIX_ACL_ACCESS:
+	case F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT:
+		print_acl(value, le16_to_cpu(ent->e_value_size));
+		break;
+	case F2FS_XATTR_INDEX_TRUSTED:
+	case F2FS_XATTR_INDEX_LUSTRE:
+		for (i = 0; i < le16_to_cpu(ent->e_value_size); i++)
+			MSG(0, "%02X", value[i]);
+		MSG(0, "\n");
+		break;
+	case F2FS_XATTR_INDEX_ENCRYPTION:
+		ctx = (struct fscrypt_context *)value;
+		MSG(0, "format: %d\n", ctx->format);
+		MSG(0, "contents_encryption_mode: 0x%x\n", ctx->contents_encryption_mode);
+		MSG(0, "filenames_encryption_mode: 0x%x\n", ctx->filenames_encryption_mode);
+		MSG(0, "flags: 0x%x\n", ctx->flags);
+		MSG(0, "master_key_descriptor: ");
+		for (i = 0; i < FS_KEY_DESCRIPTOR_SIZE; i++)
+			MSG(0, "%02X", ctx->master_key_descriptor[i]);
+		MSG(0, "\nnonce: ");
+		for (i = 0; i < FS_KEY_DERIVATION_NONCE_SIZE; i++)
+			MSG(0, "%02X", ctx->nonce[i]);
+		MSG(0, "\n");
+		break;
+	default:
+		break;
+	}
+}
+
+void print_inode_info(struct f2fs_sb_info *sbi,
+			struct f2fs_node *node, int name)
+{
+	struct f2fs_inode *inode = &node->i;
+	void *xattr_addr;
+	struct f2fs_xattr_entry *ent;
 	unsigned char en[F2FS_NAME_LEN + 1];
 	unsigned int i = 0;
 	int namelen = le32_to_cpu(inode->i_namelen);
@@ -111,17 +225,24 @@ void print_inode_info(struct f2fs_inode *inode, int name)
 	DISP_u32(inode, i_nid[3]);	/* indirect */
 	DISP_u32(inode, i_nid[4]);	/* double indirect */
 
+	xattr_addr = read_all_xattrs(sbi, node);
+	list_for_each_xattr(ent, xattr_addr) {
+		print_xattr_entry(ent);
+	}
+	free(xattr_addr);
+
 	printf("\n");
 }
 
-void print_node_info(struct f2fs_node *node_block, int verbose)
+void print_node_info(struct f2fs_sb_info *sbi,
+			struct f2fs_node *node_block, int verbose)
 {
 	nid_t ino = le32_to_cpu(node_block->footer.ino);
 	nid_t nid = le32_to_cpu(node_block->footer.nid);
 	/* Is this inode? */
 	if (ino == nid) {
 		DBG(verbose, "Node ID [0x%x:%u] is inode\n", nid, nid);
-		print_inode_info(&node_block->i, verbose);
+		print_inode_info(sbi, node_block, verbose);
 	} else {
 		int i;
 		u32 *dump_blk = (u32 *)node_block;
diff --git a/fsck/xattr.c b/fsck/xattr.c
index 3f5c7d3..1d0f7d3 100644
--- a/fsck/xattr.c
+++ b/fsck/xattr.c
@@ -20,7 +20,7 @@
 #define XATTR_CREATE 0x1
 #define XATTR_REPLACE 0x2
 
-static void *read_all_xattrs(struct f2fs_sb_info *sbi, struct f2fs_node *inode)
+void *read_all_xattrs(struct f2fs_sb_info *sbi, struct f2fs_node *inode)
 {
 	struct f2fs_xattr_header *header;
 	void *txattr_addr;
diff --git a/fsck/xattr.h b/fsck/xattr.h
index b414629..beed3bb 100644
--- a/fsck/xattr.h
+++ b/fsck/xattr.h
@@ -31,10 +31,62 @@ struct f2fs_xattr_entry {
 	char e_name[0];		/* attribute name */
 };
 
+#define FS_KEY_DESCRIPTOR_SIZE 8
+#define FS_KEY_DERIVATION_NONCE_SIZE 16
+
+struct fscrypt_context {
+	u8 format;
+	u8 contents_encryption_mode;
+	u8 filenames_encryption_mode;
+	u8 flags;
+	u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
+	u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
+} __attribute__((packed));
+
+#define F2FS_ACL_VERSION	0x0001
+
+struct f2fs_acl_entry {
+	__le16 e_tag;
+	__le16 e_perm;
+	__le32 e_id;
+};
+
+struct f2fs_acl_entry_short {
+	__le16 e_tag;
+	__le16 e_perm;
+};
+
+struct f2fs_acl_header {
+	__le32 a_version;
+};
+
+static inline int f2fs_acl_count(int size)
+{
+	ssize_t s;
+	size -= sizeof(struct f2fs_acl_header);
+	s = size - 4 * sizeof(struct f2fs_acl_entry_short);
+	if (s < 0) {
+		if (size % sizeof(struct f2fs_acl_entry_short))
+			return -1;
+		return size / sizeof(struct f2fs_acl_entry_short);
+	} else {
+		if (s % sizeof(struct f2fs_acl_entry))
+			return -1;
+		return s / sizeof(struct f2fs_acl_entry) + 4;
+	}
+}
+
 #define XATTR_ROUND	(3)
 
 #define XATTR_SELINUX_SUFFIX "selinux"
-#define F2FS_XATTR_INDEX_SECURITY	6
+#define F2FS_XATTR_INDEX_USER			1
+#define F2FS_XATTR_INDEX_POSIX_ACL_ACCESS	2
+#define F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT	3
+#define F2FS_XATTR_INDEX_TRUSTED		4
+#define F2FS_XATTR_INDEX_LUSTRE			5
+#define F2FS_XATTR_INDEX_SECURITY		6
+#define F2FS_XATTR_INDEX_ENCRYPTION		9
+
 #define IS_XATTR_LAST_ENTRY(entry) (*(__u32 *)(entry) == 0)
 
 #define XATTR_HDR(ptr)		((struct f2fs_xattr_header *)(ptr))
-- 
2.11.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[RFC PATCH 4/8] dump.f2fs: introduce dump_xattr

[Sheng Yong]

This patch introduces dump_xattr to create xattrs for dumped files.

Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
 fsck/dump.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 60 insertions(+), 3 deletions(-)

diff --git a/fsck/dump.c b/fsck/dump.c
index 90fd073..01889fd 100644
--- a/fsck/dump.c
+++ b/fsck/dump.c
@@ -11,6 +11,9 @@
 #include <inttypes.h>
 
 #include "fsck.h"
+#include "xattr.h"
+#include <attr/xattr.h>
+#include <linux/xattr.h>
 #include <locale.h>
 
 #define BUF_SZ	80
@@ -310,15 +313,67 @@ static void dump_node_blk(struct f2fs_sb_info *sbi, int ntype,
 	free(node_blk);
 }
 
+static void dump_xattr(struct f2fs_sb_info *sbi, struct f2fs_node *node_blk)
+{
+	void *xattr;
+	struct f2fs_xattr_entry *ent;
+	char xattr_name[F2FS_NAME_LEN] = {0};
+	int ret;
+
+	xattr = read_all_xattrs(sbi, node_blk);
+
+	list_for_each_xattr(ent, xattr) {
+		char *name = strndup(ent->e_name, ent->e_name_len);
+		void *value = ent->e_name + ent->e_name_len;
+
+		if (!name)
+			continue;
+
+		switch (ent->e_name_index) {
+		case F2FS_XATTR_INDEX_USER:
+			ret = snprintf(xattr_name, F2FS_NAME_LEN, "%s%s",
+				       XATTR_USER_PREFIX, name);
+			break;
+
+		case F2FS_XATTR_INDEX_SECURITY:
+			ret = snprintf(xattr_name, F2FS_NAME_LEN, "%s%s",
+				       XATTR_SECURITY_PREFIX, name);
+			break;
+		case F2FS_XATTR_INDEX_TRUSTED:
+			ret = snprintf(xattr_name, F2FS_NAME_LEN, "%s%s",
+				       XATTR_TRUSTED_PREFIX, name);
+			break;
+		default:
+			MSG(0, "Unknown xattr index 0x%x\n", ent->e_name_index);
+			free(name);
+			continue;
+		}
+		if (ret >= F2FS_NAME_LEN) {
+			MSG(0, "XATTR index 0x%x name too long\n", ent->e_name_index);
+			free(name);
+			continue;
+		}
+
+		DBG(1, "fd %d xattr_name %s\n", c.dump_fd, xattr_name);
+		ret = fsetxattr(c.dump_fd, xattr_name, value,
+				le16_to_cpu(ent->e_value_size), XATTR_CREATE);
+		if (ret)
+			MSG(0, "XATTR index 0x%x set xattr failed error %d\n",
+			    ent->e_name_index, errno);
+
+		free(name);
+	}
+
+	free(xattr);
+}
+
 static void dump_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
 					struct f2fs_node *node_blk)
 {
 	u32 i = 0;
 	u64 ofs = 0;
 
-	/* TODO: need to dump xattr */
-
-	if((node_blk->i.i_inline & F2FS_INLINE_DATA)){
+	if((node_blk->i.i_inline & F2FS_INLINE_DATA)) {
 		DBG(3, "ino[0x%x] has inline data!\n", nid);
 		/* recover from inline data */
 		dev_write_dump(((unsigned char *)node_blk) + INLINE_DATA_OFFSET,
@@ -345,6 +400,8 @@ static void dump_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
 		else
 			ASSERT(0);
 	}
+
+	dump_xattr(sbi, node_blk);
 }
 
 static void dump_file(struct f2fs_sb_info *sbi, struct node_info *ni,
-- 
2.11.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[RFC PATCH 5/8] dump.f2fs: do not dump encrypted files

[Sheng Yong]

If a file is encrypted, its content is cipher text on the storage. So
there is no need to dump an encrypted file.

Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
 fsck/dump.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fsck/dump.c b/fsck/dump.c
index 01889fd..d11e8e2 100644
--- a/fsck/dump.c
+++ b/fsck/dump.c
@@ -410,12 +410,17 @@ static void dump_file(struct f2fs_sb_info *sbi, struct node_info *ni,
 	struct f2fs_inode *inode = &node_blk->i;
 	u32 imode = le32_to_cpu(inode->i_mode);
 	u32 namelen = le32_to_cpu(inode->i_namelen);
-	unsigned char name[F2FS_NAME_LEN + 1] = {0};
+	char name[F2FS_NAME_LEN + 1] = {0};
 	char path[1024] = {0};
 	char ans[255] = {0};
-	int enc_name = file_enc_name(inode);
+	int is_encrypted = file_is_encrypt(inode);
 	int ret;
 
+	if (is_encrypted) {
+		MSG(force, "File is encrypted\n");
+		return;
+	}
+
 	if (!S_ISREG(imode) || namelen == 0 || namelen > F2FS_NAME_LEN) {
 		MSG(force, "Not a regular file or wrong name info\n\n");
 		return;
@@ -433,8 +438,7 @@ dump:
 		ASSERT(ret >= 0);
 
 		/* make a file */
-		namelen = convert_encrypted_name(inode->i_name, namelen,
-							name, enc_name);
+		strncpy(name, (const char *)inode->i_name, namelen);
 		name[namelen] = 0;
 		sprintf(path, "./lost_found/%s", name);
 
-- 
2.11.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[RFC PATCH 6/8] fsck.f2fs: introduce new option --dry-run

[Sheng Yong]

With --dry-run enabled, fsck.f2fs will do all checks and "fixes" except
that all fixes will not be written to storage at last.

Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
 fsck/main.c       | 14 +++++++++++++-
 include/f2fs_fs.h |  1 +
 lib/libf2fs.c     |  1 +
 lib/libf2fs_io.c  |  3 +++
 4 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/fsck/main.c b/fsck/main.c
index 93037e1..baa8efc 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -18,6 +18,7 @@
 #include "fsck.h"
 #include <libgen.h>
 #include <ctype.h>
+#include <getopt.h>
 
 struct f2fs_fsck gfsck;
 
@@ -30,6 +31,7 @@ void fsck_usage()
 	MSG(0, "  -f check/fix entire partition\n");
 	MSG(0, "  -p preen mode [default:0 the same as -a [0|1]]\n");
 	MSG(0, "  -t show directory tree\n");
+	MSG(0, "  --dry-run do not really fix corruptions\n");
 	exit(1);
 }
 
@@ -117,10 +119,20 @@ void f2fs_parse_options(int argc, char *argv[])
 
 	if (!strcmp("fsck.f2fs", prog)) {
 		const char *option_string = ":ad:fp:t";
+		int opt = 0;
+		struct option long_opt[] = {
+			{"dry-run", no_argument, 0, 1},
+			{0, 0, 0, 0}
+		};
 
 		c.func = FSCK;
-		while ((option = getopt(argc, argv, option_string)) != EOF) {
+		while ((option = getopt_long(argc, argv, option_string,
+						long_opt, &opt)) != EOF) {
 			switch (option) {
+			case 1:
+				c.dry_run = 1;
+				MSG(0, "Info: Dry run\n");
+				break;
 			case 'a':
 				c.auto_fix = 1;
 				MSG(0, "Info: Fix the reported corruption.\n");
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index e1e299d..64a796d 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -300,6 +300,7 @@ struct f2fs_configuration {
 	int trimmed;
 	int func;
 	void *private;
+	int dry_run;
 	int fix_on;
 	int bug_on;
 	int auto_fix;
diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index 69f5ccc..259bd17 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -598,6 +598,7 @@ void f2fs_init_configuration(void)
 	c.trimmed = 0;
 	c.ro = 0;
 	c.kd = -1;
+	c.dry_run = 0;
 }
 
 static int is_mounted(const char *mpt, const char *device)
diff --git a/lib/libf2fs_io.c b/lib/libf2fs_io.c
index 50ff171..8a79672 100644
--- a/lib/libf2fs_io.c
+++ b/lib/libf2fs_io.c
@@ -129,6 +129,9 @@ int dev_write(void *buf, __u64 offset, size_t len)
 {
 	int fd;
 
+	if (c.dry_run)
+		return 0;
+
 	if (c.sparse_mode)
 		return dev_write_sparse(buf, offset, len);
 
-- 
2.11.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[RFC PATCH 7/8] fsck.f2fs: introduce sanity_check_inode

[Sheng Yong]

f2fs_iget checks if i_mode is valid. If it is not, the file cannot be
accessed as well as deleted. To make sure such files can be removed,
fsck adds the same check, and removes incorrect inode blocks.

Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
 fsck/fsck.c  | 21 +++++++++++++++++++++
 fsck/mount.c |  4 ++--
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index 56a47be..77490d8 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -458,6 +458,25 @@ static int sanity_check_nid(struct f2fs_sb_info *sbi, u32 nid,
 	return 0;
 }
 
+static int sanity_check_inode(struct f2fs_sb_info *sbi, struct f2fs_node *node)
+{
+	struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
+	struct f2fs_inode *fi = &node->i;
+
+	if (!(le16_to_cpu(fi->i_mode) & S_IFMT)) {
+		ASSERT_MSG("i_mode is not valid. [0x%x]", le16_to_cpu(fi->i_mode));
+		goto remove_node;
+	}
+
+	return 0;
+
+remove_node:
+	f2fs_set_bit(le32_to_cpu(node->footer.ino), fsck->nat_area_bitmap);
+	fsck->chk.valid_blk_cnt--;
+	fsck->chk.valid_node_cnt--;
+	return -EINVAL;
+}
+
 static int fsck_chk_xattr_blk(struct f2fs_sb_info *sbi, u32 ino,
 					u32 x_nid, u32 *blk_cnt)
 {
@@ -500,6 +519,8 @@ int fsck_chk_node_blk(struct f2fs_sb_info *sbi, struct f2fs_inode *inode,
 		goto err;
 
 	if (ntype == TYPE_INODE) {
+		if (sanity_check_inode(sbi, node_blk))
+			goto err;
 		fsck_chk_inode_blk(sbi, nid, ftype, node_blk, blk_cnt, &ni);
 	} else {
 		switch (ntype) {
diff --git a/fsck/mount.c b/fsck/mount.c
index a9cd581..45341bc 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -1985,7 +1985,7 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
 		if (le32_to_cpu(nid_in_journal(journal, i)) == nid) {
 			memset(&nat_in_journal(journal, i), 0,
 					sizeof(struct f2fs_nat_entry));
-			FIX_MSG("Remove nid [0x%x] in nat journal\n", nid);
+			FIX_MSG("Remove nid [0x%x] in nat journal", nid);
 			return;
 		}
 	}
@@ -2005,7 +2005,7 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
 	} else {
 		memset(&nat_block->entries[entry_off], 0,
 					sizeof(struct f2fs_nat_entry));
-		FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
+		FIX_MSG("Remove nid [0x%x] in NAT", nid);
 	}
 
 	ret = dev_write_block(nat_block, block_addr);
-- 
2.11.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[RFC PATCH 8/8] f2fs-tools: remove unused list.h

[Sheng Yong]

Commit 7d96d138a378 ("fsck.f2fs: remove list.h") has already the usage
of list.h. So let's remove list.h itself.

Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
 include/list.h | 88 ----------------------------------------------------------
 1 file changed, 88 deletions(-)
 delete mode 100644 include/list.h

diff --git a/include/list.h b/include/list.h
deleted file mode 100644
index 571cd5c..0000000
--- a/include/list.h
+++ /dev/null
@@ -1,88 +0,0 @@
-
-#define POISON_POINTER_DELTA 0
-#define LIST_POISON1  ((void *) (0x00100100 + POISON_POINTER_DELTA))
-#define LIST_POISON2  ((void *) (0x00200200 + POISON_POINTER_DELTA))
-
-#if !defined(offsetof)
-#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
-#endif
-#define container_of(ptr, type, member) ({                      \
-		const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
-		(type *)( (char *)__mptr - offsetof(type,member) );})
-
-struct list_head {
-	struct list_head *next, *prev;
-};
-
-#define LIST_HEAD_INIT(name) { &(name), &(name) }
-
-#define LIST_HEAD(name) \
-	struct list_head name = LIST_HEAD_INIT(name)
-
-static inline void INIT_LIST_HEAD(struct list_head *list)
-{
-	list->next = list;
-	list->prev = list;
-}
-
-static inline void __list_add(struct list_head *new,
-		struct list_head *prev,
-		struct list_head *next)
-{
-	next->prev = new;
-	new->next = next;
-	new->prev = prev;
-	prev->next = new;
-}
-
-static inline void list_add(struct list_head *new, struct list_head *head)
-{
-	__list_add(new, head, head->next);
-}
-
-static inline void list_add_tail(struct list_head *new, struct list_head *head)
-{
-	__list_add(new, head->prev, head);
-}
-
-static inline void __list_del(struct list_head * prev, struct list_head * next)
-{
-	next->prev = prev;
-	prev->next = next;
-}
-
-static inline void __list_del_entry(struct list_head *entry)
-{
-	__list_del(entry->prev, entry->next);
-}
-
-static inline void list_del(struct list_head *entry)
-{
-	__list_del(entry->prev, entry->next);
-	entry->next = LIST_POISON1;
-	entry->prev = LIST_POISON2;
-}
-
-static inline int list_empty(const struct list_head *head)
-{
-	return head->next == head;
-}
-
-#define list_entry(ptr, type, member) \
-	container_of(ptr, type, member)
-
-#define list_for_each(pos, head) \
-	for (pos = (head)->next; pos != (head); pos = pos->next)
-
-#define list_for_each_safe(pos, n, head) \
-	for (pos = (head)->next, n = pos->next; pos != (head); \
-			pos = n, n = pos->next)
-#define list_for_each_entry(pos, head, member)                          \
-	for (pos = list_entry((head)->next, typeof(*pos), member);      \
-			&pos->member != (head);    \
-			pos = list_entry(pos->member.next, typeof(*pos), member))
-#define list_for_each_entry_safe(pos, n, head, member)                  \
-	for (pos = list_entry((head)->next, typeof(*pos), member),      \
-			n = list_entry(pos->member.next, typeof(*pos), member); \
-			&pos->member != (head);                                    \
-			pos = n, n = list_entry(n->member.next, typeof(*n), member))
-- 
2.11.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 1/8] fsck.f2fs: do not set fix_on directly

[Chao Yu]

On 2017/10/31 9:38, Sheng Yong wrote:
> Do not set fix_on if it is allowed by user.
> 
> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
> ---
>  fsck/main.c  | 2 +-
>  fsck/mount.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fsck/main.c b/fsck/main.c
> index c9411eb..93037e1 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -429,7 +429,7 @@ static void do_fsck(struct f2fs_sb_info *sbi)
>  				c.fix_on = 1;
>  			break;
>  		}
> -	} else {
> +	} else if (c.preen_mode) {

Needs to update below comments?

>  		/*
>  		 * we can hit this in 3 situations:
>  		 *  1. fsck -f, fix_on has already been set to 1 when
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 29af3b7..faa9bc8 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -1879,6 +1879,7 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
>  
>  	memset(&nat_block->entries[entry_off], 0,
>  					sizeof(struct f2fs_nat_entry));
> +	FIX_MSG("Remove nid [0x%x] in NAT\n", nid);

How about adding debug message in separated patch?

Thanks,

>  
>  	ret = dev_write_block(nat_block, block_addr);
>  	ASSERT(ret >= 0);
> @@ -2031,7 +2032,6 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
>  				 */
>  				ASSERT_MSG("Invalid nat entry[0]: "
>  					"blk_addr[0x%x]\n", ni.blk_addr);
> -				c.fix_on = 1;
>  				fsck->chk.valid_nat_entry_cnt--;
>  			}
>  
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 2/8] fsck.f2fs: do not fix corrupted nat entries in build_nat_area_bitmap

[Chao Yu]

On 2017/10/31 9:38, Sheng Yong wrote:
> Fixing corrupted data depends on c.fix_on. If it's not set, we should not
> force fixing corrupted data.  So if nat entries are found invalid when
> building nat_area_bitmap, we should just set c.bug_on, and fix it later.

It looks comments and code change doesn't match? do you mean

@@ -2031,7 +2032,6 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
 				 */
 				ASSERT_MSG("Invalid nat entry[0]: "
 					"blk_addr[0x%x]\n", ni.blk_addr);
-				c.fix_on = 1;
 				fsck->chk.valid_nat_entry_cnt--;
 			}

Thanks,

> 
> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
> ---
>  fsck/mount.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/fsck/mount.c b/fsck/mount.c
> index faa9bc8..2d51678 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -1877,9 +1877,15 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
>  	ret = dev_read_block(nat_block, block_addr);
>  	ASSERT(ret >= 0);
>  
> -	memset(&nat_block->entries[entry_off], 0,
> +	if (nid == F2FS_NODE_INO(sbi) || nid == F2FS_META_INO(sbi)) {
> +		FIX_MSG("nid [0x%x] block_addr= 0x%x -> 0x1", nid,
> +			le32_to_cpu(nat_block->entries[entry_off].block_addr));
> +		nat_block->entries[entry_off].block_addr = cpu_to_le32(0x1);
> +	} else {
> +		memset(&nat_block->entries[entry_off], 0,
>  					sizeof(struct f2fs_nat_entry));
> -	FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
> +		FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
> +	}
>  
>  	ret = dev_write_block(nat_block, block_addr);
>  	ASSERT(ret >= 0);
> @@ -2001,13 +2007,14 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
>  
>  			if ((nid + i) == F2FS_NODE_INO(sbi) ||
>  					(nid + i) == F2FS_META_INO(sbi)) {
> -				/* block_addr of node/meta inode should be 0x1 */
> +				/*
> +				 * block_addr of node/meta inode should be 0x1.
> +				 * Set this bit, and fsck_verify will fix it.
> +				 */
>  				if (le32_to_cpu(nat_block->entries[i].block_addr) != 0x1) {
> -					FIX_MSG("ino: 0x%x node/meta inode, block_addr= 0x%x -> 0x1",
> +					ASSERT_MSG("\tError: ino[0x%x] block_addr[0x%x] is invalid\n",
>  							nid + i, le32_to_cpu(nat_block->entries[i].block_addr));
> -					nat_block->entries[i].block_addr = cpu_to_le32(0x1);
> -					ret = dev_write_block(nat_block, block_addr);
> -					ASSERT(ret >= 0);
> +					f2fs_set_bit(nid + i, fsck->nat_area_bitmap);
>  				}
>  				continue;
>  			}
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 3/8] dump/fsck: introduce print_xattr_entry

[Chao Yu]

On 2017/10/31 9:38, Sheng Yong wrote:
> This patch exports read_all_xattrs to allow dump/fsck to get all xattrs,
> and introduces print_xattr_entry which tries to parse an xattr entry
> accroding to its xattr index.
> 
> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
> ---
>  fsck/dump.c  |  18 ++++-----
>  fsck/fsck.h  |   7 +++-
>  fsck/mount.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  fsck/xattr.c |   2 +-
>  fsck/xattr.h |  54 ++++++++++++++++++++++++-
>  5 files changed, 192 insertions(+), 16 deletions(-)
> 
> diff --git a/fsck/dump.c b/fsck/dump.c
> index 128dc53..90fd073 100644
> --- a/fsck/dump.c
> +++ b/fsck/dump.c
> @@ -419,17 +419,17 @@ void dump_node(struct f2fs_sb_info *sbi, nid_t nid, int force)
>  	if (le32_to_cpu(node_blk->footer.ino) == ni.ino &&
>  			le32_to_cpu(node_blk->footer.nid) == ni.nid &&
>  			ni.ino == ni.nid) {
> -		print_node_info(node_blk, force);
> +		print_node_info(sbi, node_blk, force);
>  		dump_file(sbi, &ni, node_blk, force);
>  	} else {
> -		print_node_info(node_blk, force);
> +		print_node_info(sbi, node_blk, force);
>  		MSG(force, "Invalid (i)node block\n\n");
>  	}
>  
>  	free(node_blk);
>  }
>  
> -static void dump_node_from_blkaddr(u32 blk_addr)
> +static void dump_node_from_blkaddr(struct f2fs_sb_info *sbi, u32 blk_addr)
>  {
>  	struct f2fs_node *node_blk;
>  	int ret;
> @@ -441,9 +441,9 @@ static void dump_node_from_blkaddr(u32 blk_addr)
>  	ASSERT(ret >= 0);
>  
>  	if (c.dbg_lv > 0)
> -		print_node_info(node_blk, 0);
> +		print_node_info(sbi, node_blk, 0);
>  	else
> -		print_inode_info(&node_blk->i, 1);
> +		print_inode_info(sbi, node_blk, 1);
>  
>  	free(node_blk);
>  }
> @@ -567,7 +567,7 @@ int dump_info_from_blkaddr(struct f2fs_sb_info *sbi, u32 blk_addr)
>  
>  	/* print inode */
>  	if (c.dbg_lv > 0)
> -		dump_node_from_blkaddr(ino_ni.blk_addr);
> +		dump_node_from_blkaddr(sbi, ino_ni.blk_addr);
>  
>  	if (type == SEG_TYPE_CUR_DATA || type == SEG_TYPE_DATA) {
>  		MSG(0, "FS Userdata Area: Data block from 0x%x\n", blk_addr);
> @@ -575,7 +575,7 @@ int dump_info_from_blkaddr(struct f2fs_sb_info *sbi, u32 blk_addr)
>  					nid, ni.blk_addr);
>  		MSG(0, " - Inode block       : id = 0x%x from 0x%x\n",
>  					ni.ino, ino_ni.blk_addr);
> -		dump_node_from_blkaddr(ino_ni.blk_addr);
> +		dump_node_from_blkaddr(sbi, ino_ni.blk_addr);
>  		dump_data_offset(ni.blk_addr,
>  			le16_to_cpu(sum_entry.ofs_in_node));
>  	} else {
> @@ -583,13 +583,13 @@ int dump_info_from_blkaddr(struct f2fs_sb_info *sbi, u32 blk_addr)
>  		if (ni.ino == ni.nid) {
>  			MSG(0, " - Inode block       : id = 0x%x from 0x%x\n",
>  					ni.ino, ino_ni.blk_addr);
> -			dump_node_from_blkaddr(ino_ni.blk_addr);
> +			dump_node_from_blkaddr(sbi, ino_ni.blk_addr);
>  		} else {
>  			MSG(0, " - Node block        : id = 0x%x from 0x%x\n",
>  					nid, ni.blk_addr);
>  			MSG(0, " - Inode block       : id = 0x%x from 0x%x\n",
>  					ni.ino, ino_ni.blk_addr);
> -			dump_node_from_blkaddr(ino_ni.blk_addr);
> +			dump_node_from_blkaddr(sbi, ino_ni.blk_addr);
>  			dump_node_offset(ni.blk_addr);
>  		}
>  	}
> diff --git a/fsck/fsck.h b/fsck/fsck.h
> index 1e8ed0b..f8caa46 100644
> --- a/fsck/fsck.h
> +++ b/fsck/fsck.h
> @@ -141,8 +141,8 @@ int convert_encrypted_name(unsigned char *, int, unsigned char *, int);
>  
>  extern void update_free_segments(struct f2fs_sb_info *);
>  void print_cp_state(u32);
> -extern void print_node_info(struct f2fs_node *, int);
> -extern void print_inode_info(struct f2fs_inode *, int);
> +extern void print_node_info(struct f2fs_sb_info *, struct f2fs_node *, int);
> +extern void print_inode_info(struct f2fs_sb_info *, struct f2fs_node *, int);
>  extern struct seg_entry *get_seg_entry(struct f2fs_sb_info *, unsigned int);
>  extern struct f2fs_summary_block *get_sum_block(struct f2fs_sb_info *,
>  				unsigned int, int *);
> @@ -228,4 +228,7 @@ int f2fs_symlink(struct f2fs_sb_info *, struct dentry *);
>  int inode_set_selinux(struct f2fs_sb_info *, u32, const char *);
>  int f2fs_find_path(struct f2fs_sb_info *, char *, nid_t *);
>  
> +/* xattr.c */
> +void *read_all_xattrs(struct f2fs_sb_info *, struct f2fs_node *);
> +
>  #endif /* _FSCK_H_ */
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 2d51678..a9cd581 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -9,7 +9,9 @@
>   * published by the Free Software Foundation.
>   */
>  #include "fsck.h"
> +#include "xattr.h"
>  #include <locale.h>
> +#include <linux/posix_acl.h>
>  
>  u32 get_free_segments(struct f2fs_sb_info *sbi)
>  {
> @@ -35,8 +37,120 @@ void update_free_segments(struct f2fs_sb_info *sbi)
>  	i++;
>  }
>  
> -void print_inode_info(struct f2fs_inode *inode, int name)
> +void print_acl(char *value, int size)
>  {
> +	struct f2fs_acl_header *hdr = (struct f2fs_acl_header *)value;
> +	struct f2fs_acl_entry *entry = (struct f2fs_acl_entry *)(hdr + 1);
> +	const char *end = value + size;
> +	int i, count;
> +
> +	if (hdr->a_version != cpu_to_le32(F2FS_ACL_VERSION)) {
> +		MSG(0, "Invalid ACL version [0x%x : 0x%x]\n",
> +				le32_to_cpu(hdr->a_version), F2FS_ACL_VERSION);
> +		return;
> +	}
> +
> +	count = f2fs_acl_count(size);
> +	if (count <= 0) {
> +		MSG(0, "Invalid ACL value size %d\n", size);
> +		return;
> +	}
> +
> +	for (i = 0; i < count; i++) {
> +		if ((char *)entry > end) {
> +			MSG(0, "Invalid ACL entries count %d\n", count);
> +			return;
> +		}
> +
> +		switch (le16_to_cpu(entry->e_tag)) {
> +		case ACL_USER_OBJ:
> +		case ACL_GROUP_OBJ:
> +		case ACL_MASK:
> +		case ACL_OTHER:
> +			MSG(0, "tag:0x%x perm:0x%x\n",
> +					le16_to_cpu(entry->e_tag),
> +					le16_to_cpu(entry->e_perm));
> +			entry = (struct f2fs_acl_entry *)((char *)entry +
> +					sizeof(struct f2fs_acl_entry_short));
> +			break;
> +		case ACL_USER:
> +			MSG(0, "tag:0x%x perm:0x%x uid:%u\n",
> +					le16_to_cpu(entry->e_tag),
> +					le16_to_cpu(entry->e_perm),
> +					le32_to_cpu(entry->e_id));
> +			entry = (struct f2fs_acl_entry *)((char *)entry +
> +					sizeof(struct f2fs_acl_entry));
> +			break;
> +		case ACL_GROUP:
> +			MSG(0, "tag:0x%x perm:0x%x gid:%u\n",
> +					le16_to_cpu(entry->e_tag),
> +					le16_to_cpu(entry->e_perm),
> +					le32_to_cpu(entry->e_id));
> +			entry = (struct f2fs_acl_entry *)((char *)entry +
> +					sizeof(struct f2fs_acl_entry));
> +			break;
> +		default:
> +			MSG(0, "Unknown ACL tag 0x%x\n",
> +					le16_to_cpu(entry->e_tag));
> +			return;
> +		}
> +	}
> +}
> +
> +void print_xattr_entry(struct f2fs_xattr_entry *ent)
> +{
> +	char *value = (char *)(ent->e_name + le16_to_cpu(ent->e_name_len));
> +	struct fscrypt_context *ctx;
> +	int i;
> +
> +	MSG(0, "\nxattr: e_name_index:%d e_name:", ent->e_name_index);
> +	for (i = 0; i < le16_to_cpu(ent->e_name_len); i++)
> +		MSG(0, "%c", ent->e_name[i]);
> +	MSG(0, " e_name_len:%d e_value_size:%d e_value:\n",
> +			ent->e_name_len, le16_to_cpu(ent->e_value_size));
> +
> +	switch (ent->e_name_index) {
> +	case F2FS_XATTR_INDEX_USER:
> +	case F2FS_XATTR_INDEX_SECURITY:
> +		for (i = 0; i < le16_to_cpu(ent->e_value_size); i++)
> +			MSG(0, "%c", value[i]);

It could be unreadable if user stores pure value in xattr entry.

Thanks,

> +		MSG(0, "\n");
> +		break;
> +	case F2FS_XATTR_INDEX_POSIX_ACL_ACCESS:
> +	case F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT:
> +		print_acl(value, le16_to_cpu(ent->e_value_size));
> +		break;
> +	case F2FS_XATTR_INDEX_TRUSTED:
> +	case F2FS_XATTR_INDEX_LUSTRE:
> +		for (i = 0; i < le16_to_cpu(ent->e_value_size); i++)
> +			MSG(0, "%02X", value[i]);
> +		MSG(0, "\n");
> +		break;
> +	case F2FS_XATTR_INDEX_ENCRYPTION:
> +		ctx = (struct fscrypt_context *)value;
> +		MSG(0, "format: %d\n", ctx->format);
> +		MSG(0, "contents_encryption_mode: 0x%x\n", ctx->contents_encryption_mode);
> +		MSG(0, "filenames_encryption_mode: 0x%x\n", ctx->filenames_encryption_mode);
> +		MSG(0, "flags: 0x%x\n", ctx->flags);
> +		MSG(0, "master_key_descriptor: ");
> +		for (i = 0; i < FS_KEY_DESCRIPTOR_SIZE; i++)
> +			MSG(0, "%02X", ctx->master_key_descriptor[i]);
> +		MSG(0, "\nnonce: ");
> +		for (i = 0; i < FS_KEY_DERIVATION_NONCE_SIZE; i++)
> +			MSG(0, "%02X", ctx->nonce[i]);
> +		MSG(0, "\n");
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +void print_inode_info(struct f2fs_sb_info *sbi,
> +			struct f2fs_node *node, int name)
> +{
> +	struct f2fs_inode *inode = &node->i;
> +	void *xattr_addr;
> +	struct f2fs_xattr_entry *ent;
>  	unsigned char en[F2FS_NAME_LEN + 1];
>  	unsigned int i = 0;
>  	int namelen = le32_to_cpu(inode->i_namelen);
> @@ -111,17 +225,24 @@ void print_inode_info(struct f2fs_inode *inode, int name)
>  	DISP_u32(inode, i_nid[3]);	/* indirect */
>  	DISP_u32(inode, i_nid[4]);	/* double indirect */
>  
> +	xattr_addr = read_all_xattrs(sbi, node);
> +	list_for_each_xattr(ent, xattr_addr) {
> +		print_xattr_entry(ent);
> +	}
> +	free(xattr_addr);
> +
>  	printf("\n");
>  }
>  
> -void print_node_info(struct f2fs_node *node_block, int verbose)
> +void print_node_info(struct f2fs_sb_info *sbi,
> +			struct f2fs_node *node_block, int verbose)
>  {
>  	nid_t ino = le32_to_cpu(node_block->footer.ino);
>  	nid_t nid = le32_to_cpu(node_block->footer.nid);
>  	/* Is this inode? */
>  	if (ino == nid) {
>  		DBG(verbose, "Node ID [0x%x:%u] is inode\n", nid, nid);
> -		print_inode_info(&node_block->i, verbose);
> +		print_inode_info(sbi, node_block, verbose);
>  	} else {
>  		int i;
>  		u32 *dump_blk = (u32 *)node_block;
> diff --git a/fsck/xattr.c b/fsck/xattr.c
> index 3f5c7d3..1d0f7d3 100644
> --- a/fsck/xattr.c
> +++ b/fsck/xattr.c
> @@ -20,7 +20,7 @@
>  #define XATTR_CREATE 0x1
>  #define XATTR_REPLACE 0x2
>  
> -static void *read_all_xattrs(struct f2fs_sb_info *sbi, struct f2fs_node *inode)
> +void *read_all_xattrs(struct f2fs_sb_info *sbi, struct f2fs_node *inode)
>  {
>  	struct f2fs_xattr_header *header;
>  	void *txattr_addr;
> diff --git a/fsck/xattr.h b/fsck/xattr.h
> index b414629..beed3bb 100644
> --- a/fsck/xattr.h
> +++ b/fsck/xattr.h
> @@ -31,10 +31,62 @@ struct f2fs_xattr_entry {
>  	char e_name[0];		/* attribute name */
>  };
>  
> +#define FS_KEY_DESCRIPTOR_SIZE 8
> +#define FS_KEY_DERIVATION_NONCE_SIZE 16
> +
> +struct fscrypt_context {
> +	u8 format;
> +	u8 contents_encryption_mode;
> +	u8 filenames_encryption_mode;
> +	u8 flags;
> +	u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
> +	u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
> +} __attribute__((packed));
> +
> +#define F2FS_ACL_VERSION	0x0001
> +
> +struct f2fs_acl_entry {
> +	__le16 e_tag;
> +	__le16 e_perm;
> +	__le32 e_id;
> +};
> +
> +struct f2fs_acl_entry_short {
> +	__le16 e_tag;
> +	__le16 e_perm;
> +};
> +
> +struct f2fs_acl_header {
> +	__le32 a_version;
> +};
> +
> +static inline int f2fs_acl_count(int size)
> +{
> +	ssize_t s;
> +	size -= sizeof(struct f2fs_acl_header);
> +	s = size - 4 * sizeof(struct f2fs_acl_entry_short);
> +	if (s < 0) {
> +		if (size % sizeof(struct f2fs_acl_entry_short))
> +			return -1;
> +		return size / sizeof(struct f2fs_acl_entry_short);
> +	} else {
> +		if (s % sizeof(struct f2fs_acl_entry))
> +			return -1;
> +		return s / sizeof(struct f2fs_acl_entry) + 4;
> +	}
> +}
> +
>  #define XATTR_ROUND	(3)
>  
>  #define XATTR_SELINUX_SUFFIX "selinux"
> -#define F2FS_XATTR_INDEX_SECURITY	6
> +#define F2FS_XATTR_INDEX_USER			1
> +#define F2FS_XATTR_INDEX_POSIX_ACL_ACCESS	2
> +#define F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT	3
> +#define F2FS_XATTR_INDEX_TRUSTED		4
> +#define F2FS_XATTR_INDEX_LUSTRE			5
> +#define F2FS_XATTR_INDEX_SECURITY		6
> +#define F2FS_XATTR_INDEX_ENCRYPTION		9
> +
>  #define IS_XATTR_LAST_ENTRY(entry) (*(__u32 *)(entry) == 0)
>  
>  #define XATTR_HDR(ptr)		((struct f2fs_xattr_header *)(ptr))
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 4/8] dump.f2fs: introduce dump_xattr

[Chao Yu]

On 2017/10/31 9:38, Sheng Yong wrote:
> This patch introduces dump_xattr to create xattrs for dumped files.
> 
> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
> ---
>  fsck/dump.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 60 insertions(+), 3 deletions(-)
> 
> diff --git a/fsck/dump.c b/fsck/dump.c
> index 90fd073..01889fd 100644
> --- a/fsck/dump.c
> +++ b/fsck/dump.c
> @@ -11,6 +11,9 @@
>  #include <inttypes.h>
>  
>  #include "fsck.h"
> +#include "xattr.h"
> +#include <attr/xattr.h>
> +#include <linux/xattr.h>
>  #include <locale.h>
>  
>  #define BUF_SZ	80
> @@ -310,15 +313,67 @@ static void dump_node_blk(struct f2fs_sb_info *sbi, int ntype,
>  	free(node_blk);
>  }
>  
> +static void dump_xattr(struct f2fs_sb_info *sbi, struct f2fs_node *node_blk)
> +{
> +	void *xattr;
> +	struct f2fs_xattr_entry *ent;
> +	char xattr_name[F2FS_NAME_LEN] = {0};
> +	int ret;
> +
> +	xattr = read_all_xattrs(sbi, node_blk);
> +
> +	list_for_each_xattr(ent, xattr) {
> +		char *name = strndup(ent->e_name, ent->e_name_len);
> +		void *value = ent->e_name + ent->e_name_len;
> +
> +		if (!name)
> +			continue;
> +
> +		switch (ent->e_name_index) {
> +		case F2FS_XATTR_INDEX_USER:
> +			ret = snprintf(xattr_name, F2FS_NAME_LEN, "%s%s",
> +				       XATTR_USER_PREFIX, name);
> +			break;
> +
> +		case F2FS_XATTR_INDEX_SECURITY:
> +			ret = snprintf(xattr_name, F2FS_NAME_LEN, "%s%s",
> +				       XATTR_SECURITY_PREFIX, name);
> +			break;
> +		case F2FS_XATTR_INDEX_TRUSTED:
> +			ret = snprintf(xattr_name, F2FS_NAME_LEN, "%s%s",
> +				       XATTR_TRUSTED_PREFIX, name);
> +			break;
> +		default:
> +			MSG(0, "Unknown xattr index 0x%x\n", ent->e_name_index);
> +			free(name);
> +			continue;
> +		}
> +		if (ret >= F2FS_NAME_LEN) {
> +			MSG(0, "XATTR index 0x%x name too long\n", ent->e_name_index);
> +			free(name);
> +			continue;
> +		}
> +
> +		DBG(1, "fd %d xattr_name %s\n", c.dump_fd, xattr_name);
> +		ret = fsetxattr(c.dump_fd, xattr_name, value,
> +				le16_to_cpu(ent->e_value_size), XATTR_CREATE);
> +		if (ret)

if (ret == -EEXIST)
	fsetxattr(,,,,XATTR_REPLACE)?

Thanks,

> +			MSG(0, "XATTR index 0x%x set xattr failed error %d\n",
> +			    ent->e_name_index, errno);
> +
> +		free(name);
> +	}
> +
> +	free(xattr);
> +}
> +
>  static void dump_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>  					struct f2fs_node *node_blk)
>  {
>  	u32 i = 0;
>  	u64 ofs = 0;
>  
> -	/* TODO: need to dump xattr */
> -
> -	if((node_blk->i.i_inline & F2FS_INLINE_DATA)){
> +	if((node_blk->i.i_inline & F2FS_INLINE_DATA)) {
>  		DBG(3, "ino[0x%x] has inline data!\n", nid);
>  		/* recover from inline data */
>  		dev_write_dump(((unsigned char *)node_blk) + INLINE_DATA_OFFSET,
> @@ -345,6 +400,8 @@ static void dump_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>  		else
>  			ASSERT(0);
>  	}
> +
> +	dump_xattr(sbi, node_blk);
>  }
>  
>  static void dump_file(struct f2fs_sb_info *sbi, struct node_info *ni,
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 5/8] dump.f2fs: do not dump encrypted files

[Chao Yu]

On 2017/10/31 9:38, Sheng Yong wrote:
> If a file is encrypted, its content is cipher text on the storage. So
> there is no need to dump an encrypted file.

IMO, if user have encryption key, it still has chance to read that file,
right? So how about keeping this functionality?

Thanks,

> 
> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
> ---
>  fsck/dump.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/fsck/dump.c b/fsck/dump.c
> index 01889fd..d11e8e2 100644
> --- a/fsck/dump.c
> +++ b/fsck/dump.c
> @@ -410,12 +410,17 @@ static void dump_file(struct f2fs_sb_info *sbi, struct node_info *ni,
>  	struct f2fs_inode *inode = &node_blk->i;
>  	u32 imode = le32_to_cpu(inode->i_mode);
>  	u32 namelen = le32_to_cpu(inode->i_namelen);
> -	unsigned char name[F2FS_NAME_LEN + 1] = {0};
> +	char name[F2FS_NAME_LEN + 1] = {0};
>  	char path[1024] = {0};
>  	char ans[255] = {0};
> -	int enc_name = file_enc_name(inode);
> +	int is_encrypted = file_is_encrypt(inode);
>  	int ret;
>  
> +	if (is_encrypted) {
> +		MSG(force, "File is encrypted\n");
> +		return;
> +	}
> +
>  	if (!S_ISREG(imode) || namelen == 0 || namelen > F2FS_NAME_LEN) {
>  		MSG(force, "Not a regular file or wrong name info\n\n");
>  		return;
> @@ -433,8 +438,7 @@ dump:
>  		ASSERT(ret >= 0);
>  
>  		/* make a file */
> -		namelen = convert_encrypted_name(inode->i_name, namelen,
> -							name, enc_name);
> +		strncpy(name, (const char *)inode->i_name, namelen);
>  		name[namelen] = 0;
>  		sprintf(path, "./lost_found/%s", name);
>  
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 6/8] fsck.f2fs: introduce new option --dry-run

[Chao Yu]

On 2017/10/31 9:38, Sheng Yong wrote:
> With --dry-run enabled, fsck.f2fs will do all checks and "fixes" except
> that all fixes will not be written to storage at last.
> 
> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
> ---
>  fsck/main.c       | 14 +++++++++++++-
>  include/f2fs_fs.h |  1 +
>  lib/libf2fs.c     |  1 +
>  lib/libf2fs_io.c  |  3 +++
>  4 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/fsck/main.c b/fsck/main.c
> index 93037e1..baa8efc 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -18,6 +18,7 @@
>  #include "fsck.h"
>  #include <libgen.h>
>  #include <ctype.h>
> +#include <getopt.h>
>  
>  struct f2fs_fsck gfsck;
>  
> @@ -30,6 +31,7 @@ void fsck_usage()
>  	MSG(0, "  -f check/fix entire partition\n");
>  	MSG(0, "  -p preen mode [default:0 the same as -a [0|1]]\n");
>  	MSG(0, "  -t show directory tree\n");
> +	MSG(0, "  --dry-run do not really fix corruptions\n");
>  	exit(1);
>  }
>  
> @@ -117,10 +119,20 @@ void f2fs_parse_options(int argc, char *argv[])
>  
>  	if (!strcmp("fsck.f2fs", prog)) {
>  		const char *option_string = ":ad:fp:t";
> +		int opt = 0;
> +		struct option long_opt[] = {
> +			{"dry-run", no_argument, 0, 1},
> +			{0, 0, 0, 0}
> +		};
>  
>  		c.func = FSCK;
> -		while ((option = getopt(argc, argv, option_string)) != EOF) {
> +		while ((option = getopt_long(argc, argv, option_string,
> +						long_opt, &opt)) != EOF) {
>  			switch (option) {
> +			case 1:
> +				c.dry_run = 1;
> +				MSG(0, "Info: Dry run\n");
> +				break;
>  			case 'a':
>  				c.auto_fix = 1;
>  				MSG(0, "Info: Fix the reported corruption.\n");
> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> index e1e299d..64a796d 100644
> --- a/include/f2fs_fs.h
> +++ b/include/f2fs_fs.h
> @@ -300,6 +300,7 @@ struct f2fs_configuration {
>  	int trimmed;
>  	int func;
>  	void *private;
> +	int dry_run;
>  	int fix_on;
>  	int bug_on;
>  	int auto_fix;
> diff --git a/lib/libf2fs.c b/lib/libf2fs.c
> index 69f5ccc..259bd17 100644
> --- a/lib/libf2fs.c
> +++ b/lib/libf2fs.c
> @@ -598,6 +598,7 @@ void f2fs_init_configuration(void)
>  	c.trimmed = 0;
>  	c.ro = 0;
>  	c.kd = -1;
> +	c.dry_run = 0;
>  }
>  
>  static int is_mounted(const char *mpt, const char *device)
> diff --git a/lib/libf2fs_io.c b/lib/libf2fs_io.c
> index 50ff171..8a79672 100644
> --- a/lib/libf2fs_io.c
> +++ b/lib/libf2fs_io.c
> @@ -129,6 +129,9 @@ int dev_write(void *buf, __u64 offset, size_t len)
>  {
>  	int fd;
>  
> +	if (c.dry_run)
> +		return 0;
> +
>  	if (c.sparse_mode)
>  		return dev_write_sparse(buf, offset, len);

Do we need to cover dev_write_block?

Thanks,

>  
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 7/8] fsck.f2fs: introduce sanity_check_inode

[Chao Yu]

On 2017/10/31 9:38, Sheng Yong wrote:
> f2fs_iget checks if i_mode is valid. If it is not, the file cannot be
> accessed as well as deleted. To make sure such files can be removed,
> fsck adds the same check, and removes incorrect inode blocks.
> 
> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
> ---
>  fsck/fsck.c  | 21 +++++++++++++++++++++
>  fsck/mount.c |  4 ++--
>  2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index 56a47be..77490d8 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -458,6 +458,25 @@ static int sanity_check_nid(struct f2fs_sb_info *sbi, u32 nid,
>  	return 0;
>  }
>  
> +static int sanity_check_inode(struct f2fs_sb_info *sbi, struct f2fs_node *node)
> +{
> +	struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
> +	struct f2fs_inode *fi = &node->i;
> +
> +	if (!(le16_to_cpu(fi->i_mode) & S_IFMT)) {
> +		ASSERT_MSG("i_mode is not valid. [0x%x]", le16_to_cpu(fi->i_mode));
> +		goto remove_node;
> +	}
> +
> +	return 0;
> +
> +remove_node:> +	f2fs_set_bit(le32_to_cpu(node->footer.ino), fsck->nat_area_bitmap);
> +	fsck->chk.valid_blk_cnt--;
> +	fsck->chk.valid_node_cnt--;
> +	return -EINVAL;
> +}
> +
>  static int fsck_chk_xattr_blk(struct f2fs_sb_info *sbi, u32 ino,
>  					u32 x_nid, u32 *blk_cnt)
>  {
> @@ -500,6 +519,8 @@ int fsck_chk_node_blk(struct f2fs_sb_info *sbi, struct f2fs_inode *inode,
>  		goto err;
>  
>  	if (ntype == TYPE_INODE) {
> +		if (sanity_check_inode(sbi, node_blk))
> +			goto err;
>  		fsck_chk_inode_blk(sbi, nid, ftype, node_blk, blk_cnt, &ni);
>  	} else {
>  		switch (ntype) {
> diff --git a/fsck/mount.c b/fsck/mount.c
> index a9cd581..45341bc 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -1985,7 +1985,7 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
>  		if (le32_to_cpu(nid_in_journal(journal, i)) == nid) {
>  			memset(&nat_in_journal(journal, i), 0,
>  					sizeof(struct f2fs_nat_entry));
> -			FIX_MSG("Remove nid [0x%x] in nat journal\n", nid);
> +			FIX_MSG("Remove nid [0x%x] in nat journal", nid);
>  			return;
>  		}
>  	}
> @@ -2005,7 +2005,7 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
>  	} else {
>  		memset(&nat_block->entries[entry_off], 0,
>  					sizeof(struct f2fs_nat_entry));
> -		FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
> +		FIX_MSG("Remove nid [0x%x] in NAT", nid);

How about moving all message printing modification into one patch?

Thanks,

>  	}
>  
>  	ret = dev_write_block(nat_block, block_addr);
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 8/8] f2fs-tools: remove unused list.h

[Chao Yu]

On 2017/10/31 9:38, Sheng Yong wrote:
> Commit 7d96d138a378 ("fsck.f2fs: remove list.h") has already the usage
> of list.h. So let's remove list.h itself.
> 
> Signed-off-by: Sheng Yong <shengyong1@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

> ---
>  include/list.h | 88 ----------------------------------------------------------
>  1 file changed, 88 deletions(-)
>  delete mode 100644 include/list.h
> 
> diff --git a/include/list.h b/include/list.h
> deleted file mode 100644
> index 571cd5c..0000000
> --- a/include/list.h
> +++ /dev/null
> @@ -1,88 +0,0 @@
> -
> -#define POISON_POINTER_DELTA 0
> -#define LIST_POISON1  ((void *) (0x00100100 + POISON_POINTER_DELTA))
> -#define LIST_POISON2  ((void *) (0x00200200 + POISON_POINTER_DELTA))
> -
> -#if !defined(offsetof)
> -#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
> -#endif
> -#define container_of(ptr, type, member) ({                      \
> -		const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
> -		(type *)( (char *)__mptr - offsetof(type,member) );})
> -
> -struct list_head {
> -	struct list_head *next, *prev;
> -};
> -
> -#define LIST_HEAD_INIT(name) { &(name), &(name) }
> -
> -#define LIST_HEAD(name) \
> -	struct list_head name = LIST_HEAD_INIT(name)
> -
> -static inline void INIT_LIST_HEAD(struct list_head *list)
> -{
> -	list->next = list;
> -	list->prev = list;
> -}
> -
> -static inline void __list_add(struct list_head *new,
> -		struct list_head *prev,
> -		struct list_head *next)
> -{
> -	next->prev = new;
> -	new->next = next;
> -	new->prev = prev;
> -	prev->next = new;
> -}
> -
> -static inline void list_add(struct list_head *new, struct list_head *head)
> -{
> -	__list_add(new, head, head->next);
> -}
> -
> -static inline void list_add_tail(struct list_head *new, struct list_head *head)
> -{
> -	__list_add(new, head->prev, head);
> -}
> -
> -static inline void __list_del(struct list_head * prev, struct list_head * next)
> -{
> -	next->prev = prev;
> -	prev->next = next;
> -}
> -
> -static inline void __list_del_entry(struct list_head *entry)
> -{
> -	__list_del(entry->prev, entry->next);
> -}
> -
> -static inline void list_del(struct list_head *entry)
> -{
> -	__list_del(entry->prev, entry->next);
> -	entry->next = LIST_POISON1;
> -	entry->prev = LIST_POISON2;
> -}
> -
> -static inline int list_empty(const struct list_head *head)
> -{
> -	return head->next == head;
> -}
> -
> -#define list_entry(ptr, type, member) \
> -	container_of(ptr, type, member)
> -
> -#define list_for_each(pos, head) \
> -	for (pos = (head)->next; pos != (head); pos = pos->next)
> -
> -#define list_for_each_safe(pos, n, head) \
> -	for (pos = (head)->next, n = pos->next; pos != (head); \
> -			pos = n, n = pos->next)
> -#define list_for_each_entry(pos, head, member)                          \
> -	for (pos = list_entry((head)->next, typeof(*pos), member);      \
> -			&pos->member != (head);    \
> -			pos = list_entry(pos->member.next, typeof(*pos), member))
> -#define list_for_each_entry_safe(pos, n, head, member)                  \
> -	for (pos = list_entry((head)->next, typeof(*pos), member),      \
> -			n = list_entry(pos->member.next, typeof(*pos), member); \
> -			&pos->member != (head);                                    \
> -			pos = n, n = list_entry(n->member.next, typeof(*n), member))
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 2/8] fsck.f2fs: do not fix corrupted nat entries in build_nat_area_bitmap

[Sheng Yong]

Hi, Chao

On 2017/10/31 15:20, Chao Yu wrote:
> On 2017/10/31 9:38, Sheng Yong wrote:
>> Fixing corrupted data depends on c.fix_on. If it's not set, we should not
>> force fixing corrupted data.  So if nat entries are found invalid when
>> building nat_area_bitmap, we should just set c.bug_on, and fix it later.
> 
> It looks comments and code change doesn't match?
In build_nat_area_bitmap(), if node/meta inode is invalid, the node block is
corrected and written back directly. The "write" should depend on c.fix_on.
So I think we should not fix the node block in build_nat_area_bitmap().

> do you mean
> 
> @@ -2031,7 +2032,6 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
>   				 */
>   				ASSERT_MSG("Invalid nat entry[0]: "
>   					"blk_addr[0x%x]\n", ni.blk_addr);
> -				c.fix_on = 1;
>   				fsck->chk.valid_nat_entry_cnt--;
>   			}
> 
> Thanks,
> 
>>
>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>> ---
>>   fsck/mount.c | 21 ++++++++++++++-------
>>   1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index faa9bc8..2d51678 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -1877,9 +1877,15 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
>>   	ret = dev_read_block(nat_block, block_addr);
>>   	ASSERT(ret >= 0);
>>   
>> -	memset(&nat_block->entries[entry_off], 0,
>> +	if (nid == F2FS_NODE_INO(sbi) || nid == F2FS_META_INO(sbi)) {
>> +		FIX_MSG("nid [0x%x] block_addr= 0x%x -> 0x1", nid,
>> +			le32_to_cpu(nat_block->entries[entry_off].block_addr));
>> +		nat_block->entries[entry_off].block_addr = cpu_to_le32(0x1);
>> +	} else {
>> +		memset(&nat_block->entries[entry_off], 0,
>>   					sizeof(struct f2fs_nat_entry));
>> -	FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
>> +		FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
>> +	}
>>   
>>   	ret = dev_write_block(nat_block, block_addr);
>>   	ASSERT(ret >= 0);
>> @@ -2001,13 +2007,14 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
>>   
>>   			if ((nid + i) == F2FS_NODE_INO(sbi) ||
>>   					(nid + i) == F2FS_META_INO(sbi)) {
>> -				/* block_addr of node/meta inode should be 0x1 */
>> +				/*
>> +				 * block_addr of node/meta inode should be 0x1.
>> +				 * Set this bit, and fsck_verify will fix it.
>> +				 */
>>   				if (le32_to_cpu(nat_block->entries[i].block_addr) != 0x1) {
>> -					FIX_MSG("ino: 0x%x node/meta inode, block_addr= 0x%x -> 0x1",
>> +					ASSERT_MSG("\tError: ino[0x%x] block_addr[0x%x] is invalid\n",
>>   							nid + i, le32_to_cpu(nat_block->entries[i].block_addr));
>> -					nat_block->entries[i].block_addr = cpu_to_le32(0x1);
>> -					ret = dev_write_block(nat_block, block_addr);
dev_write_block should depend on (c.fix_on == 1). ^^^^^^^^^^^
The corruption will be fixed in nullify_nat_entry().

thanks,
Sheng
>> -					ASSERT(ret >= 0);
>> +					f2fs_set_bit(nid + i, fsck->nat_area_bitmap);
>>   				}
>>   				continue;
>>   			}
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 3/8] dump/fsck: introduce print_xattr_entry

[Sheng Yong]

On 2017/10/31 15:51, Chao Yu wrote:
> On 2017/10/31 9:38, Sheng Yong wrote:
[...]
>> +
>> +void print_xattr_entry(struct f2fs_xattr_entry *ent)
>> +{
>> +	char *value = (char *)(ent->e_name + le16_to_cpu(ent->e_name_len));
>> +	struct fscrypt_context *ctx;
>> +	int i;
>> +
>> +	MSG(0, "\nxattr: e_name_index:%d e_name:", ent->e_name_index);
>> +	for (i = 0; i < le16_to_cpu(ent->e_name_len); i++)
>> +		MSG(0, "%c", ent->e_name[i]);
>> +	MSG(0, " e_name_len:%d e_value_size:%d e_value:\n",
>> +			ent->e_name_len, le16_to_cpu(ent->e_value_size));
>> +
>> +	switch (ent->e_name_index) {
>> +	case F2FS_XATTR_INDEX_USER:
>> +	case F2FS_XATTR_INDEX_SECURITY:
>> +		for (i = 0; i < le16_to_cpu(ent->e_value_size); i++)
>> +			MSG(0, "%c", value[i]);
> 
> It could be unreadable if user stores pure value in xattr entry.
Actually, I'm not quite sure about the value of each type of xattr. But does
USER xattr contain only plain text?

thanks,
Sheng
> 
> Thanks,
> 
>> +		MSG(0, "\n");
>> +		break;
>> +	case F2FS_XATTR_INDEX_POSIX_ACL_ACCESS:
>> +	case F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT:
>> +		print_acl(value, le16_to_cpu(ent->e_value_size));
>> +		break;
>> +	case F2FS_XATTR_INDEX_TRUSTED:
>> +	case F2FS_XATTR_INDEX_LUSTRE:
>> +		for (i = 0; i < le16_to_cpu(ent->e_value_size); i++)
>> +			MSG(0, "%02X", value[i]);
>> +		MSG(0, "\n");
>> +		break;
>> +	case F2FS_XATTR_INDEX_ENCRYPTION:
>> +		ctx = (struct fscrypt_context *)value;
>> +		MSG(0, "format: %d\n", ctx->format);
>> +		MSG(0, "contents_encryption_mode: 0x%x\n", ctx->contents_encryption_mode);
>> +		MSG(0, "filenames_encryption_mode: 0x%x\n", ctx->filenames_encryption_mode);
>> +		MSG(0, "flags: 0x%x\n", ctx->flags);
>> +		MSG(0, "master_key_descriptor: ");
>> +		for (i = 0; i < FS_KEY_DESCRIPTOR_SIZE; i++)
>> +			MSG(0, "%02X", ctx->master_key_descriptor[i]);
>> +		MSG(0, "\nnonce: ");
>> +		for (i = 0; i < FS_KEY_DERIVATION_NONCE_SIZE; i++)
>> +			MSG(0, "%02X", ctx->nonce[i]);
>> +		MSG(0, "\n");
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +}
>> +
>> +void print_inode_info(struct f2fs_sb_info *sbi,
>> +			struct f2fs_node *node, int name)
>> +{
>> +	struct f2fs_inode *inode = &node->i;
>> +	void *xattr_addr;
>> +	struct f2fs_xattr_entry *ent;
>>   	unsigned char en[F2FS_NAME_LEN + 1];
>>   	unsigned int i = 0;
>>   	int namelen = le32_to_cpu(inode->i_namelen);
>> @@ -111,17 +225,24 @@ void print_inode_info(struct f2fs_inode *inode, int name)
>>   	DISP_u32(inode, i_nid[3]);	/* indirect */
>>   	DISP_u32(inode, i_nid[4]);	/* double indirect */
>>   
>> +	xattr_addr = read_all_xattrs(sbi, node);
>> +	list_for_each_xattr(ent, xattr_addr) {
>> +		print_xattr_entry(ent);
>> +	}
>> +	free(xattr_addr);
>> +
>>   	printf("\n");
>>   }
>>   
>> -void print_node_info(struct f2fs_node *node_block, int verbose)
>> +void print_node_info(struct f2fs_sb_info *sbi,
>> +			struct f2fs_node *node_block, int verbose)
>>   {
>>   	nid_t ino = le32_to_cpu(node_block->footer.ino);
>>   	nid_t nid = le32_to_cpu(node_block->footer.nid);
>>   	/* Is this inode? */
>>   	if (ino == nid) {
>>   		DBG(verbose, "Node ID [0x%x:%u] is inode\n", nid, nid);
>> -		print_inode_info(&node_block->i, verbose);
>> +		print_inode_info(sbi, node_block, verbose);
>>   	} else {
>>   		int i;
>>   		u32 *dump_blk = (u32 *)node_block;
>> diff --git a/fsck/xattr.c b/fsck/xattr.c
>> index 3f5c7d3..1d0f7d3 100644
>> --- a/fsck/xattr.c
>> +++ b/fsck/xattr.c
>> @@ -20,7 +20,7 @@
>>   #define XATTR_CREATE 0x1
>>   #define XATTR_REPLACE 0x2
>>   
>> -static void *read_all_xattrs(struct f2fs_sb_info *sbi, struct f2fs_node *inode)
>> +void *read_all_xattrs(struct f2fs_sb_info *sbi, struct f2fs_node *inode)
>>   {
>>   	struct f2fs_xattr_header *header;
>>   	void *txattr_addr;
>> diff --git a/fsck/xattr.h b/fsck/xattr.h
>> index b414629..beed3bb 100644
>> --- a/fsck/xattr.h
>> +++ b/fsck/xattr.h
>> @@ -31,10 +31,62 @@ struct f2fs_xattr_entry {
>>   	char e_name[0];		/* attribute name */
>>   };
>>   
>> +#define FS_KEY_DESCRIPTOR_SIZE 8
>> +#define FS_KEY_DERIVATION_NONCE_SIZE 16
>> +
>> +struct fscrypt_context {
>> +	u8 format;
>> +	u8 contents_encryption_mode;
>> +	u8 filenames_encryption_mode;
>> +	u8 flags;
>> +	u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
>> +	u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
>> +} __attribute__((packed));
>> +
>> +#define F2FS_ACL_VERSION	0x0001
>> +
>> +struct f2fs_acl_entry {
>> +	__le16 e_tag;
>> +	__le16 e_perm;
>> +	__le32 e_id;
>> +};
>> +
>> +struct f2fs_acl_entry_short {
>> +	__le16 e_tag;
>> +	__le16 e_perm;
>> +};
>> +
>> +struct f2fs_acl_header {
>> +	__le32 a_version;
>> +};
>> +
>> +static inline int f2fs_acl_count(int size)
>> +{
>> +	ssize_t s;
>> +	size -= sizeof(struct f2fs_acl_header);
>> +	s = size - 4 * sizeof(struct f2fs_acl_entry_short);
>> +	if (s < 0) {
>> +		if (size % sizeof(struct f2fs_acl_entry_short))
>> +			return -1;
>> +		return size / sizeof(struct f2fs_acl_entry_short);
>> +	} else {
>> +		if (s % sizeof(struct f2fs_acl_entry))
>> +			return -1;
>> +		return s / sizeof(struct f2fs_acl_entry) + 4;
>> +	}
>> +}
>> +
>>   #define XATTR_ROUND	(3)
>>   
>>   #define XATTR_SELINUX_SUFFIX "selinux"
>> -#define F2FS_XATTR_INDEX_SECURITY	6
>> +#define F2FS_XATTR_INDEX_USER			1
>> +#define F2FS_XATTR_INDEX_POSIX_ACL_ACCESS	2
>> +#define F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT	3
>> +#define F2FS_XATTR_INDEX_TRUSTED		4
>> +#define F2FS_XATTR_INDEX_LUSTRE			5
>> +#define F2FS_XATTR_INDEX_SECURITY		6
>> +#define F2FS_XATTR_INDEX_ENCRYPTION		9
>> +
>>   #define IS_XATTR_LAST_ENTRY(entry) (*(__u32 *)(entry) == 0)
>>   
>>   #define XATTR_HDR(ptr)		((struct f2fs_xattr_header *)(ptr))
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 4/8] dump.f2fs: introduce dump_xattr

[Sheng Yong]

On 2017/10/31 15:51, Chao Yu wrote:
> On 2017/10/31 9:38, Sheng Yong wrote:
>> This patch introduces dump_xattr to create xattrs for dumped files.
>>
>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>> ---
>>   fsck/dump.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 60 insertions(+), 3 deletions(-)
>>
>> diff --git a/fsck/dump.c b/fsck/dump.c
>> index 90fd073..01889fd 100644
>> --- a/fsck/dump.c
>> +++ b/fsck/dump.c
>> @@ -11,6 +11,9 @@
>>   #include <inttypes.h>
>>   
>>   #include "fsck.h"
>> +#include "xattr.h"
>> +#include <attr/xattr.h>
>> +#include <linux/xattr.h>
>>   #include <locale.h>
>>   
>>   #define BUF_SZ	80
>> @@ -310,15 +313,67 @@ static void dump_node_blk(struct f2fs_sb_info *sbi, int ntype,
>>   	free(node_blk);
>>   }
>>   
[...]
>> +		DBG(1, "fd %d xattr_name %s\n", c.dump_fd, xattr_name);
>> +		ret = fsetxattr(c.dump_fd, xattr_name, value,
>> +				le16_to_cpu(ent->e_value_size), XATTR_CREATE);
>> +		if (ret)
> 
> if (ret == -EEXIST)
> 	fsetxattr(,,,,XATTR_REPLACE)?
Hmm, there should be no 2 xattrs with the same name in one file， right? :)

thanks,
Sheng
> 
> Thanks,
> 
>> +			MSG(0, "XATTR index 0x%x set xattr failed error %d\n",
>> +			    ent->e_name_index, errno);
>> +
>> +		free(name);
>> +	}
>> +
>> +	free(xattr);
>> +}
>> +
>>   static void dump_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>   					struct f2fs_node *node_blk)
>>   {
>>   	u32 i = 0;
>>   	u64 ofs = 0;
>>   
>> -	/* TODO: need to dump xattr */
>> -
>> -	if((node_blk->i.i_inline & F2FS_INLINE_DATA)){
>> +	if((node_blk->i.i_inline & F2FS_INLINE_DATA)) {
>>   		DBG(3, "ino[0x%x] has inline data!\n", nid);
>>   		/* recover from inline data */
>>   		dev_write_dump(((unsigned char *)node_blk) + INLINE_DATA_OFFSET,
>> @@ -345,6 +400,8 @@ static void dump_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>   		else
>>   			ASSERT(0);
>>   	}
>> +
>> +	dump_xattr(sbi, node_blk);
>>   }
>>   
>>   static void dump_file(struct f2fs_sb_info *sbi, struct node_info *ni,
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [RFC PATCH 5/8] dump.f2fs: do not dump encrypted files

[Sheng Yong]

On 2017/10/31 15:57, Chao Yu wrote:
> On 2017/10/31 9:38, Sheng Yong wrote:
>> If a file is encrypted, its content is cipher text on the storage. So
>> there is no need to dump an encrypted file.
> 
> IMO, if user have encryption key, it still has chance to read that file,
> right? So how about keeping this functionality?
Nope. When we dump an encrypted file, we only get the cipher data. If we want
to decrypt the data, we must provide a method to writeback the data directly
to the device. Then we get a chance to get the correct plain data. Otherwise,
if we simply copy the dumped file to filesystem, it doesn't work.

thanks,
Sheng
> 
> Thanks,
> 
>>
>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>> ---
>>   fsck/dump.c | 12 ++++++++----
>>   1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/fsck/dump.c b/fsck/dump.c
>> index 01889fd..d11e8e2 100644
>> --- a/fsck/dump.c
>> +++ b/fsck/dump.c
>> @@ -410,12 +410,17 @@ static void dump_file(struct f2fs_sb_info *sbi, struct node_info *ni,
>>   	struct f2fs_inode *inode = &node_blk->i;
>>   	u32 imode = le32_to_cpu(inode->i_mode);
>>   	u32 namelen = le32_to_cpu(inode->i_namelen);
>> -	unsigned char name[F2FS_NAME_LEN + 1] = {0};
>> +	char name[F2FS_NAME_LEN + 1] = {0};
>>   	char path[1024] = {0};
>>   	char ans[255] = {0};
>> -	int enc_name = file_enc_name(inode);
>> +	int is_encrypted = file_is_encrypt(inode);
>>   	int ret;
>>   
>> +	if (is_encrypted) {
>> +		MSG(force, "File is encrypted\n");
>> +		return;
>> +	}
>> +
>>   	if (!S_ISREG(imode) || namelen == 0 || namelen > F2FS_NAME_LEN) {
>>   		MSG(force, "Not a regular file or wrong name info\n\n");
>>   		return;
>> @@ -433,8 +438,7 @@ dump:
>>   		ASSERT(ret >= 0);
>>   
>>   		/* make a file */
>> -		namelen = convert_encrypted_name(inode->i_name, namelen,
>> -							name, enc_name);
>> +		strncpy(name, (const char *)inode->i_name, namelen);
>>   		name[namelen] = 0;
>>   		sprintf(path, "./lost_found/%s", name);
>>   
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 6/8] fsck.f2fs: introduce new option --dry-run

[Sheng Yong]

On 2017/10/31 15:57, Chao Yu wrote:
> On 2017/10/31 9:38, Sheng Yong wrote:
>> With --dry-run enabled, fsck.f2fs will do all checks and "fixes" except
>> that all fixes will not be written to storage at last.
>>
>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>> ---
>>   fsck/main.c       | 14 +++++++++++++-
>>   include/f2fs_fs.h |  1 +
>>   lib/libf2fs.c     |  1 +
>>   lib/libf2fs_io.c  |  3 +++
>>   4 files changed, 18 insertions(+), 1 deletion(-)
>>
>>   
[...]
>>   static int is_mounted(const char *mpt, const char *device)
>> diff --git a/lib/libf2fs_io.c b/lib/libf2fs_io.c
>> index 50ff171..8a79672 100644
>> --- a/lib/libf2fs_io.c
>> +++ b/lib/libf2fs_io.c
>> @@ -129,6 +129,9 @@ int dev_write(void *buf, __u64 offset, size_t len)
>>   {
>>   	int fd;
>>   
>> +	if (c.dry_run)
>> +		return 0;
>> +
>>   	if (c.sparse_mode)
>>   		return dev_write_sparse(buf, offset, len);
> 
> Do we need to cover dev_write_block?
dev_write_block calls dev_write. So dev_write_block is also covered by intercepting
write in dev_write. :)

Thanks,
Sheng
> 
> Thanks,
> 
>>   
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 7/8] fsck.f2fs: introduce sanity_check_inode

[Sheng Yong]

On 2017/10/31 16:04, Chao Yu wrote:
> On 2017/10/31 9:38, Sheng Yong wrote:
>> f2fs_iget checks if i_mode is valid. If it is not, the file cannot be
>> accessed as well as deleted. To make sure such files can be removed,
>> fsck adds the same check, and removes incorrect inode blocks.
>>
>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>> ---
>>   fsck/fsck.c  | 21 +++++++++++++++++++++
>>   fsck/mount.c |  4 ++--
>>   2 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/fsck/fsck.c b/fsck/fsck.c
>> index 56a47be..77490d8 100644
>> --- a/fsck/fsck.c
>> +++ b/fsck/fsck.c
>> @@ -458,6 +458,25 @@ static int sanity_check_nid(struct f2fs_sb_info *sbi, u32 nid,
>>   	return 0;
>>   }
>>   
>> +static int sanity_check_inode(struct f2fs_sb_info *sbi, struct f2fs_node *node)
>> +{
>> +	struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
>> +	struct f2fs_inode *fi = &node->i;
>> +
>> +	if (!(le16_to_cpu(fi->i_mode) & S_IFMT)) {
>> +		ASSERT_MSG("i_mode is not valid. [0x%x]", le16_to_cpu(fi->i_mode));
>> +		goto remove_node;
>> +	}
>> +
>> +	return 0;
>> +
>> +remove_node:> +	f2fs_set_bit(le32_to_cpu(node->footer.ino), fsck->nat_area_bitmap);
>> +	fsck->chk.valid_blk_cnt--;
>> +	fsck->chk.valid_node_cnt--;
>> +	return -EINVAL;
>> +}
>> +
>>   static int fsck_chk_xattr_blk(struct f2fs_sb_info *sbi, u32 ino,
>>   					u32 x_nid, u32 *blk_cnt)
>>   {
>> @@ -500,6 +519,8 @@ int fsck_chk_node_blk(struct f2fs_sb_info *sbi, struct f2fs_inode *inode,
>>   		goto err;
>>   
>>   	if (ntype == TYPE_INODE) {
>> +		if (sanity_check_inode(sbi, node_blk))
>> +			goto err;
>>   		fsck_chk_inode_blk(sbi, nid, ftype, node_blk, blk_cnt, &ni);
>>   	} else {
>>   		switch (ntype) {
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index a9cd581..45341bc 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -1985,7 +1985,7 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
>>   		if (le32_to_cpu(nid_in_journal(journal, i)) == nid) {
>>   			memset(&nat_in_journal(journal, i), 0,
>>   					sizeof(struct f2fs_nat_entry));
>> -			FIX_MSG("Remove nid [0x%x] in nat journal\n", nid);
>> +			FIX_MSG("Remove nid [0x%x] in nat journal", nid);
>>   			return;
>>   		}
>>   	}
>> @@ -2005,7 +2005,7 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
>>   	} else {
>>   		memset(&nat_block->entries[entry_off], 0,
>>   					sizeof(struct f2fs_nat_entry));
>> -		FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
>> +		FIX_MSG("Remove nid [0x%x] in NAT", nid);
> 
> How about moving all message printing modification into one patch?
OK. I'll split these changes into different patches :)

thanks,
Sheng
> 
> Thanks,
> 
>>   	}
>>   
>>   	ret = dev_write_block(nat_block, block_addr);
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 1/8] fsck.f2fs: do not set fix_on directly

[Sheng Yong]

On 2017/10/31 15:06, Chao Yu wrote:
> On 2017/10/31 9:38, Sheng Yong wrote:
>> Do not set fix_on if it is allowed by user.
>>
>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>> ---
>>   fsck/main.c  | 2 +-
>>   fsck/mount.c | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fsck/main.c b/fsck/main.c
>> index c9411eb..93037e1 100644
>> --- a/fsck/main.c
>> +++ b/fsck/main.c
>> @@ -429,7 +429,7 @@ static void do_fsck(struct f2fs_sb_info *sbi)
>>   				c.fix_on = 1;
>>   			break;
>>   		}
>> -	} else {
>> +	} else if (c.preen_mode) {
> 
> Needs to update below comments?
I think the new condition hits comment 3 ("fsck -p 1 && error is detected, then
bug_on is set fix_on = 1 here, so that fsck can fix errors automatically"). The
condition was missing and the comment seems not that correct before ;)

thanks,
Sheng
> 
>>   		/*
>>   		 * we can hit this in 3 situations:
>>   		 *  1. fsck -f, fix_on has already been set to 1 when
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index 29af3b7..faa9bc8 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -1879,6 +1879,7 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
>>   
>>   	memset(&nat_block->entries[entry_off], 0,
>>   					sizeof(struct f2fs_nat_entry));
>> +	FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
> 
> How about adding debug message in separated patch?
> 
> Thanks,
> 
>>   
>>   	ret = dev_write_block(nat_block, block_addr);
>>   	ASSERT(ret >= 0);
>> @@ -2031,7 +2032,6 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
>>   				 */
>>   				ASSERT_MSG("Invalid nat entry[0]: "
>>   					"blk_addr[0x%x]\n", ni.blk_addr);
>> -				c.fix_on = 1;
>>   				fsck->chk.valid_nat_entry_cnt--;
>>   			}
>>   
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 2/8] fsck.f2fs: do not fix corrupted nat entries in build_nat_area_bitmap

[Chao Yu]

On 2017/10/31 16:57, Sheng Yong wrote:
> Hi, Chao
> 
> On 2017/10/31 15:20, Chao Yu wrote:
>> On 2017/10/31 9:38, Sheng Yong wrote:
>>> Fixing corrupted data depends on c.fix_on. If it's not set, we should not
>>> force fixing corrupted data.  So if nat entries are found invalid when
>>> building nat_area_bitmap, we should just set c.bug_on, and fix it later.
>>
>> It looks comments and code change doesn't match?
> In build_nat_area_bitmap(), if node/meta inode is invalid, the node block is
> corrected and written back directly. The "write" should depend on c.fix_on.
> So I think we should not fix the node block in build_nat_area_bitmap().

Oh, I got it, sorry for misunderstanding. ;)

Reviewed-by: Chao Yu <yuchao0@huawei.com>

> 
>> do you mean
>>
>> @@ -2031,7 +2032,6 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
>>   				 */
>>   				ASSERT_MSG("Invalid nat entry[0]: "
>>   					"blk_addr[0x%x]\n", ni.blk_addr);
>> -				c.fix_on = 1;
>>   				fsck->chk.valid_nat_entry_cnt--;
>>   			}
>>
>> Thanks,
>>
>>>
>>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>>> ---
>>>   fsck/mount.c | 21 ++++++++++++++-------
>>>   1 file changed, 14 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/fsck/mount.c b/fsck/mount.c
>>> index faa9bc8..2d51678 100644
>>> --- a/fsck/mount.c
>>> +++ b/fsck/mount.c
>>> @@ -1877,9 +1877,15 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
>>>   	ret = dev_read_block(nat_block, block_addr);
>>>   	ASSERT(ret >= 0);
>>>   
>>> -	memset(&nat_block->entries[entry_off], 0,
>>> +	if (nid == F2FS_NODE_INO(sbi) || nid == F2FS_META_INO(sbi)) {
>>> +		FIX_MSG("nid [0x%x] block_addr= 0x%x -> 0x1", nid,
>>> +			le32_to_cpu(nat_block->entries[entry_off].block_addr));
>>> +		nat_block->entries[entry_off].block_addr = cpu_to_le32(0x1);
>>> +	} else {
>>> +		memset(&nat_block->entries[entry_off], 0,
>>>   					sizeof(struct f2fs_nat_entry));
>>> -	FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
>>> +		FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
>>> +	}
>>>   
>>>   	ret = dev_write_block(nat_block, block_addr);
>>>   	ASSERT(ret >= 0);
>>> @@ -2001,13 +2007,14 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
>>>   
>>>   			if ((nid + i) == F2FS_NODE_INO(sbi) ||
>>>   					(nid + i) == F2FS_META_INO(sbi)) {
>>> -				/* block_addr of node/meta inode should be 0x1 */
>>> +				/*
>>> +				 * block_addr of node/meta inode should be 0x1.
>>> +				 * Set this bit, and fsck_verify will fix it.
>>> +				 */
>>>   				if (le32_to_cpu(nat_block->entries[i].block_addr) != 0x1) {
>>> -					FIX_MSG("ino: 0x%x node/meta inode, block_addr= 0x%x -> 0x1",
>>> +					ASSERT_MSG("\tError: ino[0x%x] block_addr[0x%x] is invalid\n",
>>>   							nid + i, le32_to_cpu(nat_block->entries[i].block_addr));
>>> -					nat_block->entries[i].block_addr = cpu_to_le32(0x1);
>>> -					ret = dev_write_block(nat_block, block_addr);
> dev_write_block should depend on (c.fix_on == 1). ^^^^^^^^^^^
> The corruption will be fixed in nullify_nat_entry().
> 
> thanks,
> Sheng
>>> -					ASSERT(ret >= 0);
>>> +					f2fs_set_bit(nid + i, fsck->nat_area_bitmap);
>>>   				}
>>>   				continue;
>>>   			}
>>>
>>
>>
>> .
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 3/8] dump/fsck: introduce print_xattr_entry

[Chao Yu]

On 2017/10/31 17:02, Sheng Yong wrote:
> 
> 
> On 2017/10/31 15:51, Chao Yu wrote:
>> On 2017/10/31 9:38, Sheng Yong wrote:
> [...]
>>> +
>>> +void print_xattr_entry(struct f2fs_xattr_entry *ent)
>>> +{
>>> +	char *value = (char *)(ent->e_name + le16_to_cpu(ent->e_name_len));
>>> +	struct fscrypt_context *ctx;
>>> +	int i;
>>> +
>>> +	MSG(0, "\nxattr: e_name_index:%d e_name:", ent->e_name_index);
>>> +	for (i = 0; i < le16_to_cpu(ent->e_name_len); i++)
>>> +		MSG(0, "%c", ent->e_name[i]);
>>> +	MSG(0, " e_name_len:%d e_value_size:%d e_value:\n",
>>> +			ent->e_name_len, le16_to_cpu(ent->e_value_size));
>>> +
>>> +	switch (ent->e_name_index) {
>>> +	case F2FS_XATTR_INDEX_USER:
>>> +	case F2FS_XATTR_INDEX_SECURITY:
>>> +		for (i = 0; i < le16_to_cpu(ent->e_value_size); i++)
>>> +			MSG(0, "%c", value[i]);
>>
>> It could be unreadable if user stores pure value in xattr entry.
> Actually, I'm not quite sure about the value of each type of xattr. But does
> USER xattr contain only plain text?

IIRC, there is no such restriction, value can be any type, with variable length.

Thanks,

> 
> thanks,
> Sheng
>>
>> Thanks,
>>
>>> +		MSG(0, "\n");
>>> +		break;
>>> +	case F2FS_XATTR_INDEX_POSIX_ACL_ACCESS:
>>> +	case F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT:
>>> +		print_acl(value, le16_to_cpu(ent->e_value_size));
>>> +		break;
>>> +	case F2FS_XATTR_INDEX_TRUSTED:
>>> +	case F2FS_XATTR_INDEX_LUSTRE:
>>> +		for (i = 0; i < le16_to_cpu(ent->e_value_size); i++)
>>> +			MSG(0, "%02X", value[i]);
>>> +		MSG(0, "\n");
>>> +		break;
>>> +	case F2FS_XATTR_INDEX_ENCRYPTION:
>>> +		ctx = (struct fscrypt_context *)value;
>>> +		MSG(0, "format: %d\n", ctx->format);
>>> +		MSG(0, "contents_encryption_mode: 0x%x\n", ctx->contents_encryption_mode);
>>> +		MSG(0, "filenames_encryption_mode: 0x%x\n", ctx->filenames_encryption_mode);
>>> +		MSG(0, "flags: 0x%x\n", ctx->flags);
>>> +		MSG(0, "master_key_descriptor: ");
>>> +		for (i = 0; i < FS_KEY_DESCRIPTOR_SIZE; i++)
>>> +			MSG(0, "%02X", ctx->master_key_descriptor[i]);
>>> +		MSG(0, "\nnonce: ");
>>> +		for (i = 0; i < FS_KEY_DERIVATION_NONCE_SIZE; i++)
>>> +			MSG(0, "%02X", ctx->nonce[i]);
>>> +		MSG(0, "\n");
>>> +		break;
>>> +	default:
>>> +		break;
>>> +	}
>>> +}
>>> +
>>> +void print_inode_info(struct f2fs_sb_info *sbi,
>>> +			struct f2fs_node *node, int name)
>>> +{
>>> +	struct f2fs_inode *inode = &node->i;
>>> +	void *xattr_addr;
>>> +	struct f2fs_xattr_entry *ent;
>>>   	unsigned char en[F2FS_NAME_LEN + 1];
>>>   	unsigned int i = 0;
>>>   	int namelen = le32_to_cpu(inode->i_namelen);
>>> @@ -111,17 +225,24 @@ void print_inode_info(struct f2fs_inode *inode, int name)
>>>   	DISP_u32(inode, i_nid[3]);	/* indirect */
>>>   	DISP_u32(inode, i_nid[4]);	/* double indirect */
>>>   
>>> +	xattr_addr = read_all_xattrs(sbi, node);
>>> +	list_for_each_xattr(ent, xattr_addr) {
>>> +		print_xattr_entry(ent);
>>> +	}
>>> +	free(xattr_addr);
>>> +
>>>   	printf("\n");
>>>   }
>>>   
>>> -void print_node_info(struct f2fs_node *node_block, int verbose)
>>> +void print_node_info(struct f2fs_sb_info *sbi,
>>> +			struct f2fs_node *node_block, int verbose)
>>>   {
>>>   	nid_t ino = le32_to_cpu(node_block->footer.ino);
>>>   	nid_t nid = le32_to_cpu(node_block->footer.nid);
>>>   	/* Is this inode? */
>>>   	if (ino == nid) {
>>>   		DBG(verbose, "Node ID [0x%x:%u] is inode\n", nid, nid);
>>> -		print_inode_info(&node_block->i, verbose);
>>> +		print_inode_info(sbi, node_block, verbose);
>>>   	} else {
>>>   		int i;
>>>   		u32 *dump_blk = (u32 *)node_block;
>>> diff --git a/fsck/xattr.c b/fsck/xattr.c
>>> index 3f5c7d3..1d0f7d3 100644
>>> --- a/fsck/xattr.c
>>> +++ b/fsck/xattr.c
>>> @@ -20,7 +20,7 @@
>>>   #define XATTR_CREATE 0x1
>>>   #define XATTR_REPLACE 0x2
>>>   
>>> -static void *read_all_xattrs(struct f2fs_sb_info *sbi, struct f2fs_node *inode)
>>> +void *read_all_xattrs(struct f2fs_sb_info *sbi, struct f2fs_node *inode)
>>>   {
>>>   	struct f2fs_xattr_header *header;
>>>   	void *txattr_addr;
>>> diff --git a/fsck/xattr.h b/fsck/xattr.h
>>> index b414629..beed3bb 100644
>>> --- a/fsck/xattr.h
>>> +++ b/fsck/xattr.h
>>> @@ -31,10 +31,62 @@ struct f2fs_xattr_entry {
>>>   	char e_name[0];		/* attribute name */
>>>   };
>>>   
>>> +#define FS_KEY_DESCRIPTOR_SIZE 8
>>> +#define FS_KEY_DERIVATION_NONCE_SIZE 16
>>> +
>>> +struct fscrypt_context {
>>> +	u8 format;
>>> +	u8 contents_encryption_mode;
>>> +	u8 filenames_encryption_mode;
>>> +	u8 flags;
>>> +	u8 master_key_descriptor[FS_KEY_DESCRIPTOR_SIZE];
>>> +	u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
>>> +} __attribute__((packed));
>>> +
>>> +#define F2FS_ACL_VERSION	0x0001
>>> +
>>> +struct f2fs_acl_entry {
>>> +	__le16 e_tag;
>>> +	__le16 e_perm;
>>> +	__le32 e_id;
>>> +};
>>> +
>>> +struct f2fs_acl_entry_short {
>>> +	__le16 e_tag;
>>> +	__le16 e_perm;
>>> +};
>>> +
>>> +struct f2fs_acl_header {
>>> +	__le32 a_version;
>>> +};
>>> +
>>> +static inline int f2fs_acl_count(int size)
>>> +{
>>> +	ssize_t s;
>>> +	size -= sizeof(struct f2fs_acl_header);
>>> +	s = size - 4 * sizeof(struct f2fs_acl_entry_short);
>>> +	if (s < 0) {
>>> +		if (size % sizeof(struct f2fs_acl_entry_short))
>>> +			return -1;
>>> +		return size / sizeof(struct f2fs_acl_entry_short);
>>> +	} else {
>>> +		if (s % sizeof(struct f2fs_acl_entry))
>>> +			return -1;
>>> +		return s / sizeof(struct f2fs_acl_entry) + 4;
>>> +	}
>>> +}
>>> +
>>>   #define XATTR_ROUND	(3)
>>>   
>>>   #define XATTR_SELINUX_SUFFIX "selinux"
>>> -#define F2FS_XATTR_INDEX_SECURITY	6
>>> +#define F2FS_XATTR_INDEX_USER			1
>>> +#define F2FS_XATTR_INDEX_POSIX_ACL_ACCESS	2
>>> +#define F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT	3
>>> +#define F2FS_XATTR_INDEX_TRUSTED		4
>>> +#define F2FS_XATTR_INDEX_LUSTRE			5
>>> +#define F2FS_XATTR_INDEX_SECURITY		6
>>> +#define F2FS_XATTR_INDEX_ENCRYPTION		9
>>> +
>>>   #define IS_XATTR_LAST_ENTRY(entry) (*(__u32 *)(entry) == 0)
>>>   
>>>   #define XATTR_HDR(ptr)		((struct f2fs_xattr_header *)(ptr))
>>>
>>
>>
>> .
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 4/8] dump.f2fs: introduce dump_xattr

[Chao Yu]

On 2017/10/31 17:11, Sheng Yong wrote:
> 
> 
> On 2017/10/31 15:51, Chao Yu wrote:
>> On 2017/10/31 9:38, Sheng Yong wrote:
>>> This patch introduces dump_xattr to create xattrs for dumped files.
>>>
>>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>>> ---
>>>   fsck/dump.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>>   1 file changed, 60 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fsck/dump.c b/fsck/dump.c
>>> index 90fd073..01889fd 100644
>>> --- a/fsck/dump.c
>>> +++ b/fsck/dump.c
>>> @@ -11,6 +11,9 @@
>>>   #include <inttypes.h>
>>>   
>>>   #include "fsck.h"
>>> +#include "xattr.h"
>>> +#include <attr/xattr.h>
>>> +#include <linux/xattr.h>
>>>   #include <locale.h>
>>>   
>>>   #define BUF_SZ	80
>>> @@ -310,15 +313,67 @@ static void dump_node_blk(struct f2fs_sb_info *sbi, int ntype,
>>>   	free(node_blk);
>>>   }
>>>   
> [...]
>>> +		DBG(1, "fd %d xattr_name %s\n", c.dump_fd, xattr_name);
>>> +		ret = fsetxattr(c.dump_fd, xattr_name, value,
>>> +				le16_to_cpu(ent->e_value_size), XATTR_CREATE);
>>> +		if (ret)
>>
>> if (ret == -EEXIST)
>> 	fsetxattr(,,,,XATTR_REPLACE)?
> Hmm, there should be no 2 xattrs with the same name in one file， right? :)

I'm not sure, maybe we should handle this in this patch to avoid missing this
condition.

Thanks,

> 
> thanks,
> Sheng
>>
>> Thanks,
>>
>>> +			MSG(0, "XATTR index 0x%x set xattr failed error %d\n",
>>> +			    ent->e_name_index, errno);
>>> +
>>> +		free(name);
>>> +	}
>>> +
>>> +	free(xattr);
>>> +}
>>> +
>>>   static void dump_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>>   					struct f2fs_node *node_blk)
>>>   {
>>>   	u32 i = 0;
>>>   	u64 ofs = 0;
>>>   
>>> -	/* TODO: need to dump xattr */
>>> -
>>> -	if((node_blk->i.i_inline & F2FS_INLINE_DATA)){
>>> +	if((node_blk->i.i_inline & F2FS_INLINE_DATA)) {
>>>   		DBG(3, "ino[0x%x] has inline data!\n", nid);
>>>   		/* recover from inline data */
>>>   		dev_write_dump(((unsigned char *)node_blk) + INLINE_DATA_OFFSET,
>>> @@ -345,6 +400,8 @@ static void dump_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>>   		else
>>>   			ASSERT(0);
>>>   	}
>>> +
>>> +	dump_xattr(sbi, node_blk);
>>>   }
>>>   
>>>   static void dump_file(struct f2fs_sb_info *sbi, struct node_info *ni,
>>>
>>
>>
>> .
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [RFC PATCH 5/8] dump.f2fs: do not dump encrypted files

[Chao Yu]

On 2017/10/31 17:16, Sheng Yong wrote:
> 
> 
> On 2017/10/31 15:57, Chao Yu wrote:
>> On 2017/10/31 9:38, Sheng Yong wrote:
>>> If a file is encrypted, its content is cipher text on the storage. So
>>> there is no need to dump an encrypted file.
>>
>> IMO, if user have encryption key, it still has chance to read that file,
>> right? So how about keeping this functionality?
> Nope. When we dump an encrypted file, we only get the cipher data. If we want
> to decrypt the data, we must provide a method to writeback the data directly> to the device. Then we get a chance to get the correct plain data. Otherwise,
> if we simply copy the dumped file to filesystem, it doesn't work.

I just thought we can provider the cipher data and encryption policy info,
then particular filesystem can provider specified interface to recover this
file, maybe via DIO or ioctl.

Maybe we can discuss with google developer that how can we handle dumpfile
flow with encrypted regular.

Anyway, enable/disable this functionality are both okay to me.

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

> 
> thanks,
> Sheng
>>
>> Thanks,
>>
>>>
>>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>>> ---
>>>   fsck/dump.c | 12 ++++++++----
>>>   1 file changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/fsck/dump.c b/fsck/dump.c
>>> index 01889fd..d11e8e2 100644
>>> --- a/fsck/dump.c
>>> +++ b/fsck/dump.c
>>> @@ -410,12 +410,17 @@ static void dump_file(struct f2fs_sb_info *sbi, struct node_info *ni,
>>>   	struct f2fs_inode *inode = &node_blk->i;
>>>   	u32 imode = le32_to_cpu(inode->i_mode);
>>>   	u32 namelen = le32_to_cpu(inode->i_namelen);
>>> -	unsigned char name[F2FS_NAME_LEN + 1] = {0};
>>> +	char name[F2FS_NAME_LEN + 1] = {0};
>>>   	char path[1024] = {0};
>>>   	char ans[255] = {0};
>>> -	int enc_name = file_enc_name(inode);
>>> +	int is_encrypted = file_is_encrypt(inode);
>>>   	int ret;
>>>   
>>> +	if (is_encrypted) {
>>> +		MSG(force, "File is encrypted\n");
>>> +		return;
>>> +	}
>>> +
>>>   	if (!S_ISREG(imode) || namelen == 0 || namelen > F2FS_NAME_LEN) {
>>>   		MSG(force, "Not a regular file or wrong name info\n\n");
>>>   		return;
>>> @@ -433,8 +438,7 @@ dump:
>>>   		ASSERT(ret >= 0);
>>>   
>>>   		/* make a file */
>>> -		namelen = convert_encrypted_name(inode->i_name, namelen,
>>> -							name, enc_name);
>>> +		strncpy(name, (const char *)inode->i_name, namelen);
>>>   		name[namelen] = 0;
>>>   		sprintf(path, "./lost_found/%s", name);
>>>   
>>>
>>
>>
>> .
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 6/8] fsck.f2fs: introduce new option --dry-run

[Chao Yu]

On 2017/10/31 17:19, Sheng Yong wrote:
> 
> 
> On 2017/10/31 15:57, Chao Yu wrote:
>> On 2017/10/31 9:38, Sheng Yong wrote:
>>> With --dry-run enabled, fsck.f2fs will do all checks and "fixes" except
>>> that all fixes will not be written to storage at last.
>>>
>>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>>> ---
>>>   fsck/main.c       | 14 +++++++++++++-
>>>   include/f2fs_fs.h |  1 +
>>>   lib/libf2fs.c     |  1 +
>>>   lib/libf2fs_io.c  |  3 +++
>>>   4 files changed, 18 insertions(+), 1 deletion(-)
>>>
>>>   
> [...]
>>>   static int is_mounted(const char *mpt, const char *device)
>>> diff --git a/lib/libf2fs_io.c b/lib/libf2fs_io.c
>>> index 50ff171..8a79672 100644
>>> --- a/lib/libf2fs_io.c
>>> +++ b/lib/libf2fs_io.c
>>> @@ -129,6 +129,9 @@ int dev_write(void *buf, __u64 offset, size_t len)
>>>   {
>>>   	int fd;
>>>   
>>> +	if (c.dry_run)
>>> +		return 0;
>>> +
>>>   	if (c.sparse_mode)
>>>   		return dev_write_sparse(buf, offset, len);
>>
>> Do we need to cover dev_write_block?
> dev_write_block calls dev_write. So dev_write_block is also covered by intercepting
> write in dev_write. :)

Correct. :)

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

> 
> Thanks,
> Sheng
>>
>> Thanks,
>>
>>>   
>>>
>>
>>
>> .
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH 1/8] fsck.f2fs: do not set fix_on directly

[Chao Yu]

On 2017/10/31 17:34, Sheng Yong wrote:
> 
> 
> On 2017/10/31 15:06, Chao Yu wrote:
>> On 2017/10/31 9:38, Sheng Yong wrote:
>>> Do not set fix_on if it is allowed by user.
>>>
>>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>>> ---
>>>   fsck/main.c  | 2 +-
>>>   fsck/mount.c | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fsck/main.c b/fsck/main.c
>>> index c9411eb..93037e1 100644
>>> --- a/fsck/main.c
>>> +++ b/fsck/main.c
>>> @@ -429,7 +429,7 @@ static void do_fsck(struct f2fs_sb_info *sbi)
>>>   				c.fix_on = 1;
>>>   			break;
>>>   		}
>>> -	} else {
>>> +	} else if (c.preen_mode) {
>>
>> Needs to update below comments?
> I think the new condition hits comment 3 ("fsck -p 1 && error is detected, then
> bug_on is set fix_on = 1 here, so that fsck can fix errors automatically"). The
> condition was missing and the comment seems not that correct before ;)

OK, so please fix it in your patch. :)

Thanks,

> 
> thanks,
> Sheng
>>
>>>   		/*
>>>   		 * we can hit this in 3 situations:
>>>   		 *  1. fsck -f, fix_on has already been set to 1 when
>>> diff --git a/fsck/mount.c b/fsck/mount.c
>>> index 29af3b7..faa9bc8 100644
>>> --- a/fsck/mount.c
>>> +++ b/fsck/mount.c
>>> @@ -1879,6 +1879,7 @@ void nullify_nat_entry(struct f2fs_sb_info *sbi, u32 nid)
>>>   
>>>   	memset(&nat_block->entries[entry_off], 0,
>>>   					sizeof(struct f2fs_nat_entry));
>>> +	FIX_MSG("Remove nid [0x%x] in NAT\n", nid);
>>
>> How about adding debug message in separated patch?
>>
>> Thanks,
>>
>>>   
>>>   	ret = dev_write_block(nat_block, block_addr);
>>>   	ASSERT(ret >= 0);
>>> @@ -2031,7 +2032,6 @@ void build_nat_area_bitmap(struct f2fs_sb_info *sbi)
>>>   				 */
>>>   				ASSERT_MSG("Invalid nat entry[0]: "
>>>   					"blk_addr[0x%x]\n", ni.blk_addr);
>>> -				c.fix_on = 1;
>>>   				fsck->chk.valid_nat_entry_cnt--;
>>>   			}
>>>   
>>>
>>
>>
>> .
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot