[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

diff: plug telldir/seekdir leaks and more



Hi,

This is a revised version of the diff Paul Thorn
<pthorn-obsd_(_at_)_styx2002_(_dot_)_no-ip_(_dot_)_org> send some time ago. It's a mix of
Paul's diff, FreeBSD code and my own:

- plug a huge leak that occurs if telldir() is called, but no
corresponding seekdir(). Samba is suffering from that. 
- Use a data structure local to DIR to store the telldir data. FreeBSD
uses a linked list, I chose to use an array, which avoids allocating
lots of small chunks, the index becomes implicit as well, so we can
drop a field from the struct.
- Make sure that loc = telldir(); .... seekdir(loc); telldir() returns loc,
as POSIX requires. This is an area that can be improved, since it now
scans the array.

Note that one documented behaviour is changed. POSIX does not require
it, and most other Unix-like system do not give that guarantee.

Please review and test, especially on setup that uses telldir and
seekdir. AFAIK, no program in base does that, but at least samba
does.

	-Otto

Index: include/dirent.h
===================================================================
RCS file: /cvs/src/include/dirent.h,v
retrieving revision 1.15
diff -u -p -r1.15 dirent.h
--- include/dirent.h	13 Dec 2005 00:35:22 -0000	1.15
+++ include/dirent.h	24 Mar 2006 09:08:19 -0000
@@ -59,6 +59,7 @@
 /* definitions for library routines operating on directories. */
 #define	DIRBLKSIZ	1024
 
+struct _telldir;
 /* structure describing an open directory. */
 typedef struct _dirdesc {
 	int	dd_fd;		/* file descriptor associated with directory */
@@ -69,6 +70,7 @@ typedef struct _dirdesc {
 	long	dd_seek;	/* magic cookie returned by getdirentries */
 	long	dd_rewind;	/* magic cookie for rewinding */
 	int	dd_flags;	/* flags for readdir */
+	struct _telldir *dd_td; /* telldir position recording */
 } DIR;
 
 #define	dirfd(dirp)	((dirp)->dd_fd)
@@ -106,7 +108,7 @@ int getdirentries(int, char *, int, long
 		__attribute__ ((__bounded__(__string__,2,3)));
 #endif /* __BSD_VISIBLE */
 #if __XPG_VISIBLE
-long telldir(const DIR *);
+long telldir(DIR *);
 void seekdir(DIR *, long);
 #endif
 #if __POSIX_VISIBLE >= 199506 || __XPG_VISIBLE >= 500
Index: lib/libc/gen/closedir.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/closedir.c,v
retrieving revision 1.6
diff -u -p -r1.6 closedir.c
--- lib/libc/gen/closedir.c	8 Aug 2005 08:05:33 -0000	1.6
+++ lib/libc/gen/closedir.c	24 Mar 2006 09:08:19 -0000
@@ -33,6 +33,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include "thread_private.h"
+#include "telldir.h"
 
 /*
  * close a directory.
@@ -45,12 +46,12 @@ closedir(DIR *dirp)
 
 	if ((ret = _FD_LOCK(dirp->dd_fd, FD_READ, NULL)) != 0)
 		return (ret);
-	seekdir(dirp, dirp->dd_rewind);	/* free seekdir storage */
 	fd = dirp->dd_fd;
 	dirp->dd_fd = -1;
 	dirp->dd_loc = 0;
-	free((void *)dirp->dd_buf);
-	free((void *)dirp);
+	free(dirp->dd_td->td_locs);
+	free(dirp->dd_buf);
+	free(dirp);
 	ret = close(fd);
 	_FD_UNLOCK(fd, FD_READ);
 	return (ret);
Index: lib/libc/gen/directory.3
===================================================================
RCS file: /cvs/src/lib/libc/gen/directory.3,v
retrieving revision 1.16
diff -u -p -r1.16 directory.3
--- lib/libc/gen/directory.3	2 Jun 2003 20:18:34 -0000	1.16
+++ lib/libc/gen/directory.3	24 Mar 2006 09:08:19 -0000
@@ -135,12 +135,6 @@ from which they are derived.
 If the directory is closed and then reopened, the
 .Fn telldir
 value may be invalidated due to undetected directory compaction.
-It is safe to use a previous
-.Fn telldir
-value immediately after a call to
-.Fn opendir
-and before any calls to
-.Fn readdir .
 .Pp
 The
 .Fn rewinddir
Index: lib/libc/gen/opendir.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/opendir.c,v
retrieving revision 1.15
diff -u -p -r1.15 opendir.c
--- lib/libc/gen/opendir.c	10 Oct 2005 17:37:43 -0000	1.15
+++ lib/libc/gen/opendir.c	24 Mar 2006 09:08:19 -0000
@@ -40,6 +40,8 @@
 #include <string.h>
 #include <unistd.h>
 
+#include "telldir.h"
+
 /*
  * Open a directory.
  */
@@ -67,10 +69,16 @@ __opendir2(const char *name, int flags)
 		return (NULL);
 	}
 	if (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1 ||
-	    (dirp = (DIR *)malloc(sizeof(DIR))) == NULL) {
+	    (dirp = malloc(sizeof(DIR) + sizeof(struct _telldir))) == NULL) {
 		close(fd);
 		return (NULL);
 	}
+
+	dirp->dd_td = (struct _telldir *)((char *)dirp + sizeof(DIR));
+	dirp->dd_td->td_locs = NULL;
+	dirp->dd_td->td_sz = 0;
+	dirp->dd_td->td_loccnt = 0;
+
 
 	/*
 	 * If the machine's page size is an exact multiple of DIRBLKSIZ,
Index: lib/libc/gen/seekdir.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/seekdir.c,v
retrieving revision 1.7
diff -u -p -r1.7 seekdir.c
--- lib/libc/gen/seekdir.c	8 Aug 2005 08:05:34 -0000	1.7
+++ lib/libc/gen/seekdir.c	24 Mar 2006 09:08:19 -0000
@@ -30,8 +30,7 @@
 
 #include <sys/param.h>
 #include <dirent.h>
-
-void __seekdir(DIR *, long);
+#include "telldir.h"
 
 /*
  * Seek to an entry in a directory.
Index: lib/libc/gen/telldir.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/telldir.c,v
retrieving revision 1.7
diff -u -p -r1.7 telldir.c
--- lib/libc/gen/telldir.c	8 Aug 2005 08:05:34 -0000	1.7
+++ lib/libc/gen/telldir.c	24 Mar 2006 09:08:19 -0000
@@ -29,56 +29,43 @@
  */
 
 #include <sys/param.h>
+#include <sys/queue.h>
 #include <dirent.h>
 #include <stdlib.h>
 #include <unistd.h>
 
-/*
- * The option SINGLEUSE may be defined to say that a telldir
- * cookie may be used only once before it is freed. This option
- * is used to avoid having memory usage grow without bound.
- */
-#define SINGLEUSE
-
-/*
- * One of these structures is malloced to describe the current directory
- * position each time telldir is called. It records the current magic 
- * cookie returned by getdirentries and the offset within the buffer
- * associated with that return value.
- */
-struct ddloc {
-	struct	ddloc *loc_next;/* next structure in list */
-	long	loc_index;	/* key associated with structure */
-	long	loc_seek;	/* magic cookie returned by getdirentries */
-	long	loc_loc;	/* offset of entry in buffer */
-};
-
-#define	NDIRHASH	32	/* Num of hash lists, must be a power of 2 */
-#define	LOCHASH(i)	((i)&(NDIRHASH-1))
-
-static long	dd_loccnt;	/* Index of entry for sequential readdir's */
-static struct	ddloc *dd_hash[NDIRHASH];   /* Hash list heads for ddlocs */
-
-void __seekdir(DIR *, long);
+#include "telldir.h"
 
 /*
  * return a pointer into a directory
  */
 long
-telldir(const DIR *dirp)
+telldir(DIR *dirp)
 {
-	int index;
-	struct ddloc *lp;
+	long i = 0;
+	struct ddloc *lp = dirp->dd_td->td_locs;
+
+	/* return previous telldir, if there */
+	for (; i < dirp->dd_td->td_loccnt; i++, lp++) {
+		if (lp->loc_seek == dirp->dd_seek && 
+		    lp->loc_loc == dirp->dd_loc)
+			return (i);
+	}
 
-	if ((lp = (struct ddloc *)malloc(sizeof(struct ddloc))) == NULL)
-		return (-1);
-	index = dd_loccnt++;
-	lp->loc_index = index;
+	if (dirp->dd_td->td_loccnt == dirp->dd_td->td_sz) {
+		size_t newsz = dirp->dd_td->td_sz * 2 + 1;
+		struct ddloc *p;
+		p = realloc(dirp->dd_td->td_locs, newsz * sizeof(*p));
+		if (p == NULL)
+			return (-1);
+		dirp->dd_td->td_sz = newsz;
+		dirp->dd_td->td_locs = p;
+		lp = &dirp->dd_td->td_locs[i];
+	}
+	dirp->dd_td->td_loccnt++;
 	lp->loc_seek = dirp->dd_seek;
 	lp->loc_loc = dirp->dd_loc;
-	lp->loc_next = dd_hash[LOCHASH(index)];
-	dd_hash[LOCHASH(index)] = lp;
-	return (index);
+	return (i);
 }
 
 /*
@@ -89,21 +76,13 @@ void
 __seekdir(DIR *dirp, long loc)
 {
 	struct ddloc *lp;
-	struct ddloc **prevlp;
 	struct dirent *dp;
 
-	prevlp = &dd_hash[LOCHASH(loc)];
-	lp = *prevlp;
-	while (lp != NULL) {
-		if (lp->loc_index == loc)
-			break;
-		prevlp = &lp->loc_next;
-		lp = lp->loc_next;
-	}
-	if (lp == NULL)
+	if (loc < 0 || loc >= dirp->dd_td->td_loccnt)
 		return;
+	lp = &dirp->dd_td->td_locs[loc];
 	if (lp->loc_loc == dirp->dd_loc && lp->loc_seek == dirp->dd_seek)
-		goto found;
+		return;
 	(void) lseek(dirp->dd_fd, (off_t)lp->loc_seek, SEEK_SET);
 	dirp->dd_seek = lp->loc_seek;
 	dirp->dd_loc = 0;
@@ -112,9 +91,4 @@ __seekdir(DIR *dirp, long loc)
 		if (dp == NULL)
 			break;
 	}
-found:
-#ifdef SINGLEUSE
-	*prevlp = lp->loc_next;
-	free((caddr_t)lp);
-#endif
 }
Index: lib/libc/gen/telldir.h
===================================================================
RCS file: lib/libc/gen/telldir.h
diff -N lib/libc/gen/telldir.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ lib/libc/gen/telldir.h	24 Mar 2006 09:08:19 -0000
@@ -0,0 +1,62 @@
+/*	$OpenBSD$	*/
+/*
+ * Copyright (c) 1983, 1993
+ *	The Regents of the University of California.  All rights reserved.
+ *
+ * Copyright (c) 2000
+ * 	Daniel Eischen.  All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the University nor the names of its contributors
+ *    may be used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * $FreeBSD: src/lib/libc/gen/telldir.h,v 1.2 2001/01/24 12:59:24 deischen Exp $
+ */
+
+#ifndef _TELLDIR_H_
+#define	_TELLDIR_H_
+
+/*
+ * One of these structures is malloced to describe the current directory
+ * position each time telldir is called. It records the current magic
+ * cookie returned by getdirentries and the offset within the buffer
+ * associated with that return value.
+ */
+struct ddloc {
+	long	loc_seek;	/* magic cookie returned by getdirentries */
+	long	loc_loc;	/* offset of entry in buffer */
+};
+
+/*
+ * One of these structures is malloced for each DIR to record telldir
+ * positions.
+ */
+struct _telldir {
+	struct ddloc *td_locs;	/* locations */
+	size_t	td_sz;		/* size of locations */
+	long	td_loccnt;	/* index of entry for sequential readdir's */
+};
+
+void 		__seekdir(DIR *, long);
+
+#endif