Our fts(3) API, as inherited from 4.4BSD, suffers from integer

fields in FTS and FTSENT structs being too narrow.  In addition,
the narrow types creep from there into fts.c.  As a result, fts(3)
consumers, e.g., find(1) or rm(1), can't handle file trees an ordinary
user can create, which can have security implications.

To fix the historic implementation of fts(3), OpenBSD and NetBSD
have already changed <fts.h> in somewhat incompatible ways, so we
are free to do so, too.  This change is a superset of changes from
the other BSDs with a few more improvements.  It doesn't touch
fts(3) functionality; it just extends integer types used by it to
match modern reality and the C standard.

Here are its points:

o For C object sizes, use size_t unless it's 100% certain that
  the object will be really small.  (Note that fts(3) can construct
  pathnames _much_ longer than PATH_MAX for its consumers.)

o Avoid the short types because on modern platforms using them
  results in larger and slower code.  Change shorts to ints as
  follows:

	- For variables than count simple, limited things like states,
	  use plain vanilla `int' as it's the type of choice in C.

	- For a limited number of bit flags use `unsigned' because signed
	  bit-wise operations are implementation-defined, i.e., unportable,
	  in C.

o For things that should be at least 64 bits wide, use long long
  and not int64_t, as the latter is an optional type.  See
  FTSENT.fts_number aka FTS.fts_bignum.  Extending fts_number `to
  satisfy future needs' is pointless because there is fts_pointer,
  which can be used to link to arbitrary data from an FTSENT.
  However, there already are fts(3) consumers that require fts_number,
  or fts_bignum, have at least 64 bits in it, so we must allow for them.

o For the tree depth, use `long'.  This is a trade-off between making
  this field too wide and allowing for 64-bit inode numbers and/or
  chain-mounted filesystems.  On the one hand, `long' is almost
  enough for 32-bit filesystems on a 32-bit platform (our ino_t is
  uint32_t now).  On the other hand, platforms with a 64-bit (or
  wider) `long' will be ready for 64-bit inode numbers, as well as
  for several 32-bit filesystems mounted one under another.  Note
  that fts_level has to be signed because -1 is a magic value for it,
  FTS_ROOTPARENTLEVEL.

o For the `nlinks' local var in fts_build(), use `long'.  The logic
  in fts_build() requires that `nlinks' be signed, but our nlink_t
  currently is uint16_t.  Therefore let's make the signed var wide
  enough to be able to represent 2^16-1 in pure C99, and even 2^32-1
  on a 64-bit platform.  Perhaps the logic should be changed just
  to use nlink_t, but it can be done later w/o breaking fts(3) ABI
  any more because `nlinks' is just a local var.

This commit also inludes supporting stuff for the fts change:

o Preserve the old versions of fts(3) functions through libc symbol
versioning because the old versions appeared in all our former releases.

o Bump __FreeBSD_version just in case.  There is a small chance that
some ill-written 3-rd party apps may fail to build or work correctly
if compiled after this change.

o Update the fts(3) manpage accordingly.  In particular, remove
references to fts_bignum, which was a FreeBSD-specific hack to work
around the too narrow types of FTSENT members.  Now fts_number is
at least 64 bits wide (long long) and fts_bignum is an undocumented
alias for fts_number kept around for compatibility reasons.  According
to Google Code Search, the only big consumers of fts_bignum are in
our own source tree, so they can be fixed easily to use fts_number.

o Mention the change in src/UPDATING.

PR:		bin/104458
Approved by:	re (quite a while ago)
Discussed with:	deischen (the symbol versioning part)
Reviewed by:	-arch (mostly silence); das (generally OK, but we didn't
		agree on some types used; assuming that no objections on
		-arch let me to stick to my opinion)

UPDATING

@@ -22,6 +22,20 @@ NOTE TO PEOPLE WHO THINK THAT FreeBSD 8.x IS SLOW:
 	to maximize performance.  (To disable malloc debugging, run
 	ln -s aj /etc/malloc.conf.)
 
+20080126:
+	The fts(3) structures have been changed to use adequate
+	integer types for their members and so to be able to cope
+	with huge file trees.  The old fts(3) ABI is preserved
+	through symbol versioning in libc, so third-party binaries
+	using fts(3) should still work, although they will not take
+	advantage of the extended types.  At the same time, some
+	third-party software might fail to build after this change
+	due to unportable assumptions made in its source code about
+	fts(3) structure members.  Such software should be fixed
+	by its vendor or, in the worst case, in the ports tree.
+	FreeBSD_version 800015 marks this change for the unlikely
+	case that a portable fix is impossible.
+
 20080123:
 	To upgrade to -current after this date, you must be running
 	FreeBSD not older than 6.0-RELEASE.  Upgrading to -current

include/fts.h

@@ -44,8 +44,8 @@ typedef struct {
 	dev_t fts_dev;			/* starting device # */
 	char *fts_path;			/* path for this descent */
 	int fts_rfd;			/* fd for root */
-	int fts_pathlen;		/* sizeof(path) */
-	int fts_nitems;			/* elements in the sort array */
+	size_t fts_pathlen;		/* sizeof(path) */
+	size_t fts_nitems;		/* elements in the sort array */
 	int (*fts_compar)		/* compare function */
 	    (const struct _ftsent * const *, const struct _ftsent * const *);
 
@@ -69,22 +69,15 @@ typedef struct _ftsent {
 	struct _ftsent *fts_cycle;	/* cycle node */
 	struct _ftsent *fts_parent;	/* parent directory */
 	struct _ftsent *fts_link;	/* next file in directory */
-	union {
-		struct {
-			long __fts_number;	/* local numeric value */
-			void *__fts_pointer;	/* local address value */
-		} __struct_ftsent;
-		int64_t __fts_bignum;
-	} __union_ftsent;
-#define	fts_number	__union_ftsent.__struct_ftsent.__fts_number
-#define	fts_pointer	__union_ftsent.__struct_ftsent.__fts_pointer
-#define	fts_bignum	__union_ftsent.__fts_bignum
+	long long fts_number;		/* local numeric value */
+#define	fts_bignum	fts_number	/* XXX non-std, should go away */
+	void *fts_pointer;		/* local address value */
 	char *fts_accpath;		/* access path */
 	char *fts_path;			/* root path */
 	int fts_errno;			/* errno for this node */
 	int fts_symfd;			/* fd for symlink */
-	u_short fts_pathlen;		/* strlen(fts_path) */
-	u_short fts_namelen;		/* strlen(fts_name) */
+	size_t fts_pathlen;		/* strlen(fts_path) */
+	size_t fts_namelen;		/* strlen(fts_name) */
 
 	ino_t fts_ino;			/* inode */
 	dev_t fts_dev;			/* device */
@@ -92,7 +85,7 @@ typedef struct _ftsent {
 
 #define	FTS_ROOTPARENTLEVEL	-1
 #define	FTS_ROOTLEVEL		 0
-	short fts_level;		/* depth (-1 to N) */
+	long fts_level;			/* depth (-1 to N) */
 
 #define	FTS_D		 1		/* preorder directory */
 #define	FTS_DC		 2		/* directory that causes cycles */
@@ -108,18 +101,18 @@ typedef struct _ftsent {
 #define	FTS_SL		12		/* symbolic link */
 #define	FTS_SLNONE	13		/* symbolic link without target */
 #define	FTS_W		14		/* whiteout object */
-	u_short fts_info;		/* user flags for FTSENT structure */
+	int fts_info;			/* user status for FTSENT structure */
 
 #define	FTS_DONTCHDIR	 0x01		/* don't chdir .. to the parent */
 #define	FTS_SYMFOLLOW	 0x02		/* followed a symlink to get here */
 #define	FTS_ISW		 0x04		/* this is a whiteout object */
-	u_short fts_flags;		/* private flags for FTSENT structure */
+	unsigned fts_flags;		/* private flags for FTSENT structure */
 
 #define	FTS_AGAIN	 1		/* read node again */
 #define	FTS_FOLLOW	 2		/* follow symbolic link */
 #define	FTS_NOINSTR	 3		/* no instructions */
 #define	FTS_SKIP	 4		/* discard node */
-	u_short fts_instr;		/* fts_set() instructions */
+	int fts_instr;			/* fts_set() instructions */
 
 	struct stat *fts_statp;		/* stat(2) information */
 	char *fts_name;			/* file name */

lib/libc/gen/Makefile.inc

@@ -11,7 +11,7 @@ SRCS+=  __getosreldate.c __xuname.c \
 	crypt.c ctermid.c daemon.c devname.c dirname.c disklabel.c \
 	dlfcn.c dlfunc.c drand48.c erand48.c err.c errlst.c errno.c \
 	exec.c feature_present.c fmtcheck.c fmtmsg.c fnmatch.c \
-	fpclassify.c frexp.c fstab.c ftok.c fts.c ftw.c \
+	fpclassify.c frexp.c fstab.c ftok.c fts.c fts-compat.c ftw.c \
 	getbootfile.c getbsize.c \
 	getcap.c getcwd.c getdomainname.c getgrent.c getgrouplist.c \
 	gethostname.c getloadavg.c getlogin.c getmntinfo.c getnetgrent.c \

lib/libc/gen/Symbol.map

@@ -134,14 +134,6 @@ FBSD_1.0 {
 	setfsent;
 	endfsent;
 	ftok;
-	fts_open;
-	fts_close;
-	fts_read;
-	fts_set;
-	fts_children;
-	fts_get_clientptr;
-	fts_get_stream;
-	fts_set_clientptr;
 	ftw;
 	glob;
 	globfree;
@@ -336,6 +328,17 @@ FBSD_1.0 {
 	wordfree;
 };
 
+FBSD_1.1 {
+	fts_open;
+	fts_close;
+	fts_read;
+	fts_set;
+	fts_children;
+	fts_get_clientptr;
+	fts_get_stream;
+	fts_set_clientptr;
+};
+
 FBSDprivate_1.0 {
 	/* needed by thread libraries */
 	__thr_jtable;

lib/libc/gen/fts-compat.c

@@ -46,12 +46,22 @@ __FBSDID("$FreeBSD$");
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
-#include <fts.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include "fts-compat.h"
 #include "un-namespace.h"
 
+FTSENT	*__fts_children_44bsd(FTS *, int);
+int	 __fts_close_44bsd(FTS *);
+void	*__fts_get_clientptr_44bsd(FTS *);
+FTS	*__fts_get_stream_44bsd(FTSENT *);
+FTS	*__fts_open_44bsd(char * const *, int,
+	    int (*)(const FTSENT * const *, const FTSENT * const *));
+FTSENT	*__fts_read_44bsd(FTS *);
+int	 __fts_set_44bsd(FTS *, FTSENT *, int);
+void	 __fts_set_clientptr_44bsd(FTS *, void *);
+
 static FTSENT	*fts_alloc(FTS *, char *, int);
 static FTSENT	*fts_build(FTS *, int);
 static void	 fts_lfree(FTSENT *);
@@ -107,7 +117,7 @@ static const char *ufslike_filesystems[] = {
 };
 
 FTS *
-fts_open(argv, options, compar)
+__fts_open_44bsd(argv, options, compar)
 	char * const *argv;
 	int options;
 	int (*compar)(const FTSENT * const *, const FTSENT * const *);
@@ -246,7 +256,7 @@ fts_load(sp, p)
 }
 
 int
-fts_close(sp)
+__fts_close_44bsd(sp)
 	FTS *sp;
 {
 	FTSENT *freep, *p;
@@ -301,7 +311,7 @@ fts_close(sp)
 	    ? p->fts_pathlen - 1 : p->fts_pathlen)
 
 FTSENT *
-fts_read(sp)
+__fts_read_44bsd(sp)
 	FTS *sp;
 {
 	FTSENT *p, *tmp;
@@ -495,7 +505,7 @@ name:		t = sp->fts_path + NAPPEND(p->fts_parent);
  */
 /* ARGSUSED */
 int
-fts_set(sp, p, instr)
+__fts_set_44bsd(sp, p, instr)
 	FTS *sp;
 	FTSENT *p;
 	int instr;
@@ -510,7 +520,7 @@ fts_set(sp, p, instr)
 }
 
 FTSENT *
-fts_children(sp, instr)
+__fts_children_44bsd(sp, instr)
 	FTS *sp;
 	int instr;
 {
@@ -582,7 +592,7 @@ fts_children(sp, instr)
 #endif
 
 void *
-(fts_get_clientptr)(FTS *sp)
+(__fts_get_clientptr_44bsd)(FTS *sp)
 {
 
 	return (fts_get_clientptr(sp));
@@ -593,13 +603,13 @@ void *
 #endif
 
 FTS *
-(fts_get_stream)(FTSENT *p)
+(__fts_get_stream_44bsd)(FTSENT *p)
 {
 	return (fts_get_stream(p));
 }
 
 void
-fts_set_clientptr(FTS *sp, void *clientptr)
+__fts_set_clientptr_44bsd(FTS *sp, void *clientptr)
 {
 
 	sp->fts_clientptr = clientptr;
@@ -1220,3 +1230,12 @@ fts_ufslinks(FTS *sp, const FTSENT *ent)
 	}
 	return (priv->ftsp_linksreliable);
 }
+
+__sym_compat(fts_open, __fts_open_44bsd, FBSD_1.0);
+__sym_compat(fts_close, __fts_close_44bsd, FBSD_1.0);
+__sym_compat(fts_read, __fts_read_44bsd, FBSD_1.0);
+__sym_compat(fts_set, __fts_set_44bsd, FBSD_1.0);
+__sym_compat(fts_children, __fts_children_44bsd, FBSD_1.0);
+__sym_compat(fts_get_clientptr, __fts_get_clientptr_44bsd, FBSD_1.0);
+__sym_compat(fts_get_stream, __fts_get_stream_44bsd, FBSD_1.0);
+__sym_compat(fts_set_clientptr, __fts_set_clientptr_44bsd, FBSD_1.0);

lib/libc/gen/fts-compat.h

@@ -126,20 +126,7 @@ typedef struct _ftsent {
 	FTS *fts_fts;			/* back pointer to main FTS */
 } FTSENT;
 
-#include <sys/cdefs.h>
-
-__BEGIN_DECLS
-FTSENT	*fts_children(FTS *, int);
-int	 fts_close(FTS *);
-void	*fts_get_clientptr(FTS *);
 #define	 fts_get_clientptr(fts)	((fts)->fts_clientptr)
-FTS	*fts_get_stream(FTSENT *);
 #define	 fts_get_stream(ftsent)	((ftsent)->fts_fts)
-FTS	*fts_open(char * const *, int,
-	    int (*)(const FTSENT * const *, const FTSENT * const *));
-FTSENT	*fts_read(FTS *);
-int	 fts_set(FTS *, FTSENT *, int);
-void	 fts_set_clientptr(FTS *, void *);
-__END_DECLS
 
 #endif /* !_FTS_H_ */

lib/libc/gen/fts.3

@@ -28,7 +28,7 @@
 .\"     @(#)fts.3	8.5 (Berkeley) 4/16/94
 .\" $FreeBSD$
 .\"
-.Dd January 7, 2005
+.Dd January 26, 2008
 .Dt FTS 3
 .Os
 .Sh NAME
@@ -133,17 +133,16 @@ structure contains at least the following fields, which are
 described in greater detail below:
 .Bd -literal
 typedef struct _ftsent {
-	u_short fts_info;		/* flags for FTSENT structure */
+	int fts_info;			/* status for FTSENT structure */
 	char *fts_accpath;		/* access path */
 	char *fts_path;			/* root path */
-	u_short fts_pathlen;		/* strlen(fts_path) */
+	size_t fts_pathlen;		/* strlen(fts_path) */
 	char *fts_name;			/* file name */
-	u_short fts_namelen;		/* strlen(fts_name) */
-	short fts_level;		/* depth (\-1 to N) */
+	size_t fts_namelen;		/* strlen(fts_name) */
+	long fts_level;			/* depth (\-1 to N) */
 	int fts_errno;			/* file errno */
-	long fts_number;		/* local numeric value */
+	long long fts_number;		/* local numeric value */
 	void *fts_pointer;		/* local address value */
-	int64_t fts_bignum;		/* local 64-bit numeric value */
 	struct ftsent *fts_parent;	/* parent directory */
 	struct ftsent *fts_link;	/* next file structure */
 	struct ftsent *fts_cycle;	/* cycle structure */
@@ -292,8 +291,6 @@ not modified by the
 .Nm
 functions.
 It is initialized to 0.
-Note that this field is overlaid by
-.Fa fts_bignum .
 .It Fa fts_pointer
 This field is provided for the use of the application program and is
 not modified by the
@@ -301,18 +298,6 @@ not modified by the
 functions.
 It is initialized to
 .Dv NULL .
-Note that this field is overlaid by
-.Fa fts_bignum .
-.It Fa fts_bignum
-This field is provided for the use of the application program and is
-not modified by the
-.Nm
-functions.
-It is initialized to 0.
-Note that this field overlays
-.Fa fts_number
-and
-.Fa fts_pointer .
 .It Fa fts_parent
 A pointer to the
 .Vt FTSENT

lib/libc/gen/fts.c

@@ -52,15 +52,15 @@ __FBSDID("$FreeBSD$");
 #include <unistd.h>
 #include "un-namespace.h"
 
-static FTSENT	*fts_alloc(FTS *, char *, int);
+static FTSENT	*fts_alloc(FTS *, char *, size_t);
 static FTSENT	*fts_build(FTS *, int);
 static void	 fts_lfree(FTSENT *);
 static void	 fts_load(FTS *, FTSENT *);
 static size_t	 fts_maxarglen(char * const *);
 static void	 fts_padjust(FTS *, FTSENT *);
 static int	 fts_palloc(FTS *, size_t);
-static FTSENT	*fts_sort(FTS *, FTSENT *, int);
-static u_short	 fts_stat(FTS *, FTSENT *, int);
+static FTSENT	*fts_sort(FTS *, FTSENT *, size_t);
+static int	 fts_stat(FTS *, FTSENT *, int);
 static int	 fts_safe_changedir(FTS *, FTSENT *, int, char *);
 static int	 fts_ufslinks(FTS *, const FTSENT *);
 
@@ -115,9 +115,8 @@ fts_open(argv, options, compar)
 	struct _fts_private *priv;
 	FTS *sp;
 	FTSENT *p, *root;
-	int nitems;
 	FTSENT *parent, *tmp;
-	int len;
+	size_t len, nitems;
 
 	/* Options check. */
 	if (options & ~FTS_OPTIONMASK) {
@@ -224,7 +223,7 @@ fts_load(sp, p)
 	FTS *sp;
 	FTSENT *p;
 {
-	int len;
+	size_t len;
 	char *cp;
 
 	/*
@@ -626,14 +625,14 @@ fts_build(sp, type)
 {
 	struct dirent *dp;
 	FTSENT *p, *head;
-	int nitems;
 	FTSENT *cur, *tail;
 	DIR *dirp;
 	void *oldaddr;
-	size_t dnamlen;
-	int cderrno, descend, len, level, maxlen, nlinks, oflag, saved_errno,
-	    nostat, doadjust;
 	char *cp;
+	int cderrno, descend, oflag, saved_errno, nostat, doadjust;
+	long level;
+	long nlinks;	/* has to be signed because -1 is a magic value */
+	size_t dnamlen, len, maxlen, nitems;
 
 	/* Set current node pointer. */
 	cur = sp->fts_cur;
@@ -741,7 +740,7 @@ fts_build(sp, type)
 		if (!ISSET(FTS_SEEDOT) && ISDOT(dp->d_name))
 			continue;
 
-		if ((p = fts_alloc(sp, dp->d_name, (int)dnamlen)) == NULL)
+		if ((p = fts_alloc(sp, dp->d_name, dnamlen)) == NULL)
 			goto mem1;
 		if (dnamlen >= maxlen) {	/* include space for NUL */
 			oldaddr = sp->fts_path;
@@ -770,21 +769,6 @@ mem1:				saved_errno = errno;
 			maxlen = sp->fts_pathlen - len;
 		}
 
-		if (len + dnamlen >= USHRT_MAX) {
-			/*
-			 * In an FTSENT, fts_pathlen is a u_short so it is
-			 * possible to wraparound here.  If we do, free up
-			 * the current structure and the structures already
-			 * allocated, then error out with ENAMETOOLONG.
-			 */
-			free(p);
-			fts_lfree(head);
-			(void)closedir(dirp);
-			cur->fts_info = FTS_ERR;
-			SET(FTS_STOP);
-			errno = ENAMETOOLONG;
-			return (NULL);
-		}
 		p->fts_level = level;
 		p->fts_parent = sp->fts_cur;
 		p->fts_pathlen = len + dnamlen;
@@ -885,7 +869,7 @@ mem1:				saved_errno = errno;
 	return (head);
 }
 
-static u_short
+static int
 fts_stat(sp, p, follow)
 	FTS *sp;
 	FTSENT *p;
@@ -987,7 +971,7 @@ static FTSENT *
 fts_sort(sp, head, nitems)
 	FTS *sp;
 	FTSENT *head;
-	int nitems;
+	size_t nitems;
 {
 	FTSENT **ap, *p;
 
@@ -1019,7 +1003,7 @@ static FTSENT *
 fts_alloc(sp, name, namelen)
 	FTS *sp;
 	char *name;
-	int namelen;
+	size_t namelen;
 {
 	FTSENT *p;
 	size_t len;
@@ -1091,18 +1075,6 @@ fts_palloc(sp, more)
 {
 
 	sp->fts_pathlen += more + 256;
-	/*
-	 * Check for possible wraparound.  In an FTS, fts_pathlen is
-	 * a signed int but in an FTSENT it is an unsigned short.
-	 * We limit fts_pathlen to USHRT_MAX to be safe in both cases.
-	 */
-	if (sp->fts_pathlen < 0 || sp->fts_pathlen >= USHRT_MAX) {
-		if (sp->fts_path)
-			free(sp->fts_path);
-		sp->fts_path = NULL;
-		errno = ENAMETOOLONG;
-		return (1);
-	}
 	sp->fts_path = reallocf(sp->fts_path, sp->fts_pathlen);
 	return (sp->fts_path == NULL);
 }

sys/sys/param.h

@@ -57,7 +57,7 @@
  *		is created, otherwise 1.
  */
 #undef __FreeBSD_version
-#define __FreeBSD_version 800014	/* Master, propagated to newvers */
+#define __FreeBSD_version 800015	/* Master, propagated to newvers */
 
 #ifndef LOCORE
 #include <sys/types.h>