MFp4:

 - LK_RETRY prohibits vget() and vn_lock() to return error.
   Remove associated code. [1]
 - Properly use vhold() and vdrop() instead of their unlocked
   versions, we are guaranteed to have the vnode's interlock
   unheld. [1]
 - Fix a pseudo-infinite loop caused by 64/32-bit arithmetic
   with the same way used in modern NetBSD versions. [2]
 - Reorganize tmpfs_readdir to reduce duplicated code.

Submitted by:	kib [1]
Obtained from:	NetBSD [2]
Approved by:	re (tmpfs blanket)

3 files changed, 96 insertions, 60 deletions

diff --git a/sys/fs/tmpfs/tmpfs.h b/sys/fs/tmpfs/tmpfs.h
index c57eb25..b623514 100644
--- a/sys/fs/tmpfs/tmpfs.h
+++ b/sys/fs/tmpfs/tmpfs.h
@@ -1,7 +1,7 @@
-/*	$NetBSD: tmpfs.h,v 1.18 2006/03/31 20:27:49 riz Exp $	*/
+/*	$NetBSD: tmpfs.h,v 1.26 2007/02/22 06:37:00 thorpej Exp $	*/
 
 /*
- * Copyright (c) 2005 The NetBSD Foundation, Inc.
+ * Copyright (c) 2005, 2006 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -97,10 +97,73 @@ struct tmpfs_dirent {
  * importantly, to remove redundancy. */
 TAILQ_HEAD(tmpfs_dir, tmpfs_dirent);
 
-#define	TMPFS_DIRCOOKIE(dirent)	((off_t)(uintptr_t)(dirent))
+/* Each entry in a directory has a cookie that identifies it.  Cookies
+ * supersede offsets within directories because, given how tmpfs stores
+ * directories in memory, there is no such thing as an offset.  (Emulating
+ * a real offset could be very difficult.)
+ * 
+ * The '.', '..' and the end of directory markers have fixed cookies which
+ * cannot collide with the cookies generated by other entries.  The cookies
+ * fot the other entries are generated based on the memory address on which
+ * stores their information is stored.
+ *
+ * Ideally, using the entry's memory pointer as the cookie would be enough
+ * to represent it and it wouldn't cause collisions in any system.
+ * Unfortunately, this results in "offsets" with very large values which
+ * later raise problems in the Linux compatibility layer (and maybe in other
+ * places) as described in PR kern/32034.  Hence we need to workaround this
+ * with a rather ugly hack.
+ *
+ * Linux 32-bit binaries, unless built with _FILE_OFFSET_BITS=64, have off_t
+ * set to 'long', which is a 32-bit *signed* long integer.  Regardless of
+ * the macro value, GLIBC (2.3 at least) always uses the getdents64
+ * system call (when calling readdir) which internally returns off64_t
+ * offsets.  In order to make 32-bit binaries work, *GLIBC* converts the
+ * 64-bit values returned by the kernel to 32-bit ones and aborts with
+ * EOVERFLOW if the conversion results in values that won't fit in 32-bit
+ * integers (which it assumes is because the directory is extremely large).
+ * This wouldn't cause problems if we were dealing with unsigned integers,
+ * but as we have signed integers, this check fails due to sign expansion.
+ *
+ * For example, consider that the kernel returns the 0xc1234567 cookie to
+ * userspace in a off64_t integer.  Later on, GLIBC casts this value to
+ * off_t (remember, signed) with code similar to:
+ *     system call returns the offset in kernel_value;
+ *     off_t casted_value = kernel_value;
+ *     if (sizeof(off_t) != sizeof(off64_t) &&
+ *         kernel_value != casted_value)
+ *             error!
+ * In this case, casted_value still has 0xc1234567, but when it is compared
+ * for equality against kernel_value, it is promoted to a 64-bit integer and
+ * becomes 0xffffffffc1234567, which is different than 0x00000000c1234567.
+ * Then, GLIBC assumes this is because the directory is very large.
+ *
+ * Given that all the above happens in user-space, we have no control over
+ * it; therefore we must workaround the issue here.  We do this by
+ * truncating the pointer value to a 32-bit integer and hope that there
+ * won't be collisions.  In fact, this will not cause any problems in
+ * 32-bit platforms but some might arise in 64-bit machines (I'm not sure
+ * if they can happen at all in practice).
+ *
+ * XXX A nicer solution shall be attempted. */
+#ifdef _KERNEL
 #define	TMPFS_DIRCOOKIE_DOT	0
 #define	TMPFS_DIRCOOKIE_DOTDOT	1
 #define	TMPFS_DIRCOOKIE_EOF	2
