Skip to content

Commit

Permalink
ext4: remove ext4_xattr_check_entry()
Browse files Browse the repository at this point in the history
ext4_xattr_check_entry() was redundant with validation of the full xattr
entries list in ext4_xattr_check_entries(), which all callers also did.
ext4_xattr_check_entry() also didn't actually do correct validation;
specifically, it never checked that the value doesn't overlap the xattr
names, nor did it account for padding when checking whether the xattr
value overflows the available space.  So remove it to eliminate any
potential confusion.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  • Loading branch information
ebiggers authored and tytso committed Apr 30, 2017
1 parent 2c4f992 commit 6ba644b
Showing 1 changed file with 6 additions and 24 deletions.
30 changes: 6 additions & 24 deletions fs/ext4/xattr.c
Original file line number Diff line number Diff line change
Expand Up @@ -244,20 +244,9 @@ __xattr_check_inode(struct inode *inode, struct ext4_xattr_ibody_header *header,
#define xattr_check_inode(inode, header, end) \
__xattr_check_inode((inode), (header), (end), __func__, __LINE__)

static inline int
ext4_xattr_check_entry(struct ext4_xattr_entry *entry, size_t size)
{
size_t value_size = le32_to_cpu(entry->e_value_size);

if (entry->e_value_block != 0 || value_size > size ||
le16_to_cpu(entry->e_value_offs) + value_size > size)
return -EFSCORRUPTED;
return 0;
}

static int
ext4_xattr_find_entry(struct ext4_xattr_entry **pentry, int name_index,
const char *name, size_t size, int sorted)
const char *name, int sorted)
{
struct ext4_xattr_entry *entry;
size_t name_len;
Expand All @@ -277,8 +266,6 @@ ext4_xattr_find_entry(struct ext4_xattr_entry **pentry, int name_index,
break;
}
*pentry = entry;
if (!cmp && ext4_xattr_check_entry(entry, size))
return -EFSCORRUPTED;
return cmp ? -ENODATA : 0;
}

Expand Down Expand Up @@ -306,17 +293,14 @@ ext4_xattr_block_get(struct inode *inode, int name_index, const char *name,
ea_bdebug(bh, "b_count=%d, refcount=%d",
atomic_read(&(bh->b_count)), le32_to_cpu(BHDR(bh)->h_refcount));
if (ext4_xattr_check_block(inode, bh)) {
bad_block:
EXT4_ERROR_INODE(inode, "bad block %llu",
EXT4_I(inode)->i_file_acl);
error = -EFSCORRUPTED;
goto cleanup;
}
ext4_xattr_cache_insert(ext4_mb_cache, bh);
entry = BFIRST(bh);
error = ext4_xattr_find_entry(&entry, name_index, name, bh->b_size, 1);
if (error == -EFSCORRUPTED)
goto bad_block;
error = ext4_xattr_find_entry(&entry, name_index, name, 1);
if (error)
goto cleanup;
size = le32_to_cpu(entry->e_value_size);
Expand Down Expand Up @@ -353,13 +337,12 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
return error;
raw_inode = ext4_raw_inode(&iloc);
header = IHDR(inode, raw_inode);
entry = IFIRST(header);
end = (void *)raw_inode + EXT4_SB(inode->i_sb)->s_inode_size;
error = xattr_check_inode(inode, header, end);
if (error)
goto cleanup;
error = ext4_xattr_find_entry(&entry, name_index, name,
end - (void *)entry, 0);
entry = IFIRST(header);
error = ext4_xattr_find_entry(&entry, name_index, name, 0);
if (error)
goto cleanup;
size = le32_to_cpu(entry->e_value_size);
Expand Down Expand Up @@ -793,7 +776,7 @@ ext4_xattr_block_find(struct inode *inode, struct ext4_xattr_info *i,
bs->s.end = bs->bh->b_data + bs->bh->b_size;
bs->s.here = bs->s.first;
error = ext4_xattr_find_entry(&bs->s.here, i->name_index,
i->name, bs->bh->b_size, 1);
i->name, 1);
if (error && error != -ENODATA)
goto cleanup;
bs->s.not_found = error;
Expand Down Expand Up @@ -1065,8 +1048,7 @@ int ext4_xattr_ibody_find(struct inode *inode, struct ext4_xattr_info *i,
return error;
/* Find the named attribute. */
error = ext4_xattr_find_entry(&is->s.here, i->name_index,
i->name, is->s.end -
(void *)is->s.base, 0);
i->name, 0);
if (error && error != -ENODATA)
return error;
is->s.not_found = error;
Expand Down

0 comments on commit 6ba644b

Please sign in to comment.