Message ID | 1375436989-18948-5-git-send-email-wenqing.lz@taobao.com |
---|---|
State | Superseded, archived |
Headers | show |
On Fri, Aug 02, 2013 at 05:49:31PM +0800, Zheng Liu wrote: > From: Zheng Liu <wenqing.lz@taobao.com> > > Inline_data is handled in dir iterator because a lot of commands use > this function to traverse directory entries in debugfs. We need to > handle inline_data individually because inline_data is saved in two > places. One is in i_block, and another is in ibody extended attribute. > > After applied this commit, the following commands in debugfs can > support the inline_data feature: > - ncheck > - chroot > - cd > - ls > - pwd > - link* > - unlink > > * If inline_data doesn't expand to ibody extended attribute, link > command will allocate a new block for this inode instead of using > extend attribute when we exhaust all space of i_block. > > Signed-off-by: Theodore Ts'o <tytso@mit.edu> > Signed-off-by: Zheng Liu <wenqing.lz@taobao.com> > --- > lib/ext2fs/dir_iterate.c | 102 ++++++++++++++++++++++++++++++++++- > lib/ext2fs/ext2_err.et.in | 6 +++ > lib/ext2fs/ext2_fs.h | 8 +++ > lib/ext2fs/ext2fs.h | 11 ++++ > lib/ext2fs/ext2fsP.h | 11 ++++ > lib/ext2fs/inline_data.c | 132 +++++++++++++++++++++++++++++++++++++++++++++ > 6 files changed, 268 insertions(+), 2 deletions(-) > > diff --git a/lib/ext2fs/dir_iterate.c b/lib/ext2fs/dir_iterate.c > index 1a4bf5c..7d5b1da 100644 > --- a/lib/ext2fs/dir_iterate.c > +++ b/lib/ext2fs/dir_iterate.c > @@ -123,8 +123,14 @@ errcode_t ext2fs_dir_iterate2(ext2_filsys fs, > ctx.func = func; > ctx.priv_data = priv_data; > ctx.errcode = 0; > - retval = ext2fs_block_iterate3(fs, dir, BLOCK_FLAG_READ_ONLY, 0, > - ext2fs_process_dir_block, &ctx); > + if (ext2fs_inode_has_inline_data(fs, dir)) > + retval = ext2fs_inline_data_iterate(fs, dir, > + BLOCK_FLAG_READ_ONLY, 0, > + ext2fs_process_dir_inline_data, > + &ctx); > + else > + retval = ext2fs_block_iterate3(fs, dir, BLOCK_FLAG_READ_ONLY, 0, > + ext2fs_process_dir_block, &ctx); > if (!block_buf) > ext2fs_free_mem(&ctx.buf); > if (retval) > @@ -282,3 +288,95 @@ next: > return 0; > } > > +int ext2fs_process_dir_inline_data(ext2_filsys fs, > + char *buf, > + unsigned int buf_len, > + e2_blkcnt_t blockcnt, > + struct ext2_inode_large *inode, > + void *priv_data) > +{ > + struct dir_context *ctx = (struct dir_context *) priv_data; > + unsigned int offset = 0; > + unsigned int next_real_entry = 0; > + int ret = 0; > + int changed = 0; > + int do_abort = 0; > + unsigned int rec_len, size; > + int entry; > + struct ext2_dir_entry *dirent; > + > + if (blockcnt < 0) > + return 0; > + > + entry = blockcnt ? DIRENT_OTHER_FILE : DIRENT_DOT_FILE; > + > + while (offset < buf_len) { > + dirent = (struct ext2_dir_entry *) (buf + offset); > + if (ext2fs_get_rec_len(fs, dirent, &rec_len)) > + return BLOCK_ABORT; > + if (((offset + rec_len) > buf_len) || > + (rec_len < 8) || > + ((rec_len % 4) != 0) || > + ((((unsigned) dirent->name_len & 0xFF)+8) > rec_len)) { > + ctx->errcode = EXT2_ET_DIR_CORRUPTED; > + return BLOCK_ABORT; > + } > + if (!dirent->inode && > + !(ctx->flags & DIRENT_FLAG_INCLUDE_EMPTY)) > + goto next; > + > + ret = (ctx->func)(ctx->dir, > + (next_real_entry > offset) ? > + DIRENT_DELETED_FILE : entry, > + dirent, offset, > + buf_len, buf, > + ctx->priv_data); > + if (entry < DIRENT_OTHER_FILE) > + entry++; > + > + if (ret & DIRENT_CHANGED) { > + if (ext2fs_get_rec_len(fs, dirent, &rec_len)) > + return BLOCK_ABORT; > + changed++; > + } > + if (ret & DIRENT_ABORT) { > + do_abort++; > + break; > + } > +next: > + if (next_real_entry == offset) > + next_real_entry += rec_len; > + > + if (ctx->flags & DIRENT_FLAG_INCLUDE_REMOVED) { > + size = ((dirent->name_len & 0xFF) + 11) & ~3; > + > + if (rec_len != size) { > + unsigned int final_offset; > + > + final_offset = offset + rec_len; > + offset += size; > + while (offset < final_offset && > + !ext2fs_validate_entry(fs, buf, > + offset, > + final_offset)) > + offset += 4; > + continue; > + } > + } > + offset += rec_len; > + } > + > + if (changed) { > + /* change parent ino */ > + if (buf_len == EXT2_DIR_REC_LEN(2)) > + ((struct ext2_dir_entry *)inode->i_block)->inode = > + dirent->inode; > + ctx->errcode = ext2fs_write_inode_full(fs, ctx->dir, (void *)inode, > + EXT2_INODE_SIZE(fs->super)); > + if (ctx->errcode) > + return BLOCK_ABORT; > + } > + if (do_abort) > + return BLOCK_ABORT; > + return 0; > +} > diff --git a/lib/ext2fs/ext2_err.et.in b/lib/ext2fs/ext2_err.et.in > index 7e6d6e5..8a19cab 100644 > --- a/lib/ext2fs/ext2_err.et.in > +++ b/lib/ext2fs/ext2_err.et.in > @@ -68,6 +68,9 @@ ec EXT2_ET_MAGIC_EXTENT_HANDLE, > ec EXT2_ET_BAD_MAGIC, > "Bad magic number in super-block" > > +ec EXT2_ET_BAD_EXT_ATTR_MAGIC, > + "Bad magic number in extend attribute" "extended attribute" > + > ec EXT2_ET_REV_TOO_HIGH, > "Filesystem revision too high" > > @@ -479,4 +482,7 @@ ec EXT2_ET_FILE_EXISTS, > ec EXT2_ET_EXT_ATTR_CURRUPTED, > "Extended attribute currupted" > > +ec EXT2_ET_BAD_EXTRA_SIZE, > + "Bad inode extra isizevalue" "i_extra_isize value" > + > end > diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h > index 0c0bbcb..d49b95c 100644 > --- a/lib/ext2fs/ext2_fs.h > +++ b/lib/ext2fs/ext2_fs.h > @@ -906,4 +906,12 @@ struct mmp_struct { > */ > #define EXT4_MMP_MIN_CHECK_INTERVAL 5 > > +struct inline_data { > + __u16 inline_off; > + __u16 inline_size; > +}; > + > +#define EXT4_MIN_INLINE_DATA_SIZE ((sizeof(__u32) * EXT2_N_BLOCKS)) > +#define EXT4_INLINE_DATA_DOTDOT_SIZE (4) > + > #endif /* _LINUX_EXT2_FS_H */ > diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h > index 8c30197..7b05b48 100644 > --- a/lib/ext2fs/ext2fs.h > +++ b/lib/ext2fs/ext2fs.h > @@ -1344,6 +1344,17 @@ extern errcode_t ext2fs_get_memalign(unsigned long size, > > /* inline_data.c */ > extern int ext2fs_inode_has_inline_data(ext2_filsys fs, ext2_ino_t ino); > +extern errcode_t ext2fs_inline_data_iterate(ext2_filsys fs, > + ext2_ino_t ino, > + int flags, > + char *block_buf, > + int (*func)(ext2_filsys fs, > + char *buf, > + unsigned int buf_len, > + e2_blkcnt_t blockcnt, > + struct ext2_inode_large *inode, > + void *priv_data), > + void *priv_data); > > /* inode.c */ > extern void ext2fs_free_inode_cache(struct ext2_inode_cache *icache); > diff --git a/lib/ext2fs/ext2fsP.h b/lib/ext2fs/ext2fsP.h > index 3de9278..e86d452 100644 > --- a/lib/ext2fs/ext2fsP.h > +++ b/lib/ext2fs/ext2fsP.h > @@ -87,6 +87,17 @@ extern int ext2fs_process_dir_block(ext2_filsys fs, > int ref_offset, > void *priv_data); > > +extern int ext2fs_process_dir_inline_data(ext2_filsys fs, > + char *buf, > + unsigned int buf_len, > + e2_blkcnt_t blockcnt, > + struct ext2_inode_large *inode, > + void *priv_data); > + > +extern errcode_t ext2fs_inline_data_find(ext2_filsys fs, > + struct ext2_inode_large *inode, > + struct inline_data *data); > + > /* Generic numeric progress meter */ > > struct ext2fs_numeric_progress_struct { > diff --git a/lib/ext2fs/inline_data.c b/lib/ext2fs/inline_data.c > index fcf82b5..922c959 100644 > --- a/lib/ext2fs/inline_data.c > +++ b/lib/ext2fs/inline_data.c > @@ -14,10 +14,60 @@ > #include <time.h> > > #include "ext2_fs.h" > +#include "ext2_ext_attr.h" > > #include "ext2fs.h" > #include "ext2fsP.h" > > +static void *ext2fs_get_inline_xattr_pos(struct ext2_inode_large *inode, > + struct inline_data *data); > + > +errcode_t ext2fs_inline_data_find(ext2_filsys fs, > + struct ext2_inode_large *inode, > + struct inline_data *data) > +{ > + errcode_t retval; > + > + struct ext2_ext_attr_search s = { > + .not_found = ENODATA, > + }; > + struct ext2_ext_attr_info i = { > + .name_index = EXT4_EXT_ATTR_INDEX_SYSTEM, > + .name = EXT4_EXT_ATTR_SYSTEM_DATA, > + }; > + > + data->inline_off = 0; > + if (inode->i_extra_isize > (EXT2_INODE_SIZE(fs->super) - > + EXT2_GOOD_OLD_INODE_SIZE)) > + return EXT2_ET_BAD_EXTRA_SIZE; > + > + retval = ext2fs_ext_attr_ibody_find(fs, inode, &i, &s); > + if (retval) > + return retval; > + > + if (!s.not_found) { > + data->inline_off = (__u16)((char *)s.here - (char *)inode); > + data->inline_size = EXT4_MIN_INLINE_DATA_SIZE + > + s.here->e_value_size; > + return 0; > + } > + > + return EXT2_ET_BAD_EXT_ATTR_MAGIC; > +} > + > +static void *ext2fs_get_inline_xattr_pos(struct ext2_inode_large *inode, > + struct inline_data *data) > +{ > + struct ext2_ext_attr_entry *entry; > + struct ext2_ext_attr_ibody_header *header; > + > + header = IHDR(inode); > + entry = (struct ext2_ext_attr_entry *) > + ((char *)inode + data->inline_off); > + > + return (void *) (IFIRST(header) + entry->e_value_offs); > +} > + > int ext2fs_inode_has_inline_data(ext2_filsys fs, ext2_ino_t ino) > { > struct ext2_inode inode; > @@ -29,3 +79,85 @@ int ext2fs_inode_has_inline_data(ext2_filsys fs, ext2_ino_t ino) > > return (inode.i_flags & EXT4_INLINE_DATA_FL); > } > + > +errcode_t ext2fs_inline_data_iterate(ext2_filsys fs, > + ext2_ino_t ino, > + int flags, > + char *block_buf, > + int (*func)(ext2_filsys fs, > + char *buf, > + unsigned int buf_len, > + e2_blkcnt_t blockcnt, > + struct ext2_inode_large *inode, > + void *priv_data), > + void *priv_data) > +{ > + struct dir_context *ctx; > + struct ext2_inode_large *inode; > + struct ext2_dir_entry dirent; > + struct inline_data data; > + errcode_t retval = 0; > + e2_blkcnt_t blockcnt = 0; > + void *inline_start; > + int inline_size; > + > + ctx = (struct dir_context *)priv_data; > + > + retval = ext2fs_get_mem(EXT2_INODE_SIZE(fs->super), &inode); > + if (retval) > + return retval; > + > + retval = ext2fs_read_inode_full(fs, ino, (void *)inode, > + EXT2_INODE_SIZE(fs->super)); > + if (retval) > + goto out; > + > + if (inode->i_size == 0) > + goto out; > + > + /* we first check '.' and '..' dir */ > + dirent.inode = ino; > + dirent.name_len = 1; > + ext2fs_set_rec_len(fs, EXT2_DIR_REC_LEN(2), &dirent); > + dirent.name[0] = '.'; > + dirent.name[1] = '\0'; > + retval |= (*func)(fs, (void *)&dirent, dirent.rec_len, blockcnt++, > + inode, priv_data); > + if (retval & BLOCK_ABORT) > + goto out; > + > + dirent.inode = (__u32)*inode->i_block; > + dirent.name_len = 2; > + ext2fs_set_rec_len(fs, EXT2_DIR_REC_LEN(3), &dirent); > + dirent.name[0] = '.'; > + dirent.name[1] = '.'; > + dirent.name[2] = '\0'; > + retval |= (*func)(fs, (void *)&dirent, dirent.rec_len, blockcnt++, > + inode, priv_data); > + if (retval & BLOCK_ABORT) > + goto out; Perhaps this function should be called ext2fs_inline_dirblock_iterate()? The name alone makes me think this function could work for regular inline_data files, if such things ever exist. --D > + > + inline_start = (char *)inode->i_block + EXT4_INLINE_DATA_DOTDOT_SIZE; > + inline_size = EXT4_MIN_INLINE_DATA_SIZE - EXT4_INLINE_DATA_DOTDOT_SIZE; > + retval |= (*func)(fs, inline_start, inline_size, blockcnt++, > + inode, priv_data); > + if (retval & BLOCK_ABORT) > + goto out; > + > + retval = ext2fs_inline_data_find(fs, inode, &data); > + if (retval) > + goto out; > + if (data.inline_size > EXT4_MIN_INLINE_DATA_SIZE) { > + inline_start = ext2fs_get_inline_xattr_pos(inode, &data); > + inline_size = data.inline_size - EXT4_MIN_INLINE_DATA_SIZE; > + retval |= (*func)(fs, inline_start, inline_size, blockcnt++, > + inode, priv_data); > + if (retval & BLOCK_ABORT) > + goto out; > + } > + > +out: > + retval |= BLOCK_ERROR; > + ext2fs_free_mem(&inode); > + return retval & BLOCK_ERROR ? ctx->errcode : 0; > +} > -- > 1.7.9.7 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 11, 2013 at 04:33:42PM -0700, Darrick J. Wong wrote: [..] > > +errcode_t ext2fs_inline_data_iterate(ext2_filsys fs, > > + ext2_ino_t ino, > > + int flags, > > + char *block_buf, > > + int (*func)(ext2_filsys fs, > > + char *buf, > > + unsigned int buf_len, > > + e2_blkcnt_t blockcnt, > > + struct ext2_inode_large *inode, > > + void *priv_data), > > + void *priv_data) > > +{ > > + struct dir_context *ctx; > > + struct ext2_inode_large *inode; > > + struct ext2_dir_entry dirent; > > + struct inline_data data; > > + errcode_t retval = 0; > > + e2_blkcnt_t blockcnt = 0; > > + void *inline_start; > > + int inline_size; > > + > > + ctx = (struct dir_context *)priv_data; > > + > > + retval = ext2fs_get_mem(EXT2_INODE_SIZE(fs->super), &inode); > > + if (retval) > > + return retval; > > + > > + retval = ext2fs_read_inode_full(fs, ino, (void *)inode, > > + EXT2_INODE_SIZE(fs->super)); > > + if (retval) > > + goto out; > > + > > + if (inode->i_size == 0) > > + goto out; > > + > > + /* we first check '.' and '..' dir */ > > + dirent.inode = ino; > > + dirent.name_len = 1; > > + ext2fs_set_rec_len(fs, EXT2_DIR_REC_LEN(2), &dirent); > > + dirent.name[0] = '.'; > > + dirent.name[1] = '\0'; > > + retval |= (*func)(fs, (void *)&dirent, dirent.rec_len, blockcnt++, > > + inode, priv_data); > > + if (retval & BLOCK_ABORT) > > + goto out; > > + > > + dirent.inode = (__u32)*inode->i_block; > > + dirent.name_len = 2; > > + ext2fs_set_rec_len(fs, EXT2_DIR_REC_LEN(3), &dirent); > > + dirent.name[0] = '.'; > > + dirent.name[1] = '.'; > > + dirent.name[2] = '\0'; > > + retval |= (*func)(fs, (void *)&dirent, dirent.rec_len, blockcnt++, > > + inode, priv_data); > > + if (retval & BLOCK_ABORT) > > + goto out; > > Perhaps this function should be called ext2fs_inline_dirblock_iterate()? > The name alone makes me think this function could work for regular inline_data > files, if such things ever exist. Fair enough. I will fix it in next version. Thanks, - Zheng > > --D > > > + > > + inline_start = (char *)inode->i_block + EXT4_INLINE_DATA_DOTDOT_SIZE; > > + inline_size = EXT4_MIN_INLINE_DATA_SIZE - EXT4_INLINE_DATA_DOTDOT_SIZE; > > + retval |= (*func)(fs, inline_start, inline_size, blockcnt++, > > + inode, priv_data); > > + if (retval & BLOCK_ABORT) > > + goto out; > > + > > + retval = ext2fs_inline_data_find(fs, inode, &data); > > + if (retval) > > + goto out; > > + if (data.inline_size > EXT4_MIN_INLINE_DATA_SIZE) { > > + inline_start = ext2fs_get_inline_xattr_pos(inode, &data); > > + inline_size = data.inline_size - EXT4_MIN_INLINE_DATA_SIZE; > > + retval |= (*func)(fs, inline_start, inline_size, blockcnt++, > > + inode, priv_data); > > + if (retval & BLOCK_ABORT) > > + goto out; > > + } > > + > > +out: > > + retval |= BLOCK_ERROR; > > + ext2fs_free_mem(&inode); > > + return retval & BLOCK_ERROR ? ctx->errcode : 0; > > +} > > -- > > 1.7.9.7 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > > the body of a message to majordomo@vger.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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 02, 2013 at 05:49:31PM +0800, Zheng Liu wrote: > +int ext2fs_process_dir_inline_data(ext2_filsys fs, > + char *buf, > + unsigned int buf_len, > + e2_blkcnt_t blockcnt, > + struct ext2_inode_large *inode, > + void *priv_data) > +{ It looks like there is a lot of code in this function which is in common with ext2fs_process_dir_block(), so I'd suggest refactoring out the common code to reduce duplication. This will reduce code size, and more importantly, improve maintenance of the code. > +errcode_t ext2fs_inline_data_iterate(ext2_filsys fs, > + ext2_ino_t ino, > + int flags, > + char *block_buf, > + int (*func)(ext2_filsys fs, > + char *buf, > + unsigned int buf_len, > + e2_blkcnt_t blockcnt, > + struct ext2_inode_large *inode, > + void *priv_data), > + void *priv_data) This function is misnamed, which worries me a little. First of all, it only makes sense when called on directories, so some name that indicates that it is meant to iterate over directories is a good idea. so some name such as ext2fs_process_inline_data_dir might be a better choice. Secondly, it would a really good idea if there was a check to make sure it was passed an inode number which corresponds to an directory and that the inline data flag is set. A little paranoia is really healthy thing --- if we have some application bug where this function gets called accidentally on an inappropriate inode, we want to return a clean error code and not stumble on until something bad happens. > + dirent.inode = (__u32)*inode->i_block; I'd be much happier with: dirent.inode = inode->i_block[0]; We shouldn't use casts unless absolutely necessary, and it's not necessary here. Also, I suspect we have some byte-swapping problems here. It doesn't appear there is any allownaces for byte swapping in the inline data patches. Currently, the ext2fs_read_inode() function will take care of byte swapping i_blocks[], so that will be OK here, but in the case of an inode with inline data, if we byte swap all of i_blocks[] then ext2fs_read_inline_data() will malfunction since the data bytes stored in the rest of i_blocks[] will be byte swapped. And that would be wrong. So I think what you will need to do is to avoid byte swapping the i_blocks[] array if the inode contains inline_data, and then in the case where this is a directory, we will need to byte swap i_block[0] if we are running on a big-endian system. Cheers, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 02, 2013 at 05:49:31PM +0800, Zheng Liu wrote: > diff --git a/lib/ext2fs/ext2_err.et.in b/lib/ext2fs/ext2_err.et.in > index 7e6d6e5..8a19cab 100644 > --- a/lib/ext2fs/ext2_err.et.in > +++ b/lib/ext2fs/ext2_err.et.in > @@ -68,6 +68,9 @@ ec EXT2_ET_MAGIC_EXTENT_HANDLE, > ec EXT2_ET_BAD_MAGIC, > "Bad magic number in super-block" > > +ec EXT2_ET_BAD_EXT_ATTR_MAGIC, > + "Bad magic number in extend attribute" > + > ec EXT2_ET_REV_TOO_HIGH, > "Filesystem revision too high" Error cods MUST always be added at the end of the error code table. Otherwise, the error code numbers will change, and that will break the ABI --- i.e., #define EXT2_ET_REV_TOO_HIGH (2133571348L) might turn into #define EXT2_ET_REV_TOO_HIGH (2133571349L) and that will cause no end of confusion when older applications link with a newer version of the libext2fs shared library. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Oct 13, 2013 at 06:51:12PM -0400, Theodore Ts'o wrote: > On Fri, Aug 02, 2013 at 05:49:31PM +0800, Zheng Liu wrote: > > +int ext2fs_process_dir_inline_data(ext2_filsys fs, > > + char *buf, > > + unsigned int buf_len, > > + e2_blkcnt_t blockcnt, > > + struct ext2_inode_large *inode, > > + void *priv_data) > > +{ > > It looks like there is a lot of code in this function which is in > common with ext2fs_process_dir_block(), so I'd suggest refactoring out > the common code to reduce duplication. This will reduce code size, > and more importantly, improve maintenance of the code. > > > +errcode_t ext2fs_inline_data_iterate(ext2_filsys fs, > > + ext2_ino_t ino, > > + int flags, > > + char *block_buf, > > + int (*func)(ext2_filsys fs, > > + char *buf, > > + unsigned int buf_len, > > + e2_blkcnt_t blockcnt, > > + struct ext2_inode_large *inode, > > + void *priv_data), > > + void *priv_data) > > This function is misnamed, which worries me a little. First of all, > it only makes sense when called on directories, so some name that > indicates that it is meant to iterate over directories is a good idea. > so some name such as ext2fs_process_inline_data_dir might be a better > choice. Yes, Darrick has pointed it out. I will fix it in next version. > > Secondly, it would a really good idea if there was a check to make > sure it was passed an inode number which corresponds to an directory > and that the inline data flag is set. A little paranoia is really > healthy thing --- if we have some application bug where this function > gets called accidentally on an inappropriate inode, we want to return > a clean error code and not stumble on until something bad happens. > > > + dirent.inode = (__u32)*inode->i_block; > > I'd be much happier with: > > dirent.inode = inode->i_block[0]; > > We shouldn't use casts unless absolutely necessary, and it's not > necessary here. > > Also, I suspect we have some byte-swapping problems here. It doesn't > appear there is any allownaces for byte swapping in the inline data > patches. Currently, the ext2fs_read_inode() function will take care > of byte swapping i_blocks[], so that will be OK here, but in the case > of an inode with inline data, if we byte swap all of i_blocks[] then > ext2fs_read_inline_data() will malfunction since the data bytes stored > in the rest of i_blocks[] will be byte swapped. And that would be > wrong. > > So I think what you will need to do is to avoid byte swapping the > i_blocks[] array if the inode contains inline_data, and then in the > case where this is a directory, we will need to byte swap i_block[0] > if we are running on a big-endian system. Yes, I have noticed that we only byte swap i_block[0], and the following things don't be swapped. So I will fix it. Thanks, - Zheng -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/lib/ext2fs/dir_iterate.c b/lib/ext2fs/dir_iterate.c index 1a4bf5c..7d5b1da 100644 --- a/lib/ext2fs/dir_iterate.c +++ b/lib/ext2fs/dir_iterate.c @@ -123,8 +123,14 @@ errcode_t ext2fs_dir_iterate2(ext2_filsys fs, ctx.func = func; ctx.priv_data = priv_data; ctx.errcode = 0; - retval = ext2fs_block_iterate3(fs, dir, BLOCK_FLAG_READ_ONLY, 0, - ext2fs_process_dir_block, &ctx); + if (ext2fs_inode_has_inline_data(fs, dir)) + retval = ext2fs_inline_data_iterate(fs, dir, + BLOCK_FLAG_READ_ONLY, 0, + ext2fs_process_dir_inline_data, + &ctx); + else + retval = ext2fs_block_iterate3(fs, dir, BLOCK_FLAG_READ_ONLY, 0, + ext2fs_process_dir_block, &ctx); if (!block_buf) ext2fs_free_mem(&ctx.buf); if (retval) @@ -282,3 +288,95 @@ next: return 0; } +int ext2fs_process_dir_inline_data(ext2_filsys fs, + char *buf, + unsigned int buf_len, + e2_blkcnt_t blockcnt, + struct ext2_inode_large *inode, + void *priv_data) +{ + struct dir_context *ctx = (struct dir_context *) priv_data; + unsigned int offset = 0; + unsigned int next_real_entry = 0; + int ret = 0; + int changed = 0; + int do_abort = 0; + unsigned int rec_len, size; + int entry; + struct ext2_dir_entry *dirent; + + if (blockcnt < 0) + return 0; + + entry = blockcnt ? DIRENT_OTHER_FILE : DIRENT_DOT_FILE; + + while (offset < buf_len) { + dirent = (struct ext2_dir_entry *) (buf + offset); + if (ext2fs_get_rec_len(fs, dirent, &rec_len)) + return BLOCK_ABORT; + if (((offset + rec_len) > buf_len) || + (rec_len < 8) || + ((rec_len % 4) != 0) || + ((((unsigned) dirent->name_len & 0xFF)+8) > rec_len)) { + ctx->errcode = EXT2_ET_DIR_CORRUPTED; + return BLOCK_ABORT; + } + if (!dirent->inode && + !(ctx->flags & DIRENT_FLAG_INCLUDE_EMPTY)) + goto next; + + ret = (ctx->func)(ctx->dir, + (next_real_entry > offset) ? + DIRENT_DELETED_FILE : entry, + dirent, offset, + buf_len, buf, + ctx->priv_data); + if (entry < DIRENT_OTHER_FILE) + entry++; + + if (ret & DIRENT_CHANGED) { + if (ext2fs_get_rec_len(fs, dirent, &rec_len)) + return BLOCK_ABORT; + changed++; + } + if (ret & DIRENT_ABORT) { + do_abort++; + break; + } +next: + if (next_real_entry == offset) + next_real_entry += rec_len; + + if (ctx->flags & DIRENT_FLAG_INCLUDE_REMOVED) { + size = ((dirent->name_len & 0xFF) + 11) & ~3; + + if (rec_len != size) { + unsigned int final_offset; + + final_offset = offset + rec_len; + offset += size; + while (offset < final_offset && + !ext2fs_validate_entry(fs, buf, + offset, + final_offset)) + offset += 4; + continue; + } + } + offset += rec_len; + } + + if (changed) { + /* change parent ino */ + if (buf_len == EXT2_DIR_REC_LEN(2)) + ((struct ext2_dir_entry *)inode->i_block)->inode = + dirent->inode; + ctx->errcode = ext2fs_write_inode_full(fs, ctx->dir, (void *)inode, + EXT2_INODE_SIZE(fs->super)); + if (ctx->errcode) + return BLOCK_ABORT; + } + if (do_abort) + return BLOCK_ABORT; + return 0; +} diff --git a/lib/ext2fs/ext2_err.et.in b/lib/ext2fs/ext2_err.et.in index 7e6d6e5..8a19cab 100644 --- a/lib/ext2fs/ext2_err.et.in +++ b/lib/ext2fs/ext2_err.et.in @@ -68,6 +68,9 @@ ec EXT2_ET_MAGIC_EXTENT_HANDLE, ec EXT2_ET_BAD_MAGIC, "Bad magic number in super-block" +ec EXT2_ET_BAD_EXT_ATTR_MAGIC, + "Bad magic number in extend attribute" + ec EXT2_ET_REV_TOO_HIGH, "Filesystem revision too high" @@ -479,4 +482,7 @@ ec EXT2_ET_FILE_EXISTS, ec EXT2_ET_EXT_ATTR_CURRUPTED, "Extended attribute currupted" +ec EXT2_ET_BAD_EXTRA_SIZE, + "Bad inode extra isizevalue" + end diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h index 0c0bbcb..d49b95c 100644 --- a/lib/ext2fs/ext2_fs.h +++ b/lib/ext2fs/ext2_fs.h @@ -906,4 +906,12 @@ struct mmp_struct { */ #define EXT4_MMP_MIN_CHECK_INTERVAL 5 +struct inline_data { + __u16 inline_off; + __u16 inline_size; +}; + +#define EXT4_MIN_INLINE_DATA_SIZE ((sizeof(__u32) * EXT2_N_BLOCKS)) +#define EXT4_INLINE_DATA_DOTDOT_SIZE (4) + #endif /* _LINUX_EXT2_FS_H */ diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h index 8c30197..7b05b48 100644 --- a/lib/ext2fs/ext2fs.h +++ b/lib/ext2fs/ext2fs.h @@ -1344,6 +1344,17 @@ extern errcode_t ext2fs_get_memalign(unsigned long size, /* inline_data.c */ extern int ext2fs_inode_has_inline_data(ext2_filsys fs, ext2_ino_t ino); +extern errcode_t ext2fs_inline_data_iterate(ext2_filsys fs, + ext2_ino_t ino, + int flags, + char *block_buf, + int (*func)(ext2_filsys fs, + char *buf, + unsigned int buf_len, + e2_blkcnt_t blockcnt, + struct ext2_inode_large *inode, + void *priv_data), + void *priv_data); /* inode.c */ extern void ext2fs_free_inode_cache(struct ext2_inode_cache *icache); diff --git a/lib/ext2fs/ext2fsP.h b/lib/ext2fs/ext2fsP.h index 3de9278..e86d452 100644 --- a/lib/ext2fs/ext2fsP.h +++ b/lib/ext2fs/ext2fsP.h @@ -87,6 +87,17 @@ extern int ext2fs_process_dir_block(ext2_filsys fs, int ref_offset, void *priv_data); +extern int ext2fs_process_dir_inline_data(ext2_filsys fs, + char *buf, + unsigned int buf_len, + e2_blkcnt_t blockcnt, + struct ext2_inode_large *inode, + void *priv_data); + +extern errcode_t ext2fs_inline_data_find(ext2_filsys fs, + struct ext2_inode_large *inode, + struct inline_data *data); + /* Generic numeric progress meter */ struct ext2fs_numeric_progress_struct { diff --git a/lib/ext2fs/inline_data.c b/lib/ext2fs/inline_data.c index fcf82b5..922c959 100644 --- a/lib/ext2fs/inline_data.c +++ b/lib/ext2fs/inline_data.c @@ -14,10 +14,60 @@ #include <time.h> #include "ext2_fs.h" +#include "ext2_ext_attr.h" #include "ext2fs.h" #include "ext2fsP.h" +static void *ext2fs_get_inline_xattr_pos(struct ext2_inode_large *inode, + struct inline_data *data); + +errcode_t ext2fs_inline_data_find(ext2_filsys fs, + struct ext2_inode_large *inode, + struct inline_data *data) +{ + errcode_t retval; + + struct ext2_ext_attr_search s = { + .not_found = ENODATA, + }; + struct ext2_ext_attr_info i = { + .name_index = EXT4_EXT_ATTR_INDEX_SYSTEM, + .name = EXT4_EXT_ATTR_SYSTEM_DATA, + }; + + data->inline_off = 0; + if (inode->i_extra_isize > (EXT2_INODE_SIZE(fs->super) - + EXT2_GOOD_OLD_INODE_SIZE)) + return EXT2_ET_BAD_EXTRA_SIZE; + + retval = ext2fs_ext_attr_ibody_find(fs, inode, &i, &s); + if (retval) + return retval; + + if (!s.not_found) { + data->inline_off = (__u16)((char *)s.here - (char *)inode); + data->inline_size = EXT4_MIN_INLINE_DATA_SIZE + + s.here->e_value_size; + return 0; + } + + return EXT2_ET_BAD_EXT_ATTR_MAGIC; +} + +static void *ext2fs_get_inline_xattr_pos(struct ext2_inode_large *inode, + struct inline_data *data) +{ + struct ext2_ext_attr_entry *entry; + struct ext2_ext_attr_ibody_header *header; + + header = IHDR(inode); + entry = (struct ext2_ext_attr_entry *) + ((char *)inode + data->inline_off); + + return (void *) (IFIRST(header) + entry->e_value_offs); +} + int ext2fs_inode_has_inline_data(ext2_filsys fs, ext2_ino_t ino) { struct ext2_inode inode; @@ -29,3 +79,85 @@ int ext2fs_inode_has_inline_data(ext2_filsys fs, ext2_ino_t ino) return (inode.i_flags & EXT4_INLINE_DATA_FL); } + +errcode_t ext2fs_inline_data_iterate(ext2_filsys fs, + ext2_ino_t ino, + int flags, + char *block_buf, + int (*func)(ext2_filsys fs, + char *buf, + unsigned int buf_len, + e2_blkcnt_t blockcnt, + struct ext2_inode_large *inode, + void *priv_data), + void *priv_data) +{ + struct dir_context *ctx; + struct ext2_inode_large *inode; + struct ext2_dir_entry dirent; + struct inline_data data; + errcode_t retval = 0; + e2_blkcnt_t blockcnt = 0; + void *inline_start; + int inline_size; + + ctx = (struct dir_context *)priv_data; + + retval = ext2fs_get_mem(EXT2_INODE_SIZE(fs->super), &inode); + if (retval) + return retval; + + retval = ext2fs_read_inode_full(fs, ino, (void *)inode, + EXT2_INODE_SIZE(fs->super)); + if (retval) + goto out; + + if (inode->i_size == 0) + goto out; + + /* we first check '.' and '..' dir */ + dirent.inode = ino; + dirent.name_len = 1; + ext2fs_set_rec_len(fs, EXT2_DIR_REC_LEN(2), &dirent); + dirent.name[0] = '.'; + dirent.name[1] = '\0'; + retval |= (*func)(fs, (void *)&dirent, dirent.rec_len, blockcnt++, + inode, priv_data); + if (retval & BLOCK_ABORT) + goto out; + + dirent.inode = (__u32)*inode->i_block; + dirent.name_len = 2; + ext2fs_set_rec_len(fs, EXT2_DIR_REC_LEN(3), &dirent); + dirent.name[0] = '.'; + dirent.name[1] = '.'; + dirent.name[2] = '\0'; + retval |= (*func)(fs, (void *)&dirent, dirent.rec_len, blockcnt++, + inode, priv_data); + if (retval & BLOCK_ABORT) + goto out; + + inline_start = (char *)inode->i_block + EXT4_INLINE_DATA_DOTDOT_SIZE; + inline_size = EXT4_MIN_INLINE_DATA_SIZE - EXT4_INLINE_DATA_DOTDOT_SIZE; + retval |= (*func)(fs, inline_start, inline_size, blockcnt++, + inode, priv_data); + if (retval & BLOCK_ABORT) + goto out; + + retval = ext2fs_inline_data_find(fs, inode, &data); + if (retval) + goto out; + if (data.inline_size > EXT4_MIN_INLINE_DATA_SIZE) { + inline_start = ext2fs_get_inline_xattr_pos(inode, &data); + inline_size = data.inline_size - EXT4_MIN_INLINE_DATA_SIZE; + retval |= (*func)(fs, inline_start, inline_size, blockcnt++, + inode, priv_data); + if (retval & BLOCK_ABORT) + goto out; + } + +out: + retval |= BLOCK_ERROR; + ext2fs_free_mem(&inode); + return retval & BLOCK_ERROR ? ctx->errcode : 0; +}