linux-ext4 - [PATCH 3/3] ext2: Strengthen xattr block checks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 16 May 2019 12:03:22 +0200
From:   Jan Kara <jack@...e.cz>
To:     <linux-ext4@...r.kernel.org>
Cc:     cgxu519@...o.com.cn, Jan Kara <jack@...e.cz>
Subject: [PATCH 3/3] ext2: Strengthen xattr block checks

Check every entry in xattr block for validity in ext2_xattr_set() to
detect on disk corruption early. Also since e_value_block field in xattr
entry is never != 0 in a valid filesystem, just remove checks for it
once we have established entries are valid.

Signed-off-by: Jan Kara <jack@...e.cz>
---
 fs/ext2/xattr.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index f9fda6d16d78..d21dbf297b74 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -218,6 +218,8 @@ ext2_xattr_get(struct inode *inode, int name_index, const char *name,
 			EXT2_XATTR_NEXT(entry);
 		if ((char *)next >= end)
 			goto bad_block;
+		if (!ext2_xattr_entry_valid(entry, inode->i_sb->s_blocksize))
+			goto bad_block;
 		if (name_index == entry->e_name_index &&
 		    name_len == entry->e_name_len &&
 		    memcmp(name, entry->e_name, name_len) == 0)
@@ -229,9 +231,6 @@ ext2_xattr_get(struct inode *inode, int name_index, const char *name,
 	error = -ENODATA;
 	goto cleanup;
 found:
-	if (!ext2_xattr_entry_valid(entry, inode->i_sb->s_blocksize))
-		goto bad_block;
-
 	size = le32_to_cpu(entry->e_value_size);
 	if (ext2_xattr_cache_insert(ea_block_cache, bh))
 		ea_idebug(inode, "cache insert failed");
@@ -304,6 +303,8 @@ ext2_xattr_list(struct dentry *dentry, char *buffer, size_t buffer_size)
 
 		if ((char *)next >= end)
 			goto bad_block;
+		if (!ext2_xattr_entry_valid(entry, inode->i_sb->s_blocksize))
+			goto bad_block;
 		entry = next;
 	}
 	if (ext2_xattr_cache_insert(ea_block_cache, bh))
@@ -446,7 +447,9 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
 			struct ext2_xattr_entry *next = EXT2_XATTR_NEXT(last);
 			if ((char *)next >= end)
 				goto bad_block;
-			if (!last->e_value_block && last->e_value_size) {
+			if (!ext2_xattr_entry_valid(last, sb->s_blocksize))
+				goto bad_block;
+			if (last->e_value_size) {
 				size_t offs = le16_to_cpu(last->e_value_offs);
 				if (offs < min_offs)
 					min_offs = offs;
@@ -489,12 +492,7 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
 		error = -EEXIST;
 		if (flags & XATTR_CREATE)
 			goto cleanup;
-		if (!here->e_value_block && here->e_value_size) {
-			if (!ext2_xattr_entry_valid(here, sb->s_blocksize))
-				goto bad_block;
-			free += EXT2_XATTR_SIZE(
-					le32_to_cpu(here->e_value_size));
-		}
+		free += EXT2_XATTR_SIZE(le32_to_cpu(here->e_value_size));
 		free += EXT2_XATTR_LEN(name_len);
 	}
 	error = -ENOSPC;
@@ -559,7 +557,7 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
 		here->e_name_len = name_len;
 		memcpy(here->e_name, name, name_len);
 	} else {
-		if (!here->e_value_block && here->e_value_size) {
+		if (here->e_value_size) {
 			char *first_val = (char *)header + min_offs;
 			size_t offs = le16_to_cpu(here->e_value_offs);
 			char *val = (char *)header + offs;
@@ -586,7 +584,7 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
 			last = ENTRY(header+1);
 			while (!IS_LAST_ENTRY(last)) {
 				size_t o = le16_to_cpu(last->e_value_offs);
-				if (!last->e_value_block && o < offs)
+				if (o < offs)
 					last->e_value_offs =
 						cpu_to_le16(o + size);
 				last = EXT2_XATTR_NEXT(last);
-- 
2.16.4