Fix readlink(2)

This patch addresses three issues related to symlinks.

1) Revert the zfs_follow_link() function to a modified version
of the original zfs_readlink().  The only changes from the
original OpenSolaris version relate to using Linux types.
For the moment this means no vnode's and no zfsvfs_t.  The
caller zpl_follow_link() was also updated accordingly.  This
change was reverted because it was slightly gratuitious.

2) Update zpl_follow_link() to use local variables for the
link buffer.  I'd forgotten that iov.iov_base is updated by
uiomove() so after the call to zfs_readlink() it can not longer
be used.  We need our own private copy of the link pointer.

3) Allocate MAXPATHLEN instead of MAXPATHLEN+1.  By default
MAXPATHLEN is 4096 bytes which is a full page, adding one to
it pushes it slightly over a page.  That means you'll likely
end up allocating 2 pages which is wasteful of memory and
possibly slightly slower.

diff --git a/module/zfs/zfs_vnops.c b/module/zfs/zfs_vnops.c
index 3633079..8bff0ef 100644 (file)
--- a/module/zfs/zfs_vnops.c
+++ b/module/zfs/zfs_vnops.c
@@ -3183,8 +3183,9 @@ EXPORT_SYMBOL(zfs_symlink);
  * Return, in the buffer contained in the provided uio structure,
  * the symbolic path referred to by ip.
  *
- *     IN:     dentry  - dentry of symbolic link.
- *             nd      - namedata for symlink
+ *     IN:     ip      - inode of symbolic link
+ *             uio     - structure to contain the link path.
+ *             cr      - credentials of caller.
  *
  *     RETURN: 0 if success
  *             error code if failure
@@ -3194,47 +3195,29 @@ EXPORT_SYMBOL(zfs_symlink);
  */
 /* ARGSUSED */
 int
-zfs_follow_link(struct dentry *dentry, struct nameidata *nd)
+zfs_readlink(struct inode *ip, uio_t *uio, cred_t *cr)
 {
-       struct inode    *ip = dentry->d_inode;
        znode_t         *zp = ITOZ(ip);
        zfs_sb_t        *zsb = ITOZSB(ip);
-       struct iovec    iov;
-       uio_t           uio;
        int             error;
 
        ZFS_ENTER(zsb);
        ZFS_VERIFY_ZP(zp);
 
-       iov.iov_len = MAXPATHLEN + 1;
-       iov.iov_base = kmem_zalloc(iov.iov_len, KM_SLEEP);
-
-       uio.uio_iov = &iov;
-       uio.uio_iovcnt = 1;
-       uio.uio_resid = iov.iov_len;
-       uio.uio_segflg = UIO_SYSSPACE;
-
        mutex_enter(&zp->z_lock);
        if (zp->z_is_sa)
-               error = sa_lookup_uio(zp->z_sa_hdl, SA_ZPL_SYMLINK(zsb), &uio);
+               error = sa_lookup_uio(zp->z_sa_hdl,
+                   SA_ZPL_SYMLINK(zsb), uio);
        else
-               error = zfs_sa_readlink(zp, &uio);
+               error = zfs_sa_readlink(zp, uio);
        mutex_exit(&zp->z_lock);
 
        ZFS_ACCESSTIME_STAMP(zsb, zp);
        zfs_inode_update(zp);
-
-       if (error) {
-               kmem_free(iov.iov_base, iov.iov_len);
-               nd_set_link(nd, ERR_PTR(error));
-       } else {
-               nd_set_link(nd, iov.iov_base);
-       }
-
        ZFS_EXIT(zsb);
        return (error);
 }
-EXPORT_SYMBOL(zfs_follow_link);
+EXPORT_SYMBOL(zfs_readlink);
 
 /*
  * Insert a new entry into directory tdip referencing sip.

diff --git a/module/zfs/zpl_inode.c b/module/zfs/zpl_inode.c
index 637a7dc..75d299b 100644 (file)
--- a/module/zfs/zpl_inode.c
+++ b/module/zfs/zpl_inode.c
@@ -245,8 +245,33 @@ out:
 static void *
 zpl_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
-       (void) zfs_follow_link(dentry, nd);
-       return NULL;
+       struct inode *ip = dentry->d_inode;
+       struct iovec iov;
+       uio_t uio;
+       char *link;
+       cred_t *cr;
+       int error;
+
+       cr = (cred_t *)get_current_cred();
+
+       iov.iov_len = MAXPATHLEN;
+       iov.iov_base = link = kmem_zalloc(MAXPATHLEN, KM_SLEEP);
+
+       uio.uio_iov = &iov;
+       uio.uio_iovcnt = 1;
+       uio.uio_resid = (MAXPATHLEN - 1);
+       uio.uio_segflg = UIO_SYSSPACE;
+
+       error = zfs_readlink(ip, &uio, cr);
+       if (error) {
+               kmem_free(link, MAXPATHLEN);
+               nd_set_link(nd, ERR_PTR(error));
+       } else {
+               nd_set_link(nd, link);
+       }
+
+       put_cred(cr);
+       return (NULL);
 }
 
 static void
@@ -256,7 +281,7 @@ zpl_put_link(struct dentry *dentry, struct nameidata *nd, void *ptr)
 
        link = nd_get_link(nd);
        if (!IS_ERR(link))
-               kmem_free(link, MAXPATHLEN + 1);
+               kmem_free(link, MAXPATHLEN);
 }
 
 static int