The 2.6.10 ext3_get_parent attempts to use ext3_find_entry to look up the
entry "..", which fails for dx directories since ".." is not present in
the directory hash table. The patch below solves this by looking up the
dotdot entry in the dx_root block.
Typical symptoms of the above bug are intermittent claims by nfsd that
files or directories are missing on exported ext3 filesystems.
cf https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=150759
and https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=144556
Signed-off-by: Henrik Grubbstr?m <[email protected]>
--- linux/fs/ext3/namei.c.orig 2005-05-06 17:08:31.000000000 +0200
+++ linux/fs/ext3/namei.c 2005-05-07 17:57:24.000000000 +0200
@@ -999,24 +999,90 @@ static struct dentry *ext3_lookup(struct
struct dentry *ext3_get_parent(struct dentry *child)
{
- unsigned long ino;
+ unsigned long ino = 0;
struct dentry *parent;
struct inode *inode;
- struct dentry dotdot;
- struct ext3_dir_entry_2 * de;
struct buffer_head *bh;
+ int err;
+ struct inode *dir = child->d_inode;
+ struct super_block *sb = dir->i_sb;
- dotdot.d_name.name = "..";
- dotdot.d_name.len = 2;
- dotdot.d_parent = child; /* confusing, isn't it! */
+ /* In all cases the ".." entry is in block #0, so read it. */
+ bh = ext3_bread(NULL, dir, 0, 0, &err);
+ if (!bh) return ERR_PTR(err);
+
+#ifdef CONFIG_EXT3_INDEX
+ if (is_dx(dir)) {
+ /* Block #0 is a dx_root. */
+ struct dx_root *root = (struct dx_root *)bh->b_data;
+ if (root->info.hash_version == DX_HASH_TEA ||
+ root->info.hash_version == DX_HASH_HALF_MD4 ||
+ root->info.hash_version == DX_HASH_LEGACY) {
+ /* Is this paranoia actually needed? */
+ ino = le32_to_cpu(root->dotdot.inode);
+ } else {
+ ext3_error(sb, __FUNCTION__,
+ "unsupported dx hash_version (%d) for #%lu\n",
+ root->info.hash_version, dir->i_ino);
+ }
+ }
+ if (!ino)
+#endif
+ {
+ /* Block #0 is a sequence of ext3_dir_entry_2.
+ * ".." is the second entry.
+ */
+ struct ext3_dir_entry_2 *de =
+ (struct ext3_dir_entry_2 *)bh->b_data;
+ int len = le16_to_cpu(de->rec_len);
+ if (len > 0) {
+ /* The first entry should be ".". */
+ de = (struct ext3_dir_entry_2 *)((char *)de + len);
+ if ((de->name_len == 2) &&
+ (de->name[0] == '.') &&
+ (de->name[1] == '.')) {
+ /* The second entry should be "..". */
+ ino = le32_to_cpu(de->inode);
+ } else {
+ ext3_warning(sb, __FUNCTION__,
+ "second entry of #%lu not parent",
+ dir->i_ino);
+ }
+ } else {
+ ext3_error(sb, __FUNCTION__,
+ "first entry of #%lu has bad rec_len: %d",
+ dir->i_ino, len);
+ }
+#if 0
+ /* Fallback code to enable if the ".." entry ever can
+ * be somewhere else than in the second entry of the
+ * first block.
+ */
+ if (!ino) {
+ struct dentry dotdot;
+
+ ext3_warning(sb, __FUNCTION__,
+ "falling back to ext3_find_entry for #%lu\n",
+ dir->i_ino);
+ brelse(bh);
+ dotdot.d_name.name = "..";
+ dotdot.d_name.len = 2;
+ dotdot.d_parent = child; /* confusing, isn't it! */
+
+ bh = ext3_find_entry(&dotdot, &de);
+ if (!bh)
+ return ERR_PTR(-ENOENT);
+ ino = le32_to_cpu(de->inode);
+ }
+#endif /* 0 */
+ }
- bh = ext3_find_entry(&dotdot, &de);
- inode = NULL;
- if (!bh)
- return ERR_PTR(-ENOENT);
- ino = le32_to_cpu(de->inode);
brelse(bh);
- inode = iget(child->d_inode->i_sb, ino);
+
+ if (!ino)
+ return ERR_PTR(-ENOENT);
+
+ inode = iget(sb, ino);
if (!inode)
return ERR_PTR(-EACCES);
--
Henrik Grubbstr?m [email protected]
Roxen Internet Software AB [email protected]
On May 09, 2005 10:57 +0200, Henrik Grubbstr?m wrote:
> The 2.6.10 ext3_get_parent attempts to use ext3_find_entry to look up the
> entry "..", which fails for dx directories since ".." is not present in
> the directory hash table. The patch below solves this by looking up the
> dotdot entry in the dx_root block.
>
> Typical symptoms of the above bug are intermittent claims by nfsd that
> files or directories are missing on exported ext3 filesystems.
>
> cf https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=150759
> and https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=144556
>
> Signed-off-by: Henrik Grubbstr?m <[email protected]>
ext3_get_parent() is IMHO the wrong place to fix this bug as it introduces
a lot of internals from htree into that function. Instead, I think this
should be fixed in ext3_find_entry() as in the below patch. This has the
added advantage that it works for any callers of ext3_find_entry() and not
just ext3_lookup_parent().
I thought I submitted this patch to l-k at one point, but now I can't find
any mention of that in the archives...
Signed-off-by: Andreas Diler <[email protected]>
--- linux-2.6.orig/fs/ext3/namei.c 2005-04-04 05:06:46.000000000 -0600
+++ linux-2.6/fs/ext3/namei.c 2005-04-04 05:09:18.000000000 -0600
@@ -926,8 +926,16 @@
struct inode *dir = dentry->d_parent->d_inode;
sb = dir->i_sb;
- if (!(frame = dx_probe(dentry, NULL, &hinfo, frames, err)))
- return NULL;
+ /* NFS may look up ".." - look at dx_root directory block */
+ if (namelen > 2 || name[0] != '.'||(name[1] != '.' && name[1] != '\0')){
+ if (!(frame = dx_probe(dentry, NULL, &hinfo, frames, err)))
+ return NULL;
+ } else {
+ frame = frames;
+ frame->bh = NULL; /* for dx_release() */
+ frame->at = (struct dx_entry *)frames; /* hack for zero entry*/
+ dx_set_block(frame->at, 0); /* dx_root block is 0 */
+ }
hash = hinfo.hash;
do {
block = dx_get_block(frame->at);
Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.
On Mon, 9 May 2005, Andreas Dilger wrote:
> On May 09, 2005 10:57 +0200, Henrik Grubbstr?m wrote:
> > The 2.6.10 ext3_get_parent attempts to use ext3_find_entry to look up the
> > entry "..", which fails for dx directories since ".." is not present in
> > the directory hash table. The patch below solves this by looking up the
> > dotdot entry in the dx_root block.
>
> ext3_get_parent() is IMHO the wrong place to fix this bug as it introduces
> a lot of internals from htree into that function. Instead, I think this
> should be fixed in ext3_find_entry() as in the below patch. This has the
> added advantage that it works for any callers of ext3_find_entry() and not
> just ext3_lookup_parent().
The reason I didn't put it there is that handling of ".." is usually
performed by fs/namei.c:link_path_walk() and putting it in
ext3_find_entry() or one of the functions it calls would slow down the
common case.
> Cheers, Andreas
--
Henrik Grubbstr?m [email protected]
Roxen Internet Software AB
Hi,
On Mon, 2005-05-09 at 10:46, Henrik Grubbstr?m wrote:
> > ext3_get_parent() is IMHO the wrong place to fix this bug as it introduces
> > a lot of internals from htree into that function. Instead, I think this
> > should be fixed in ext3_find_entry() as in the below patch. This has the
> > added advantage that it works for any callers of ext3_find_entry() and not
> > just ext3_lookup_parent().
>
> The reason I didn't put it there is that handling of ".." is usually
> performed by fs/namei.c:link_path_walk() and putting it in
> ext3_find_entry() or one of the functions it calls would slow down the
> common case.
True, but the extra cost is to evaluate
if (namelen > 2 || name[0] != '.'||(name[1] != '.' && name[1] != '\0')
as false. That's not going to take long most of the time. And this
solution has two other big advantages: it fixes things for all lookups,
not just NFS, and hence is safer against this bug cropping up again in
the future in unexpected places; and it includes less dependency on
htree internals, as Andreas said.
I'll have a closer review of the patch, but for now I think I like
Andreas's version better.
Cheers,
Stephen
Andreas Dilger <[email protected]> wrote:
> + if (namelen > 2 || name[0] != '.'||(name[1] != '.' && name[1] != '\0')){
The patch was supposed to affect only '..'.
Maybe you can add a 'unlikely', too.
--
A "sucking chest wound" is nature's way of telling you to slow down.
On Mon, 9 May 2005, Stephen C. Tweedie wrote:
> On Mon, 2005-05-09 at 10:46, Henrik Grubbstr?m wrote:
>
> > > ext3_get_parent() is IMHO the wrong place to fix this bug as it introduces
> > > a lot of internals from htree into that function. Instead, I think this
> > > should be fixed in ext3_find_entry() as in the below patch. This has the
> > > added advantage that it works for any callers of ext3_find_entry() and not
> > > just ext3_lookup_parent().
> >
> > The reason I didn't put it there is that handling of ".." is usually
> > performed by fs/namei.c:link_path_walk() and putting it in
> > ext3_find_entry() or one of the functions it calls would slow down the
> > common case.
>
> True, but the extra cost is to evaluate
>
> if (namelen > 2 || name[0] != '.'||(name[1] != '.' && name[1] != '\0')
>
> as false. That's not going to take long most of the time. And this
> solution has two other big advantages: it fixes things for all lookups,
> not just NFS, and hence is safer against this bug cropping up again in
> the future in unexpected places; and it includes less dependency on
> htree internals, as Andreas said.
All true.
Since the htree stuff is implemented in the same file and the get_parent
operation is rather basic I didn't see much point. The advantages of my
patch are that it looks at a single directory block and that it doesn't
affect the i_dir_start_lookup state kept for non dx-directories.
> Cheers,
> Stephen
--
Henrik Grubbstr?m [email protected]
Roxen Internet Software AB
Hi,
On Mon, 2005-05-09 at 15:24, Bodo Eggert wrote:
> Andreas Dilger <[email protected]> wrote:
>
> > + if (namelen > 2 || name[0] != '.'||(name[1] != '.' && name[1] != '\0')){
>
> The patch was supposed to affect only '..'.
"." is an equivalent special case in htree. It seems reasonable to
include it here for completeness.
--Stephen
On Mon, 9 May 2005, Stephen C. Tweedie wrote:
> On Mon, 2005-05-09 at 15:24, Bodo Eggert wrote:
> > Andreas Dilger <[email protected]> wrote:
> >
> > > + if (namelen > 2 || name[0] != '.'||(name[1] != '.' && name[1] != '\0')){
> >
> > The patch was supposed to affect only '..'.
>
> "." is an equivalent special case in htree. It seems reasonable to
> include it here for completeness.
OK, but the comment should be adjusted.
--
Backups? We doan NEED no steenking baX%^~,VbKx NO CARRIER