+static __inline
+off_t
+tmpfs_dircookie(struct tmpfs_dirent *de)
+{
+	off_t cookie;
+
+	cookie = ((off_t)(uintptr_t)de >> 1) & 0x7FFFFFFF;
+	MPASS(cookie != TMPFS_DIRCOOKIE_DOT);
+	MPASS(cookie != TMPFS_DIRCOOKIE_DOTDOT);
+	MPASS(cookie != TMPFS_DIRCOOKIE_EOF);
+
+	return cookie;
+}
+#endif
 
 /* --------------------------------------------------------------------- */
 
@@ -408,7 +471,7 @@ int	tmpfs_truncate(struct vnode *, off_t);
     MPASS((node)->tn_type == VDIR); \
     MPASS((node)->tn_size % sizeof(struct tmpfs_dirent) == 0); \
     MPASS((node)->tn_dir.tn_readdir_lastp == NULL || \
-	TMPFS_DIRCOOKIE((node)->tn_dir.tn_readdir_lastp) == (node)->tn_dir.tn_readdir_lastn);
+	tmpfs_dircookie((node)->tn_dir.tn_readdir_lastp) == (node)->tn_dir.tn_readdir_lastn);
 
 /* --------------------------------------------------------------------- */
 
diff --git a/sys/fs/tmpfs/tmpfs_subr.c b/sys/fs/tmpfs/tmpfs_subr.c
index 08127fd..abfab9a 100644
--- a/sys/fs/tmpfs/tmpfs_subr.c
+++ b/sys/fs/tmpfs/tmpfs_subr.c
@@ -1,4 +1,4 @@
-/*	$NetBSD: tmpfs_subr.c,v 1.21 2006/06/07 22:33:39 kardel Exp $	*/
+/*	$NetBSD: tmpfs_subr.c,v 1.35 2007/07/09 21:10:50 ad Exp $	*/
 
 /*
  * Copyright (c) 2005 The NetBSD Foundation, Inc.
@@ -311,7 +311,7 @@ int
 tmpfs_alloc_vp(struct mount *mp, struct tmpfs_node *node, int lkflag,
     struct vnode **vpp, struct thread *td)
 {
-	int error;
+	int error = 0;
 	struct vnode *vp;
 
 loop:
@@ -320,10 +320,8 @@ loop:
 		VI_LOCK(vp);
 		TMPFS_NODE_UNLOCK(node);
 		vholdl(vp);
-		error = vget(vp, lkflag | LK_INTERLOCK | LK_RETRY, td);
+		(void) vget(vp, lkflag | LK_INTERLOCK | LK_RETRY, td);
 		vdrop(vp);
-		if (error)
-			return error;
 
 		/*
 		 * Make sure the vnode is still there after
@@ -361,13 +359,7 @@ loop:
 		goto unlock;
 	MPASS(vp != NULL);
 
-	error = vn_lock(vp, lkflag | LK_RETRY, td);
-	if (error != 0) {
-		vp->v_data = NULL;
-		vput(vp);
-		vp = NULL;
-		goto unlock;
-	}
+	(void) vn_lock(vp, lkflag | LK_RETRY, td);
 
 	vp->v_data = node;
 	vp->v_type = node->tn_type;
@@ -681,7 +673,7 @@ tmpfs_dir_getdotdotdent(struct tmpfs_node *node, struct uio *uio)
 			if (de == NULL)
 				uio->uio_offset = TMPFS_DIRCOOKIE_EOF;
 			else
-				uio->uio_offset = TMPFS_DIRCOOKIE(de);
+				uio->uio_offset = tmpfs_dircookie(de);
 		}
 	}
 
@@ -706,7 +698,7 @@ tmpfs_dir_lookupbycookie(struct tmpfs_node *node, off_t cookie)
 	}
 
 	TAILQ_FOREACH(de, &node->tn_dir.tn_dirhead, td_entries) {
-		if (TMPFS_DIRCOOKIE(de) == cookie) {
+		if (tmpfs_dircookie(de) == cookie) {
 			break;
 		}
 	}
@@ -814,7 +806,7 @@ tmpfs_dir_getdents(struct tmpfs_node *node, struct uio *uio, off_t *cntp)
 		node->tn_dir.tn_readdir_lastn = 0;
 		node->tn_dir.tn_readdir_lastp = NULL;
 	} else {
-		node->tn_dir.tn_readdir_lastn = uio->uio_offset = TMPFS_DIRCOOKIE(de);
+		node->tn_dir.tn_readdir_lastn = uio->uio_offset = tmpfs_dircookie(de);
 		node->tn_dir.tn_readdir_lastp = de;
 	}
 
diff --git a/sys/fs/tmpfs/tmpfs_vnops.c b/sys/fs/tmpfs/tmpfs_vnops.c
index d43e267..1d38ed9 100644
--- a/sys/fs/tmpfs/tmpfs_vnops.c
+++ b/sys/fs/tmpfs/tmpfs_vnops.c
@@ -1,7 +1,7 @@
-/*	$NetBSD: tmpfs_vnops.c,v 1.35 2007/01/04 15:42:37 elad Exp $	*/
+/*	$NetBSD: tmpfs_vnops.c,v 1.39 2007/07/23 15:41:01 jmmv Exp $	*/
 
 /*
- * Copyright (c) 2005 The NetBSD Foundation, Inc.
+ * Copyright (c) 2005, 2006 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -98,17 +98,14 @@ tmpfs_lookup(struct vop_cachedlookup_args *v)
 		int ltype = 0;
 
 		ltype = VOP_ISLOCKED(dvp, td);
-		vholdl(dvp);
+		vhold(dvp);
 		VOP_UNLOCK(dvp, 0, td);
 		/* Allocate a new vnode on the matching entry. */
 		error = tmpfs_alloc_vp(dvp->v_mount, dnode->tn_dir.tn_parent,
 		    cnp->cn_lkflags, vpp, td);
 
 		vn_lock(dvp, ltype | LK_RETRY, td);
