f2fs-tools: do sanity check on xattr entry

If there are corrupted xattr entries in xattr space, it may cause
traversing across end of xattr space issue, this patch adds sanity
check during xattr traverse to avoid such issue.

This patch synchronizes kernel commits:

2777e654371d ("f2fs: fix to avoid accessing xattr across the boundary")
688078e7f36c ("f2fs: fix to avoid memory leakage in f2fs_listxattr")

Signed-off-by: Chao Yu <chao@kernel.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

4 files changed, 48 insertions, 7 deletions

diff --git a/fsck/dump.c b/fsck/dump.c
index 6deaa7e..e7a1c5c 100644
--- a/fsck/dump.c
+++ b/fsck/dump.c
@@ -354,6 +354,7 @@ static void dump_node_blk(struct f2fs_sb_info *sbi, int ntype,
 static void dump_xattr(struct f2fs_sb_info *sbi, struct f2fs_node *node_blk)
 {
 	void *xattr;
+	void *last_base_addr;
 	struct f2fs_xattr_entry *ent;
 	char xattr_name[F2FS_NAME_LEN] = {0};
 	int ret;
@@ -362,10 +363,18 @@ static void dump_xattr(struct f2fs_sb_info *sbi, struct f2fs_node *node_blk)
 	if (!xattr)
 		return;
 
+	last_base_addr = (void *)xattr + XATTR_SIZE(&node_blk->i);
+
 	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 ((void *)(ent) + sizeof(__u32) > last_base_addr ||
+			(void *)XATTR_NEXT_ENTRY(ent) > last_base_addr) {
+			MSG(0, "xattr entry crosses the end of xattr space\n");
+			break;
+		}
+
 		if (!name)
 			continue;
 
diff --git a/fsck/mount.c b/fsck/mount.c
index dbb516b..b0f4eab 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -261,6 +261,7 @@ void print_inode_info(struct f2fs_sb_info *sbi,
 {
 	struct f2fs_inode *inode = &node->i;
 	void *xattr_addr;
+	void *last_base_addr;
 	struct f2fs_xattr_entry *ent;
 	char en[F2FS_PRINT_NAMELEN];
 	unsigned int i = 0;
@@ -357,13 +358,22 @@ void print_inode_info(struct f2fs_sb_info *sbi,
 	DISP_u32(inode, i_nid[4]);	/* double indirect */
 
 	xattr_addr = read_all_xattrs(sbi, node);
-	if (xattr_addr) {
-		list_for_each_xattr(ent, xattr_addr) {
-			print_xattr_entry(ent);
+	if (!xattr_addr)
+		goto out;
+
+	last_base_addr = (void *)xattr_addr + XATTR_SIZE(&node->i);
+
+	list_for_each_xattr(ent, xattr_addr) {
+		if ((void *)(ent) + sizeof(__u32) > last_base_addr ||
+			(void *)XATTR_NEXT_ENTRY(ent) > last_base_addr) {
+			MSG(0, "xattr entry crosses the end of xattr space\n");
+			break;
 		}
-		free(xattr_addr);
+		print_xattr_entry(ent);
 	}
+	free(xattr_addr);
 
+out:
 	printf("\n");
 }
 
diff --git a/fsck/xattr.c b/fsck/xattr.c
index 04c2879..fe28437 100644
--- a/fsck/xattr.c
+++ b/fsck/xattr.c
@@ -55,11 +55,19 @@ void *read_all_xattrs(struct f2fs_sb_info *sbi, struct f2fs_node *inode)
 	return txattr_addr;
 }
 
-static struct f2fs_xattr_entry *__find_xattr(void *base_addr, int index,
-		size_t len, const char *name)
+static struct f2fs_xattr_entry *__find_xattr(void *base_addr,
+				void *last_base_addr, int index,
+				size_t len, const char *name)
 {
 	struct f2fs_xattr_entry *entry;
+
 	list_for_each_xattr(entry, base_addr) {
+		if ((void *)(entry) + sizeof(__u32) > last_base_addr ||
+			(void *)XATTR_NEXT_ENTRY(entry) > last_base_addr) {
+			MSG(0, "xattr entry crosses the end of xattr space\n");
+			return NULL;
+		}
+
 		if (entry->e_name_index != index)
 			continue;
 		if (entry->e_name_len != len)
@@ -125,6 +133,7 @@ int f2fs_setxattr(struct f2fs_sb_info *sbi, nid_t ino, int index, const char *na
 {
 	struct f2fs_node *inode;
 	void *base_addr;
+	void *last_base_addr;
 	struct f2fs_xattr_entry *here, *last;
 	struct node_info ni;
 	int error = 0;
@@ -159,7 +168,14 @@ int f2fs_setxattr(struct f2fs_sb_info *sbi, nid_t ino, int index, const char *na
 	base_addr = read_all_xattrs(sbi, inode);
 	ASSERT(base_addr);
 
-	here = __find_xattr(base_addr, index, len, name);
+	last_base_addr = (void *)base_addr + XATTR_SIZE(&inode->i);
+
+	here = __find_xattr(base_addr, last_base_addr, index, len, name);
+	if (!here) {
+		MSG(0, "Need to run fsck due to corrupted xattr.\n");
+		error = -EINVAL;
+		goto exit;
+	}
 
 	found = IS_XATTR_LAST_ENTRY(here) ? 0 : 1;
 
diff --git a/fsck/xattr.h b/fsck/xattr.h
index cddafa7..2527350 100644
--- a/fsck/xattr.h
+++ b/fsck/xattr.h
@@ -171,6 +171,12 @@ static inline int f2fs_acl_count(int size)
 			!IS_XATTR_LAST_ENTRY(entry); \
 			entry = XATTR_NEXT_ENTRY(entry))
 
+#define VALID_XATTR_BLOCK_SIZE	(F2FS_BLKSIZE - sizeof(struct node_footer))
+
+#define XATTR_SIZE(i)		((le32_to_cpu((i)->i_xattr_nid) ?	\
+					VALID_XATTR_BLOCK_SIZE : 0) +	\
+						(inline_xattr_size(i)))
+
 #define MIN_OFFSET	XATTR_ALIGN(F2FS_BLKSIZE -		\
 		sizeof(struct node_footer) - sizeof(__u32))