Re: [PATCH v2 2/2] ext2: introduce helper for xattr entry validation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I think this patch need to be refined.

>> if (entry->e_value_block != 0
duplicated with check bellow ext2_xattr_entry_valid checks.




> 14 мая 2019 г., в 18:32, Andreas Dilger <adilger@xxxxxxxxx> написал(а):
> 
> On May 13, 2019, at 4:40 PM, Chengguang Xu <cgxu519@xxxxxxxxxxx> wrote:
>> 
>> Introduce helper function ext2_xattr_entry_valid()
>> for xattr entry validation and clean up the entry
>> check ralated code.
>> 
>> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxxxxxx>
> 
> Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx>
> 
>> ---
>> v1->v2:
>> - Pass end offset instead of inode to ext2_xattr_entry_valid()
>> - Change signed-off mail address.
>> 
>> fs/ext2/xattr.c | 21 +++++++++++++++++----
>> 1 file changed, 17 insertions(+), 4 deletions(-)
>> 
>> diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
>> index db27260d6a5b..d11c83529514 100644
>> --- a/fs/ext2/xattr.c
>> +++ b/fs/ext2/xattr.c
>> @@ -144,6 +144,20 @@ ext2_xattr_header_valid(struct ext2_xattr_header *header)
>> 	return true;
>> }
>> 
>> +static bool
>> +ext2_xattr_entry_valid(struct ext2_xattr_entry *entry, size_t size,
>> +		       size_t end_offs)
>> +{
>> +	if (entry->e_value_block != 0)
>> +		return false;
>> +
>> +	if (size > end_offs ||
>> +	    le16_to_cpu(entry->e_value_offs) + size > end_offs)
>> +		return false;
>> +
>> +	return true;
>> +}
>> +
>> /*
>> * ext2_xattr_get()
>> *
>> @@ -217,8 +231,7 @@ ext2_xattr_get(struct inode *inode, int name_index, const char *name,
>> 	if (entry->e_value_block != 0)
>> 		goto bad_block;
>> 	size = le32_to_cpu(entry->e_value_size);
>> -	if (size > inode->i_sb->s_blocksize ||
>> -	    le16_to_cpu(entry->e_value_offs) + size > inode->i_sb->s_blocksize)
>> +	if (!ext2_xattr_entry_valid(entry, size, inode->i_sb->s_blocksize))
>> 		goto bad_block;
>> 
>> 	if (ext2_xattr_cache_insert(ea_block_cache, bh))
>> @@ -483,8 +496,8 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
>> 		if (!here->e_value_block && here->e_value_size) {
>> 			size_t size = le32_to_cpu(here->e_value_size);
>> 
>> -			if (le16_to_cpu(here->e_value_offs) + size >
>> -			    sb->s_blocksize || size > sb->s_blocksize)
>> +			if (!ext2_xattr_entry_valid(here, size,
>> +			    inode->i_sb->s_blocksize))
>> 				goto bad_block;
>> 			free += EXT2_XATTR_SIZE(size);
>> 		}
>> --
>> 2.17.2
>> 
>> 
> 
> 
> Cheers, Andreas
> 
> 
> 
> 
> 





[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux