[OpenAFS-devel] [PATCH] fix symlink crashes on RHEL4 kernels
Christopher Allen Wing
wingc@engin.umich.edu
Thu, 22 Mar 2007 15:17:56 -0400 (EDT)
I looked into this more closely and it's not as bad as I originally
thought. The situation is:
linux-2.4 - 2.6.8
Kernel provides page_readlink(), page_follow_link() API; is safe
for OpenAFS to use.
linux-2.6.9
Kernel provides page_readlink(), page_follow_link() API, and also
generic_readlink() and page_follow_link_light() API. The former
is safe for OpenAFS to use, the latter (page_follow_link_light())
will crash.
linux-2.6.10 - 2.6.12
Kernel only provides page_follow_link_light() API,
page_follow_link() is removed. It is unsafe for OpenAFS to use
symlink text caching on these kernels.
linux-2.6.13+
Kernel developers realize their mistake and fix
page_follow_link_light() API to no longer crash. It is safe for
OpenAFS to use symlink text caching on 2.6.13 and newer kernels.
Only linux-2.6.10 - 2.6.12 are broken. Unfortunately, the RHEL4 kernel
contains the changes from 2.6.10 and thus OpenAFS will crash on all RHEL4
kernels.
Attached is my proposed patch to fix the problem. It disables symlink
text caching altogether if a usable API is not found in the kernel.
Autoconf tests are used to check the API, not the kernel release number.
The result will be that people running RHEL4 kernels will get slightly
less efficient path name lookups when symlinks are involved, but that's
better than crashes.
I tested this on a 2.4 kernel (RHEL3), as well as a 2.6 kernel with the
broken API (RHEL4), and a newer 2.6 kernel which does not have the broken
API (RHEL5). The patch should make no changes whatsoever to the source
code unless you are building on a broken 2.6 kernel.
The patch is against openafs-stable-1_4_x but it should also apply fine to
HEAD, as there do not seem to be any conflicting changes. (the only
changes I noticed in src/afs/LINUX are some code rearrangement and the
addition of the NFS exporter)
You can also download this patch from:
http://www-personal.engin.umich.edu/~wingc/openafs/patches/openafs-stable-1_4_x-20070320-symlink26.patch
-Chris Wing
wingc@engin.umich.edu
diff -uNr openafs.orig/acinclude.m4 openafs/acinclude.m4
--- openafs.orig/acinclude.m4 2007-02-22 16:48:58.000000000 -0500
+++ openafs/acinclude.m4 2007-03-22 11:24:15.000000000 -0400
@@ -606,6 +606,7 @@
LINUX_IOP_I_CREATE_TAKES_NAMEIDATA
LINUX_IOP_I_LOOKUP_TAKES_NAMEIDATA
LINUX_IOP_I_PERMISSION_TAKES_NAMEIDATA
+ LINUX_IOP_I_PUT_LINK_TAKES_COOKIE
LINUX_DOP_D_REVALIDATE_TAKES_NAMEIDATA
LINUX_AOP_WRITEBACK_CONTROL
LINUX_FS_STRUCT_FOP_HAS_FLOCK
@@ -848,6 +849,11 @@
if test "x$ac_cv_linux_exports_tasklist_lock" = "xyes" ; then
AC_DEFINE(EXPORTED_TASKLIST_LOCK, 1, [define if tasklist_lock exported])
fi
+ if test "x$ac_cv_linux_kernel_page_follow_link" = "xyes" -o "x$ac_cv_linux_func_i_put_link_takes_cookie" = "xyes"; then
+ AC_DEFINE(USABLE_KERNEL_PAGE_SYMLINK_CACHE, 1, [define if your kernel has a usable symlink cache API])
+ else
+ AC_MSG_WARN([your kernel does not have a usable symlink cache API])
+ fi
:
fi
esac
diff -uNr openafs.orig/src/afs/LINUX/osi_vnodeops.c openafs/src/afs/LINUX/osi_vnodeops.c
--- openafs.orig/src/afs/LINUX/osi_vnodeops.c 2007-02-20 13:06:24.000000000 -0500
+++ openafs/src/afs/LINUX/osi_vnodeops.c 2007-03-22 11:39:13.000000000 -0400
@@ -1279,7 +1279,7 @@
return -code;
}
-#if !defined(AFS_LINUX24_ENV)
+#if !defined(USABLE_KERNEL_PAGE_SYMLINK_CACHE)
/* afs_linux_readlink
* Fill target (which is in user space) with contents of symlink.
*/
@@ -1299,6 +1299,39 @@
/* afs_linux_follow_link
* a file system dependent link following routine.
*/
+#if defined(AFS_LINUX24_ENV)
+static int afs_linux_follow_link(struct dentry *dentry, struct nameidata *nd)
+{
+ int code;
+ char *name;
+
+ AFS_GLOCK();
+ name = osi_Alloc(PATH_MAX + 1);
+ if (!name) {
+ AFS_GUNLOCK();
+ return -EIO;
+ }
+
+ code = afs_linux_ireadlink(dentry->d_inode, name, PATH_MAX, AFS_UIOSYS);
+ AFS_GUNLOCK();
+
+ if (code < 0) {
+ goto out;
+ }
+
+ name[code] = '\0';
+ code = vfs_follow_link(nd, name);
+
+out:
+ AFS_GLOCK();
+ osi_Free(name, PATH_MAX + 1);
+ AFS_GUNLOCK();
+
+ return code;
+}
+
+#else /* !defined(AFS_LINUX24_ENV) */
+
static struct dentry *
afs_linux_follow_link(struct dentry *dp, struct dentry *basep,
unsigned int follow)
@@ -1332,7 +1365,8 @@
AFS_GUNLOCK();
return res;
}
-#endif
+#endif /* AFS_LINUX24_ENV */
+#endif /* USABLE_KERNEL_PAGE_SYMLINK_CACHE */
/* afs_linux_readpage
* all reads come through here. A strategy-like read call.
@@ -1697,7 +1731,7 @@
/* We really need a separate symlink set of ops, since do_follow_link()
* determines if it _is_ a link by checking if the follow_link op is set.
*/
-#if defined(AFS_LINUX24_ENV)
+#if defined(USABLE_KERNEL_PAGE_SYMLINK_CACHE)
static int
afs_symlink_filler(struct file *file, struct page *page)
{
@@ -1732,10 +1766,10 @@
static struct address_space_operations afs_symlink_aops = {
.readpage = afs_symlink_filler
};
-#endif
+#endif /* USABLE_KERNEL_PAGE_SYMLINK_CACHE */
static struct inode_operations afs_symlink_iops = {
-#if defined(AFS_LINUX24_ENV)
+#if defined(USABLE_KERNEL_PAGE_SYMLINK_CACHE)
.readlink = page_readlink,
#if defined(HAVE_KERNEL_PAGE_FOLLOW_LINK)
.follow_link = page_follow_link,
@@ -1743,13 +1777,17 @@
.follow_link = page_follow_link_light,
.put_link = page_put_link,
#endif
- .setattr = afs_notify_change,
-#else
+#else /* !defined(USABLE_KERNEL_PAGE_SYMLINK_CACHE) */
.readlink = afs_linux_readlink,
.follow_link = afs_linux_follow_link,
+#if !defined(AFS_LINUX24_ENV)
.permission = afs_linux_permission,
.revalidate = afs_linux_revalidate,
#endif
+#endif /* USABLE_KERNEL_PAGE_SYMLINK_CACHE */
+#if defined(AFS_LINUX24_ENV)
+ .setattr = afs_notify_change,
+#endif
};
void
@@ -1775,7 +1813,7 @@
} else if (S_ISLNK(ip->i_mode)) {
ip->i_op = &afs_symlink_iops;
-#if defined(AFS_LINUX24_ENV)
+#if defined(USABLE_KERNEL_PAGE_SYMLINK_CACHE)
ip->i_data.a_ops = &afs_symlink_aops;
ip->i_mapping = &ip->i_data;
#endif
diff -uNr openafs.orig/src/cf/linux-test4.m4 openafs/src/cf/linux-test4.m4
--- openafs.orig/src/cf/linux-test4.m4 2007-02-26 12:53:33.000000000 -0500
+++ openafs/src/cf/linux-test4.m4 2007-03-22 10:48:52.000000000 -0400
@@ -644,6 +644,22 @@
AC_MSG_RESULT($ac_cv_linux_func_i_permission_takes_nameidata)])
+AC_DEFUN([LINUX_IOP_I_PUT_LINK_TAKES_COOKIE], [
+ AC_MSG_CHECKING([whether inode_operations.put_link takes an opaque cookie])
+ AC_CACHE_VAL([ac_cv_linux_func_i_put_link_takes_cookie], [
+ AC_TRY_KBUILD(
+[#include <linux/fs.h>
+#include <linux/namei.h>],
+[struct inode _inode;
+struct dentry _dentry;
+struct nameidata _nameidata;
+void *cookie;
+(void)_inode.i_op->put_link(&_dentry, &_nameidata, cookie);],
+ ac_cv_linux_func_i_put_link_takes_cookie=yes,
+ ac_cv_linux_func_i_put_link_takes_cookie=no)])
+ AC_MSG_RESULT($ac_cv_linux_func_i_put_link_takes_cookie)])
+
+
AC_DEFUN([LINUX_DOP_D_REVALIDATE_TAKES_NAMEIDATA], [
AC_MSG_CHECKING([whether dentry_operations.d_revalidate takes a nameidata])
AC_CACHE_VAL([ac_cv_linux_func_d_revalidate_takes_nameidata], [