-		if (ltype & LK_INTERLOCK)
-			vdropl(dvp);
-		else
-			vdrop(dvp);
+		vdrop(dvp);
 
 		dnode->tn_dir.tn_parent->tn_lookup_dirent = NULL;
 	} else if (cnp->cn_namelen == 1 && cnp->cn_nameptr[0] == '.') {
@@ -1211,49 +1208,35 @@ tmpfs_readdir(struct vop_readdir_args *v)
 
 	int error;
 	off_t startoff;
-	off_t cnt;
+	off_t cnt = 0;
 	struct tmpfs_node *node;
 
 	/* This operation only makes sense on directory nodes. */
-	if (vp->v_type != VDIR) {
-		error = ENOTDIR;
-		goto out;
-	}
+	if (vp->v_type != VDIR)
+		return ENOTDIR;
 
 	node = VP_TO_TMPFS_DIR(vp);
 
 	startoff = uio->uio_offset;
 
-	cnt = 0;
-	if (uio->uio_offset == TMPFS_DIRCOOKIE_DOT) {
+	switch (startoff) {
+	case TMPFS_DIRCOOKIE_DOT:
 		error = tmpfs_dir_getdotdent(node, uio);
-		if (error == -1) {
-			error = 0;
-			goto outok;
-		} else if (error != 0)
-			goto outok;
-		cnt++;
-	}
-
-	if (uio->uio_offset == TMPFS_DIRCOOKIE_DOTDOT) {
+		if (error == 0)
+			cnt++;
+		break;
+	case TMPFS_DIRCOOKIE_DOTDOT:
 		error = tmpfs_dir_getdotdotdent(node, uio);
-		if (error == -1) {
-			error = 0;
-			goto outok;
-		} else if (error != 0)
-			goto outok;
-		cnt++;
+		if (error == 0)
+			cnt++;
+		break;
+	default:
+		error = tmpfs_dir_getdents(node, uio, &cnt);
+		MPASS(error >= -1);
 	}
 
-	error = tmpfs_dir_getdents(node, uio, &cnt);
 	if (error == -1)
 		error = 0;
-	MPASS(error >= 0);
-
-outok:
-	/* This label assumes that startoff has been
-	 * initialized.  If the compiler didn't spit out warnings, we'd
-	 * simply make this one be 'out' and drop 'outok'. */
 
 	if (eofflag != NULL)
 		*eofflag =
@@ -1283,11 +1266,10 @@ outok:
 					MPASS(de != NULL);
 					de = TAILQ_NEXT(de, td_entries);
 				}
-				if (de == NULL) {
+				if (de == NULL)
 					off = TMPFS_DIRCOOKIE_EOF;
-				} else {
-					off = TMPFS_DIRCOOKIE(de);
-				}
+				else
+					off = tmpfs_dircookie(de);
 			}
 
 			(*cookies)[i] = off;
@@ -1295,7 +1277,6 @@ outok:
 		MPASS(uio->uio_offset == off);
 	}
 
-out:
 	return error;
 }