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]
Message-ID: <20131204030242.GJ9535@birch.djwong.org>
Date:	Tue, 3 Dec 2013 19:02:42 -0800
From:	"Darrick J. Wong" <darrick.wong@...cle.com>
To:	Zheng Liu <gnehzuil.liu@...il.com>
Cc:	linux-ext4@...r.kernel.org, "Theodore Ts'o" <tytso@....edu>,
	Zheng Liu <wenqing.lz@...bao.com>
Subject: Re: [PATCH v2 17/28] libext2fs: handle inline data in read/write
 function

On Tue, Dec 03, 2013 at 08:11:44PM +0800, Zheng Liu wrote:
> From: Zheng Liu <wenqing.lz@...bao.com>
> 
> Currently ext2fs_file_read/write are used to copy data from/to a file.
> But they manipulate data by blocksize.  For supporting inline data, we
> handle it in two new fucntions called ext2fs_file_read/write_inline_data.
> 
> In read path the implementation is straightforward.  But in write path
> things get more complicated because if the size of data is greater than
> the maximum size of inline data we will expand this file.  So now we
> will check this in ext2fs_xattr_set().  If this inode doesn't have
> enough space, it will return EXT2_ET_INLINE_DATA_NO_SPACE error.  Then
> the caller will check this error and tries to expand the file.
> 
> The following commands in debugfs can handle inline_data feature after
> applying this patch:
> 	- dump
> 	- cat
> 	- rdump
> 	- write
> 
> Signed-off-by: Theodore Ts'o <tytso@....edu>
> Signed-off-by: Zheng Liu <wenqing.lz@...bao.com>
> ---
>  debugfs/debugfs.c         |   13 ++++--
>  lib/ext2fs/ext2_err.et.in |    3 ++
>  lib/ext2fs/ext2fsP.h      |    6 +++
>  lib/ext2fs/ext_attr.c     |   76 +++++++++++++++++++++++++++++++
>  lib/ext2fs/fileio.c       |  108 +++++++++++++++++++++++++++++++++++++++++++++
>  lib/ext2fs/inline_data.c  |   90 ++++++++++++++++++++++++++++++++++---
>  6 files changed, 287 insertions(+), 9 deletions(-)
> 
> diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
> index d1c81f1..0903c8e 100644
> --- a/debugfs/debugfs.c
> +++ b/debugfs/debugfs.c
> @@ -1684,7 +1684,6 @@ fail:
>  	return retval;
>  }
>  
> -
>  void do_write(int argc, char *argv[])
>  {
>  	int		fd;
> @@ -1750,8 +1749,11 @@ void do_write(int argc, char *argv[])
>  		current_fs->now ? current_fs->now : time(0);
>  	inode.i_links_count = 1;
>  	inode.i_size = statbuf.st_size;
> -	if (current_fs->super->s_feature_incompat &
> -	    EXT3_FEATURE_INCOMPAT_EXTENTS) {
> +	if (EXT2_HAS_INCOMPAT_FEATURE(current_fs->super,
> +				      EXT4_FEATURE_INCOMPAT_INLINE_DATA)) {
> +		inode.i_flags |= EXT4_INLINE_DATA_FL;
> +	} else if (current_fs->super->s_feature_incompat &
> +		   EXT3_FEATURE_INCOMPAT_EXTENTS) {
>  		int i;
>  		struct ext3_extent_header *eh;
>  
> @@ -1768,6 +1770,11 @@ void do_write(int argc, char *argv[])
>  		close(fd);
>  		return;
>  	}
> +	if (inode.i_flags & EXT4_INLINE_DATA_FL) {
> +		retval = ext2fs_inline_data_init(current_fs, newfile);
> +		if (retval)
> +			return;
> +	}
>  	if (LINUX_S_ISREG(inode.i_mode)) {
>  		if (statbuf.st_blocks < statbuf.st_size / S_BLKSIZE) {
>  			make_holes = 1;
> diff --git a/lib/ext2fs/ext2_err.et.in b/lib/ext2fs/ext2_err.et.in
> index 0781145..68aa3e0 100644
> --- a/lib/ext2fs/ext2_err.et.in
> +++ b/lib/ext2fs/ext2_err.et.in
> @@ -503,4 +503,7 @@ ec	EXT2_ET_EA_NO_SPACE,
>  ec	EXT2_ET_NO_INLINE_DATA,
>  	"Inode doesn't have inline data"
>  
> +ec	EXT2_ET_INLINE_DATA_NO_SPACE,
> +	"No free space in inline data"
> +
>  	end
> diff --git a/lib/ext2fs/ext2fsP.h b/lib/ext2fs/ext2fsP.h
> index bfc321c..e159128 100644
> --- a/lib/ext2fs/ext2fsP.h
> +++ b/lib/ext2fs/ext2fsP.h
> @@ -102,6 +102,12 @@ extern errcode_t ext2fs_inline_data_dir_iterate(ext2_filsys fs,
>  						  int	      ref_offset,
>  						  void	      *priv_data),
>  				      void *priv_data);
> +extern errcode_t ext2fs_inline_data_get(ext2_filsys fs, ext2_ino_t ino,
> +					struct ext2_inode *inode,
> +					void *buf, size_t *size);
> +extern errcode_t ext2fs_inline_data_set(ext2_filsys fs, ext2_ino_t ino,
> +					struct ext2_inode *inode,
> +					void *buf, size_t size);
>  
>  /* Generic numeric progress meter */
>  
> diff --git a/lib/ext2fs/ext_attr.c b/lib/ext2fs/ext_attr.c
> index 0e9a9ab..98f0fbb 100644
> --- a/lib/ext2fs/ext_attr.c
> +++ b/lib/ext2fs/ext_attr.c
> @@ -818,6 +818,71 @@ errcode_t ext2fs_xattr_get(struct ext2_xattr_handle *h, const char *key,
>  	return EXT2_ET_EA_KEY_NOT_FOUND;
>  }
>  
> +static errcode_t ext2fs_xattr_max_size(struct ext2_xattr_handle *handle, size_t *size)
> +{
> +	struct ext2_ext_attr_header *header;
> +	struct ext2_ext_attr_entry *entry;
> +	struct ext2_inode_large *inode;
> +	__u32 ea_inode_magic;
> +	unsigned int storage_size, freesize, minoff;
> +	void *start;
> +	int i;
> +	errcode_t err;
> +
> +	i = EXT2_INODE_SIZE(handle->fs->super);
> +	if (i < sizeof(*inode))
> +		i = sizeof(*inode);
> +	err = ext2fs_get_memzero(i, &inode);
> +	if (err)
> +		return err;
> +
> +	err = ext2fs_read_inode_full(handle->fs, handle->ino,
> +				     (struct ext2_inode *)inode,
> +				     EXT2_INODE_SIZE(handle->fs->super));
> +	if (err)
> +		goto out;
> +
> +	/* Does the inode have size for EA? */
> +	if (EXT2_INODE_SIZE(handle->fs->super) <= EXT2_GOOD_OLD_INODE_SIZE +
> +						  inode->i_extra_isize +
> +						  sizeof(__u32)) {
> +		err = EXT2_ET_INLINE_DATA_NO_SPACE;
> +		goto out;
> +	}
> +
> +	minoff = EXT2_INODE_SIZE(handle->fs->super) -
> +		 sizeof(*inode) - sizeof(__u32);
> +	memcpy(&ea_inode_magic, ((char *) inode) + EXT2_GOOD_OLD_INODE_SIZE +
> +	       inode->i_extra_isize, sizeof(__u32));
> +	if (ea_inode_magic == EXT2_EXT_ATTR_MAGIC) {
> +		/* has xattrs.  calculate the size */
> +		storage_size = EXT2_INODE_SIZE(handle->fs->super) -
> +			EXT2_GOOD_OLD_INODE_SIZE - inode->i_extra_isize -
> +			sizeof(__u32);
> +		start= ((char *) inode) + EXT2_GOOD_OLD_INODE_SIZE +
> +			inode->i_extra_isize + sizeof(__u32);
> +		entry = start;
> +		while (!EXT2_EXT_IS_LAST_ENTRY(entry)) {
> +			if (!entry->e_value_block && entry->e_value_size) {
> +				unsigned int offs = entry->e_value_offs;
> +				if (offs < minoff)
> +					minoff = offs;
> +			}
> +			entry = EXT2_EXT_ATTR_NEXT(entry);
> +		}
> +		*size = minoff - ((char *)entry - (char *)start) - sizeof(__u32);
> +	} else {
> +		/* no xattr.  return a maximum size */
> +		*size = EXT2_EXT_ATTR_SIZE(minoff -
> +					   EXT2_EXT_ATTR_LEN(strlen("data")) -
> +					   EXT2_EXT_ATTR_ROUND - sizeof(__u32));
> +	}
> +
> +out:
> +	ext2fs_free_mem(&inode);
> +	return err;
> +}
> +
>  errcode_t ext2fs_xattr_set(struct ext2_xattr_handle *handle,
>  			   const char *key,
>  			   const void *value,
> @@ -827,6 +892,17 @@ errcode_t ext2fs_xattr_set(struct ext2_xattr_handle *handle,
>  	char *new_value;
>  	errcode_t err;
>  
> +	/* we need to check inline data space */
> +	if (strcmp(key, "system.data") == 0) {
> +		size_t max_size;
> +
> +		err = ext2fs_xattr_max_size(handle, &max_size);
> +		if (err)
> +			return err;
> +		if (value_len > max_size)
> +			return EXT2_ET_INLINE_DATA_NO_SPACE;
> +	}
> +
>  	last_empty = NULL;
>  	for (x = handle->attrs; x < handle->attrs + handle->length; x++) {
>  		if (!x->name) {
> diff --git a/lib/ext2fs/fileio.c b/lib/ext2fs/fileio.c
> index 02e6263..acb705d 100644
> --- a/lib/ext2fs/fileio.c
> +++ b/lib/ext2fs/fileio.c
> @@ -224,6 +224,39 @@ errcode_t ext2fs_file_close(ext2_file_t file)
>  }
>  
>  
> +static errcode_t
> +ext2fs_file_read_inline_data(ext2_file_t file, void *buf,
> +			     unsigned int wanted, unsigned int *got)
> +{
> +	ext2_filsys fs;
> +	errcode_t retval;
> +	unsigned int start, count = 0;
> +	size_t size;
> +
> +	fs = file->fs;
> +	retval = ext2fs_inline_data_get(fs, file->ino, &file->inode,
> +					file->buf, &size);
> +	if (retval)
> +		return retval;
> +
> +	if (file->pos >= size)
> +		goto out;
> +
> +	start = file->pos % size;

If file->pos can never be larger than size, this is unnecessary.

> +	count = size - start;
> +	if (count > wanted)
> +		count = wanted;
> +	memcpy(buf, file->buf + start, count);
> +	file->pos += count;
> +	buf += count;
> +
> +out:
> +	if (got)
> +		*got = count;
> +	return retval;
> +}
> +
> +
>  errcode_t ext2fs_file_read(ext2_file_t file, void *buf,
>  			   unsigned int wanted, unsigned int *got)
>  {
> @@ -236,6 +269,10 @@ errcode_t ext2fs_file_read(ext2_file_t file, void *buf,
>  	EXT2_CHECK_MAGIC(file, EXT2_ET_MAGIC_EXT2_FILE);
>  	fs = file->fs;
>  
> +	/* If an inode has inline data, things get complicated. */
> +	if (file->inode.i_flags & EXT4_INLINE_DATA_FL)
> +		return ext2fs_file_read_inline_data(file, buf, wanted, got);
> +
>  	while ((file->pos < EXT2_I_SIZE(&file->inode)) && (wanted > 0)) {
>  		retval = sync_buffer_position(file);
>  		if (retval)
> @@ -266,6 +303,67 @@ fail:
>  }
>  
>  
> +static errcode_t
> +ext2fs_file_write_inline_data(ext2_file_t file, const void *buf,
> +			      unsigned int nbytes, unsigned int *written)
> +{
> +	ext2_filsys fs;
> +	errcode_t retval;
> +	unsigned int start, count = 0;
> +	size_t size;
> +
> +	fs = file->fs;
> +	retval = ext2fs_inline_data_get(fs, file->ino, &file->inode,
> +					file->buf, &size);
> +	if (retval)
> +		return retval;
> +
> +	if (file->pos < size) {
> +		start = file->pos % fs->blocksize;

Since we're not dealing with blocks, I don't think you need to '%' with the
block size.

--D

> +		count = nbytes - start;
> +		memcpy(file->buf + start, buf, count);
> +
> +		retval = ext2fs_inline_data_set(fs, file->ino, &file->inode,
> +						file->buf, count);
> +		if (retval == EXT2_ET_INLINE_DATA_NO_SPACE)
> +			goto expand;
> +		if (retval)
> +			return retval;
> +
> +		file->pos += count;
> +
> +		/* Update inode size */
> +		if (count != 0 && EXT2_I_SIZE(&file->inode) < file->pos) {
> +			errcode_t	rc;
> +
> +			rc = ext2fs_file_set_size2(file, file->pos);
> +			if (retval == 0)
> +				retval = rc;
> +		}
> +
> +		if (written)
> +			*written = count;
> +		return 0;
> +	}
> +
> +expand:
> +	retval = ext2fs_inline_data_expand(fs, file->ino);
> +	if (retval)
> +		return retval;
> +	/*
> +	 * reload inode and return no space error
> +	 *
> +	 * XXX: file->inode could be copied from the outside
> +	 * in ext2fs_file_open2().  We have no way to modify
> +	 * the outside inode.
> +	 */
> +	retval = ext2fs_read_inode(fs, file->ino, &file->inode);
> +	if (retval)
> +		return retval;
> +	return EXT2_ET_INLINE_DATA_NO_SPACE;
> +}
> +
> +
>  errcode_t ext2fs_file_write(ext2_file_t file, const void *buf,
>  			    unsigned int nbytes, unsigned int *written)
>  {
> @@ -280,6 +378,16 @@ errcode_t ext2fs_file_write(ext2_file_t file, const void *buf,
>  	if (!(file->flags & EXT2_FILE_WRITE))
>  		return EXT2_ET_FILE_RO;
>  
> +	/* If an inode has inline data, things get complicated. */
> +	if (file->inode.i_flags & EXT4_INLINE_DATA_FL) {
> +		retval = ext2fs_file_write_inline_data(file, buf, nbytes,
> +						       written);
> +		if (retval != EXT2_ET_INLINE_DATA_NO_SPACE)
> +			return retval;
> +		/* fall through to read data from the block */
> +		retval = 0;
> +	}
> +
>  	while (nbytes > 0) {
>  		retval = sync_buffer_position(file);
>  		if (retval)
> diff --git a/lib/ext2fs/inline_data.c b/lib/ext2fs/inline_data.c
> index 16af814..4a35209 100644
> --- a/lib/ext2fs/inline_data.c
> +++ b/lib/ext2fs/inline_data.c
> @@ -309,6 +309,7 @@ errcode_t ext2fs_inline_data_expand(ext2_filsys fs, ext2_ino_t ino)
>  	struct ext2_inline_data data;
>  	errcode_t retval;
>  	blk64_t blk;
> +	size_t inline_size;
>  	char *inline_buf = 0;
>  	char *blk_buf = 0;
>  
> @@ -327,8 +328,8 @@ errcode_t ext2fs_inline_data_expand(ext2_filsys fs, ext2_ino_t ino)
>  	retval = ext2fs_inline_data_ea_get(&data);
>  	if (retval)
>  		return retval;
> -	retval = ext2fs_get_mem(EXT4_MIN_INLINE_DATA_SIZE + data.ea_size,
> -				&inline_buf);
> +	inline_size = data.ea_size + EXT4_MIN_INLINE_DATA_SIZE;
> +	retval = ext2fs_get_mem(inline_size, &inline_buf);
>  	if (retval)
>  		goto errout;
>  
> @@ -347,10 +348,14 @@ errcode_t ext2fs_inline_data_expand(ext2_filsys fs, ext2_ino_t ino)
>  	if (retval)
>  		goto errout;
>  
> -	/* Adjust the rec_len */
> -	retval = ext2fs_inline_data_convert_dir(fs, ino, blk_buf, inline_buf,
> -						EXT4_MIN_INLINE_DATA_SIZE +
> -							data.ea_size);
> +	if (LINUX_S_ISDIR(inode.i_mode)) {
> +		/* Adjust the rec_len */
> +		retval = ext2fs_inline_data_convert_dir(fs, ino, blk_buf,
> +							inline_buf, inline_size);
> +	} else {
> +		/* Copy data for a regular inode */
> +		memcpy(blk_buf, inline_buf, inline_size);
> +	}
>  	if (retval)
>  		goto errout;
>  
> @@ -389,3 +394,76 @@ errout:
>  	ext2fs_free_mem(&data.ea_data);
>  	return retval;
>  }
> +
> +/*
> + * When caller uses this function to retrieve the inline data, it must
> + * allocate a buffer which has the size of inline data.  The size of
> + * inline data can be know by ext2fs_inline_data_get_size().
> + */
> +errcode_t ext2fs_inline_data_get(ext2_filsys fs, ext2_ino_t ino,
> +				 struct ext2_inode *inode,
> +				 void *buf, size_t *size)
> +{
> +	struct ext2_inode inode_buf;
> +	struct ext2_inline_data data;
> +	errcode_t retval;
> +
> +	if (!inode) {
> +		retval = ext2fs_read_inode(fs, ino, &inode_buf);
> +		if (retval)
> +			return retval;
> +		inode = &inode_buf;
> +	}
> +
> +	data.fs = fs;
> +	data.ino = ino;
> +	retval = ext2fs_inline_data_ea_get(&data);
> +	if (retval)
> +		return retval;
> +
> +	memcpy(buf, (void *)inode->i_block, EXT4_MIN_INLINE_DATA_SIZE);
> +	if (data.ea_size > 0)
> +		memcpy(buf + EXT4_MIN_INLINE_DATA_SIZE,
> +		       data.ea_data, data.ea_size);
> +
> +	if (size)
> +		*size = EXT4_MIN_INLINE_DATA_SIZE + data.ea_size;
> +	ext2fs_free_mem(&data.ea_data);
> +	return 0;
> +}
> +
> +errcode_t ext2fs_inline_data_set(ext2_filsys fs, ext2_ino_t ino,
> +				 struct ext2_inode *inode,
> +				 void *buf, size_t size)
> +{
> +	struct ext2_inode inode_buf;
> +	struct ext2_inline_data data;
> +	unsigned int max_size = 0;
> +	errcode_t retval;
> +
> +	if (!inode) {
> +		retval = ext2fs_read_inode(fs, ino, &inode_buf);
> +		if (retval)
> +			return retval;
> +		inode = &inode_buf;
> +	}
> +
> +	/* simple case */
> +	if (size <= EXT4_MIN_INLINE_DATA_SIZE) {
> +		memcpy((void *)inode->i_block, buf, size);
> +		return ext2fs_write_inode(fs, ino, inode);
> +	}
> +
> +	/*
> +	 * complicated case
> +	 */
> +	memcpy((void *)inode->i_block, buf, EXT4_MIN_INLINE_DATA_SIZE);
> +	retval = ext2fs_write_inode(fs, ino, inode);
> +	if (retval)
> +		return retval;
> +	data.fs = fs;
> +	data.ino = ino;
> +	data.ea_size = size - EXT4_MIN_INLINE_DATA_SIZE;
> +	data.ea_data = buf + EXT4_MIN_INLINE_DATA_SIZE;
> +	return ext2fs_inline_data_ea_set(&data);
> +}
> -- 
> 1.7.9.7
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists