On Thu, Jul 24, 2014 at 04:41:04PM -0600, Andreas Dilger wrote: > On Jul 24, 2014, at 3:22 PM, Darrick J. Wong <darrick.wong@xxxxxxxxxx> wrote: > > Before converting an inline directory to a regular directory, check > > the directory entries to make sure they're not obviously broken. > > This helps us to avoid a BUG_ON if one of the dirents is trashed. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > --- > > fs/ext4/inline.c | 37 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > > > diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c > > index 645205d..c79d8e7 100644 > > --- a/fs/ext4/inline.c > > +++ b/fs/ext4/inline.c > > @@ -1157,6 +1157,31 @@ out: > > return err; > > } > > > > +static int ext4_check_all_de(struct inode *dir, struct buffer_head *bh, > > + void *buf, int buf_size) > > It would seem that this function would be useful in ext4_readdir(), which > open-codes something very similar? It might be better to locate this > function in ext4/dir.c since it doesn't appear to have anything specific > to do with inline directories. I don't have any problem with moving it to dir.c. As for _readdir, while it does open-code a similar loop, I don't know if it's really worth it to try to shoe-horn what it does into the same function, since it has to update ctx->pos and call dir_emit when it finds a valid entry. Assuming you're talking about the while loop towards the end of the function. --D > > Cheers, Andreas > > > +{ > > + struct ext4_dir_entry_2 *de; > > + int nlen, rlen; > > + unsigned int offset = 0; > > + char *top; > > + > > + de = (struct ext4_dir_entry_2 *)buf; > > + top = buf + buf_size; > > + while ((char *) de < top) { > > + if (ext4_check_dir_entry(dir, NULL, de, bh, > > + buf, buf_size, offset)) > > + return -EIO; > > + nlen = EXT4_DIR_REC_LEN(de->name_len); > > + rlen = ext4_rec_len_from_disk(de->rec_len, buf_size); > > + de = (struct ext4_dir_entry_2 *)((char *)de + rlen); > > + offset += rlen; > > + } > > + if ((char *) de > top) > > + return -EIO; > > + > > + return 0; > > +} > > + > > static int ext4_convert_inline_data_nolock(handle_t *handle, > > struct inode *inode, > > struct ext4_iloc *iloc) > > @@ -1178,6 +1203,18 @@ static int ext4_convert_inline_data_nolock(handle_t *handle, > > if (error < 0) > > goto out; > > > > + /* > > + * Make sure the inline directory entries pass checks before we try to > > + * convert them, so that we avoid touching stuff that needs fsck. > > + */ > > + if (S_ISDIR(inode->i_mode)) { > > + error = ext4_check_all_de(inode, iloc->bh, > > + buf + EXT4_INLINE_DOTDOT_SIZE, > > + inline_size - EXT4_INLINE_DOTDOT_SIZE); > > + if (error) > > + goto out; > > + } > > + > > error = ext4_destroy_inline_data_nolock(handle, inode); > > if (error) > > goto out; > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > Cheers, Andreas > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html