From: Brian Behlendorf <behlendorf1@llnl.gov>
Date: Wed, 16 Feb 2011 23:54:55 +0000 (-0800)
Subject: Fix readlink(2)
X-Git-Tag: zfs-0.6.0-rc1~1^2~4
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8b4f9a2d55fc5ee28f69b29f2fece7d8e2cb5c7a;p=zfs

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 363307932..8bff0ef4b 100644
--- 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 637a7dc69..75d299b47 100644
--- 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