From de381aa0d39e88a1ca0c8ccbb2471c5cad5a964c Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Fri, 6 Jul 2012 16:37:39 -0500 Subject: [PATCH] Linux: Make dir dentry aliases act like symlinks Currently, we try to invalidate other dentries that exist for a particular dir inode when we look up a dentry. This is so we try to avoid duplicate dentries for a directory, which Linux does not like (you cannot have hardlinks to a dir). If we cannot invalidate the other aliases (because they are being used), right now we just return the alias. This can make it very easy to panic the client, due to the sanity checks Linux performs when dong things like 'rmdir'. If we do something like this: mkdir dir1 fs mkm dir1/mtpt vol mkdir dir1/mtpt/dir2 fs mkm dir1/mtpt/dir2/mtpt2 vol cd dir1/mtpt rmdir dir2/mtpt2 For the 'rmdir', we will lookup 'mtpt2'. Since 'mtpt' and 'mtpt2' are mountpoints for the same volume, their dentries point to the same directory inode. So when we lookup 'mtpt2', we will try to invalidate the other dentry, but we cannot do that since it is the cwd. So we return the alias dentry (for 'mtpt'). The Linux VFS layer then does a sanity check for the rmdir operation, checking that the child dentry's parent inode is the same as the inode we're performing the rmdir for. Since the dentry we returned was for 'mtpt', whose parent is 'dir1', and the actual dir we're performing the rmdir for is 'dir2', this sanity check fails and we BUG. To avoid this, make the dentry alias act like a symlink when we encounter an uninvalidateable dentry alias. That is, we allow multiple dentry aliases for a directory, however, when the dentry aliases are actually used, we redirect to a common dentry (via d_automount where possible, and follow_link elsewhere). This means that such mountpoints will behave similarly to symlinks, in that we 'point' to a specific mountpoint dentry. This means that if we have multiple different ways to get to the same volume, and all are accessed at the same time, all but one of those mountpoints will behave like symlinks, pointing to the same mountpoint. So, the '..' entries for each path will all point to the parent dir of one mountpoint, meaning that the '..' entry will be "wrong", but for most cases it will still be correct. In order to try to make the 'target', pointed-to directory consistent, we add a new field to struct vcache: target_link. This points to the dentry we should redirect to, whenever that vcache is referenced. To avoid (possibly not-feasibly-solvable) problems with refcounting, this pointer is not actually a reference to the target dentry, but just serves as a pointer to compare to. FIXES 130273 Change-Id: I990131ce95cefe8336e83c7ebfb48aed1d685109 Reviewed-on: http://gerrit.openafs.org/7741 Tested-by: BuildBot Tested-by: Andrew Deason Reviewed-by: Derrick Brashear --- acinclude.m4 | 1 + src/afs/LINUX/osi_vnodeops.c | 143 ++++++++++++++++++++++++++++++++++++++--- src/afs/LINUX24/osi_vnodeops.c | 95 ++++++++++++++++++++++++--- src/afs/afs.h | 7 ++ 4 files changed, 227 insertions(+), 19 deletions(-) diff --git a/acinclude.m4 b/acinclude.m4 index ff4ea92..e67d220 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -805,6 +805,7 @@ case $AFS_SYSNAME in *_linux* | *_umlinux*) AC_CHECK_LINUX_STRUCT([backing_dev_info], [name], [backing-dev.h]) AC_CHECK_LINUX_STRUCT([ctl_table], [ctl_name], [sysctl.h]) + AC_CHECK_LINUX_STRUCT([dentry_operations], [d_automount], [dcache.h]) AC_CHECK_LINUX_STRUCT([inode], [i_alloc_sem], [fs.h]) AC_CHECK_LINUX_STRUCT([inode], [i_blkbits], [fs.h]) AC_CHECK_LINUX_STRUCT([inode], [i_blksize], [fs.h]) diff --git a/src/afs/LINUX/osi_vnodeops.c b/src/afs/LINUX/osi_vnodeops.c index 9caaf5a..f8dc925 100644 --- a/src/afs/LINUX/osi_vnodeops.c +++ b/src/afs/LINUX/osi_vnodeops.c @@ -774,6 +774,61 @@ struct file_operations afs_file_fops = { .llseek = default_llseek, }; +static struct dentry * +canonical_dentry(struct inode *ip) +{ + struct vcache *vcp = VTOAFS(ip); + struct dentry *first = NULL, *ret = NULL, *cur; + + /* general strategy: + * if vcp->target_link is set, and can be found in ip->i_dentry, use that. + * otherwise, use the first dentry in ip->i_dentry. + * if ip->i_dentry is empty, use the 'dentry' argument we were given. + */ + /* note that vcp->target_link specifies which dentry to use, but we have + * no reference held on that dentry. so, we cannot use or dereference + * vcp->target_link itself, since it may have been freed. instead, we only + * use it to compare to pointers in the ip->i_dentry list. */ + + d_prune_aliases(ip); + +# ifdef HAVE_DCACHE_LOCK + spin_lock(&dcache_lock); +# else + spin_lock(&ip->i_lock); +# endif + + list_for_each_entry_reverse(cur, &ip->i_dentry, d_alias) { + + if (!vcp->target_link || cur == vcp->target_link) { + ret = cur; + break; + } + + if (!first) { + first = cur; + } + } + if (!ret && first) { + ret = first; + } + + vcp->target_link = ret; + +# ifdef HAVE_DCACHE_LOCK + if (ret) { + dget_locked(ret); + } + spin_unlock(&dcache_lock); +# else + if (ret) { + dget(ret); + } + spin_unlock(&ip->i_lock); +# endif + + return ret; +} /********************************************************************** * AFS Linux dentry operations @@ -1168,10 +1223,40 @@ afs_dentry_delete(struct dentry *dp) return 0; } +#ifdef STRUCT_DENTRY_OPERATIONS_HAS_D_AUTOMOUNT +static struct vfsmount * +afs_dentry_automount(struct path *path) +{ + struct dentry *target; + + target = canonical_dentry(path->dentry->d_inode); + + if (target == path->dentry) { + dput(target); + target = NULL; + } + + if (target) { + dput(path->dentry); + path->dentry = target; + + } else { + spin_lock(&path->dentry->d_lock); + path->dentry->d_flags &= ~DCACHE_NEED_AUTOMOUNT; + spin_unlock(&path->dentry->d_lock); + } + + return NULL; +} +#endif /* STRUCT_DENTRY_OPERATIONS_HAS_D_AUTOMOUNT */ + struct dentry_operations afs_dentry_operations = { .d_revalidate = afs_linux_dentry_revalidate, .d_delete = afs_dentry_delete, .d_iput = afs_dentry_iput, +#ifdef STRUCT_DENTRY_OPERATIONS_HAS_D_AUTOMOUNT + .d_automount = afs_dentry_automount, +#endif /* STRUCT_DENTRY_OPERATIONS_HAS_D_AUTOMOUNT */ }; /********************************************************************** @@ -1266,20 +1351,28 @@ afs_linux_lookup(struct inode *dip, struct dentry *dp) AFS_GUNLOCK(); if (ip && S_ISDIR(ip->i_mode)) { + int retry = 1; struct dentry *alias; - /* Try to invalidate an existing alias in favor of our new one */ - alias = d_find_alias(ip); - /* But not if it's disconnected; then we want d_splice_alias below */ - if (alias && !(alias->d_flags & DCACHE_DISCONNECTED)) { - if (d_invalidate(alias) == 0) { - dput(alias); - } else { - iput(ip); - crfree(credp); - return alias; + while (retry) { + retry = 0; + + /* Try to invalidate an existing alias in favor of our new one */ + alias = d_find_alias(ip); + /* But not if it's disconnected; then we want d_splice_alias below */ + if (alias && !(alias->d_flags & DCACHE_DISCONNECTED)) { + if (d_invalidate(alias) == 0) { + /* there may be more aliases; try again until we run out */ + retry = 1; + } } + + dput(alias); } + +#ifdef STRUCT_DENTRY_OPERATIONS_HAS_D_AUTOMOUNT + ip->i_flags |= S_AUTOMOUNT; +#endif } newdp = d_splice_alias(ip, dp); @@ -2576,6 +2669,33 @@ afs_linux_write_begin(struct file *file, struct address_space *mapping, } #endif +#ifndef STRUCT_DENTRY_OPERATIONS_HAS_D_AUTOMOUNT +static void * +afs_linux_dir_follow_link(struct dentry *dentry, struct nameidata *nd) +{ + struct dentry **dpp; + struct dentry *target; + + target = canonical_dentry(dentry->d_inode); + +# ifdef STRUCT_NAMEIDATA_HAS_PATH + dpp = &nd->path.dentry; +# else + dpp = &nd->dentry; +# endif + + dput(*dpp); + + if (target) { + *dpp = target; + } else { + *dpp = dget(dentry); + } + + return NULL; +} +#endif /* !STRUCT_DENTRY_OPERATIONS_HAS_D_AUTOMOUNT */ + static struct inode_operations afs_file_iops = { .permission = afs_linux_permission, @@ -2613,6 +2733,9 @@ static struct inode_operations afs_dir_iops = { .rename = afs_linux_rename, .getattr = afs_linux_getattr, .permission = afs_linux_permission, +#ifndef STRUCT_DENTRY_OPERATIONS_HAS_D_AUTOMOUNT + .follow_link = afs_linux_dir_follow_link, +#endif }; /* We really need a separate symlink set of ops, since do_follow_link() diff --git a/src/afs/LINUX24/osi_vnodeops.c b/src/afs/LINUX24/osi_vnodeops.c index 5d0d589..679293f 100644 --- a/src/afs/LINUX24/osi_vnodeops.c +++ b/src/afs/LINUX24/osi_vnodeops.c @@ -783,6 +783,57 @@ struct file_operations afs_file_fops = { #endif }; +static struct dentry * +canonical_dentry(struct inode *ip) +{ + struct vcache *vcp = VTOAFS(ip); + struct dentry *first = NULL, *ret = NULL, *cur; + struct list_head *head, *prev, *tmp; + + /* general strategy: + * if vcp->target_link is set, and can be found in ip->i_dentry, use that. + * otherwise, use the first dentry in ip->i_dentry. + * if ip->i_dentry is empty, use the 'dentry' argument we were given. + */ + /* note that vcp->target_link specifies which dentry to use, but we have + * no reference held on that dentry. so, we cannot use or dereference + * vcp->target_link itself, since it may have been freed. instead, we only + * use it to compare to pointers in the ip->i_dentry list. */ + + d_prune_aliases(ip); + + spin_lock(&dcache_lock); + + head = &ip->i_dentry; + prev = ip->i_dentry.prev; + + while (prev != head) { + tmp = prev; + prev = tmp->prev; + cur = list_entry(tmp, struct dentry, d_alias); + + if (!vcp->target_link || cur == vcp->target_link) { + ret = cur; + break; + } + + if (!first) { + first = cur; + } + } + if (!ret && first) { + ret = first; + } + + vcp->target_link = ret; + + if (ret) { + dget_locked(ret); + } + spin_unlock(&dcache_lock); + + return ret; +} /********************************************************************** * AFS Linux dentry operations @@ -1241,18 +1292,23 @@ afs_linux_lookup(struct inode *dip, struct dentry *dp) #if defined(AFS_LINUX24_ENV) if (ip && S_ISDIR(ip->i_mode)) { + int retry = 1; struct dentry *alias; - /* Try to invalidate an existing alias in favor of our new one */ - alias = d_find_alias(ip); - if (alias) { - if (d_invalidate(alias) == 0) { - dput(alias); - } else { - iput(ip); - crfree(credp); - return alias; + while (retry) { + retry = 0; + + /* Try to invalidate an existing alias in favor of our new one */ + alias = d_find_alias(ip); + /* But not if it's disconnected; then we want d_splice_alias below */ + if (alias) { + if (d_invalidate(alias) == 0) { + /* there may be more aliases; try again until we run out */ + retry = 1; + } } + + dput(alias); } } #endif @@ -1998,6 +2054,26 @@ afs_linux_write_begin(struct file *file, struct address_space *mapping, } #endif +static int +afs_linux_dir_follow_link(struct dentry *dentry, struct nameidata *nd) +{ + struct dentry **dpp; + struct dentry *target; + + target = canonical_dentry(dentry->d_inode); + + dpp = &nd->dentry; + + dput(*dpp); + + if (target) { + *dpp = target; + } else { + *dpp = dget(dentry); + } + + return 0; +} static struct inode_operations afs_file_iops = { #if defined(AFS_LINUX24_ENV) @@ -2047,6 +2123,7 @@ static struct inode_operations afs_dir_iops = { .rename = afs_linux_rename, .revalidate = afs_linux_revalidate, .permission = afs_linux_permission, + .follow_link = afs_linux_dir_follow_link, }; /* We really need a separate symlink set of ops, since do_follow_link() diff --git a/src/afs/afs.h b/src/afs/afs.h index 1f5996c..6a24cc0 100644 --- a/src/afs/afs.h +++ b/src/afs/afs.h @@ -928,6 +928,13 @@ struct vcache { #if defined(AFS_LINUX26_ENV) cred_t *cred; /* last writer's cred */ #endif +#ifdef AFS_LINUX24_ENV + struct dentry *target_link; /* dentry we prefer, when we are redirecting + * all requests due to duplicate dentry aliases. + * See LINUX/osi_vnodeops.c. Note that this is + * NOT an actual reference to a dentry, so this + * pointer MUST NOT be dereferenced on its own. */ +#endif afs_int32 vc_error; /* stash write error for this vnode. */ int xlatordv; /* Used by nfs xlator */ afs_ucred_t *uncred; -- 1.9.4