[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20140724225739.GH8628@birch.djwong.org>
Date: Thu, 24 Jul 2014 15:57:39 -0700
From: "Darrick J. Wong" <darrick.wong@...cle.com>
To: Andreas Dilger <adilger@...ger.ca>
Cc: "Theodore Ts'o" <tytso@....edu>,
Zheng Liu <gnehzuil.liu@...il.com>,
linux-ext4 <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] ext4: check inline directory before converting
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@...cle.com> 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@...cle.com>
> > ---
> > 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@...r.kernel.org
> > 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists