Clean up the svr4 socket cache and streams code some to make it more easily

locked.
- Move all the svr4 socket cache code into svr4_socket.c, specifically
  move svr4_delete_socket() over from streams.c.  Make the socket cache
  entry structure and svr4_head private to svr4_socket.c as a result.
- Add a mutex to protect the svr4 socket cache.
- Change svr4_find_socket() to copy the sockaddr_un struct into a
  caller-supplied sockaddr_un rather than giving the caller a pointer to
  our internal one.  This removes the one case where code outside of
  svr4_socket.c could access data in the cache.
- Add an eventhandler for process_exit and process_exec to purge the cache
  of any entries for the exiting or execing process.
- Add methods to init and destroy the socket cache and call them from the
  svr4 ABI module's event handler.
- Conditionally grab Giant around socreate() in streamsopen().
- Use fdclose() instead of inlining it in streamsopen() when handling
  socreate() failure.
- Only allocate a stream structure and attach it to a socket in
  streamsopen().  Previously, if a svr4 program performed a stream
  operation on an arbitrary socket not opened via the streams device,
  we would attach streams state data to it and change f_ops of the
  associated struct file while it was in use.  The latter was especially
  not safe, and if a program wants a stream object it should open it via
  the streams device anyway.
- Don't bother locking so_emuldata in the streams code now that we only
  touch it right after creating a socket (in streamsopen()) or when
  tearing it down when the file is closed.
- Remove D_NEEDGIANT from the streams device as it is no longer needed.

5 files changed, 139 insertions, 115 deletions

diff --git a/sys/compat/svr4/svr4_socket.c b/sys/compat/svr4/svr4_socket.c
index e8dd79b..1714bf1 100644
--- a/sys/compat/svr4/svr4_socket.c
+++ b/sys/compat/svr4/svr4_socket.c
@@ -46,7 +46,9 @@ __FBSDID("$FreeBSD$");
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/queue.h>
+#include <sys/eventhandler.h>
 #include <sys/file.h>
+#include <sys/kernel.h>
 #include <sys/lock.h>
 #include <sys/mutex.h>
 #include <sys/socket.h>
@@ -65,17 +67,34 @@ __FBSDID("$FreeBSD$");
 #include <compat/svr4/svr4_sockmod.h>
 #include <compat/svr4/svr4_proto.h>
 
-struct sockaddr_un *
-svr4_find_socket(td, fp, dev, ino)
+struct svr4_sockcache_entry {
+	struct proc *p;		/* Process for the socket		*/
+	void *cookie;		/* Internal cookie used for matching	*/
+	struct sockaddr_un sock;/* Pathname for the socket		*/
+	dev_t dev;		/* Device where the socket lives on	*/
+	ino_t ino;		/* Inode where the socket lives on	*/
+	TAILQ_ENTRY(svr4_sockcache_entry) entries;
+};
+
+static TAILQ_HEAD(, svr4_sockcache_entry) svr4_head;
+static struct mtx svr4_sockcache_lock;
+static eventhandler_tag svr4_sockcache_exit_tag, svr4_sockcache_exec_tag;
+
+static void	svr4_purge_sockcache(void *arg, struct proc *p);
+
+int
+svr4_find_socket(td, fp, dev, ino, saun)
 	struct thread *td;
 	struct file *fp;
 	dev_t dev;
 	ino_t ino;
+	struct sockaddr_un *saun;
 {
 	struct svr4_sockcache_entry *e;
 	void *cookie = ((struct socket *)fp->f_data)->so_emuldata;
 
 	DPRINTF(("svr4_find_socket: [%p,%d,%d]: ", td, dev, ino));
+	mtx_lock(&svr4_sockcache_lock);
 	TAILQ_FOREACH(e, &svr4_head, entries)
 		if (e->p == td->td_proc && e->dev == dev && e->ino == ino) {
 #ifdef DIAGNOSTIC
@@ -84,18 +103,16 @@ svr4_find_socket(td, fp, dev, ino)
 #endif
 			e->cookie = cookie;
 			DPRINTF(("%s\n", e->sock.sun_path));
-			return &e->sock;
+			*saun = e->sock;
+			mtx_unlock(&svr4_sockcache_lock);
+			return (0);
 		}
 
+	mtx_unlock(&svr4_sockcache_lock);
 	DPRINTF(("not found\n"));
-	return NULL;
+	return (ENOENT);
 }
 
-
-/*
- * svr4_delete_socket() is in sys/dev/streams.c (because it's called by
- * the streams "soo_close()" routine).
- */
 int
 svr4_add_socket(td, path, st)
 	struct thread *td;
@@ -105,8 +122,6 @@ svr4_add_socket(td, path, st)
 	struct svr4_sockcache_entry *e;
 	int len, error;
 
-	mtx_lock(&Giant);
-
 	e = malloc(sizeof(*e), M_TEMP, M_WAITOK);
 	e->cookie = NULL;
 	e->dev = st->st_dev;
@@ -115,7 +130,6 @@ svr4_add_socket(td, path, st)
 
 	if ((error = copyinstr(path, e->sock.sun_path,
 	    sizeof(e->sock.sun_path), &len)) != 0) {
-		mtx_unlock(&Giant);
 		DPRINTF(("svr4_add_socket: copyinstr failed %d\n", error));
 		free(e, M_TEMP);
 		return error;
@@ -124,13 +138,76 @@ svr4_add_socket(td, path, st)
 	e->sock.sun_family = AF_LOCAL;
 	e->sock.sun_len = len;
 
+	mtx_lock(&svr4_sockcache_lock);
 	TAILQ_INSERT_HEAD(&svr4_head, e, entries);
-	mtx_unlock(&Giant);
+	mtx_unlock(&svr4_sockcache_lock);
 	DPRINTF(("svr4_add_socket: %s [%p,%d,%d]\n", e->sock.sun_path,
 		 td->td_proc, e->dev, e->ino));
 	return 0;
 }
 
+void
+svr4_delete_socket(p, fp)
+	struct proc *p;
+	struct file *fp;
+{
+	struct svr4_sockcache_entry *e;
+	void *cookie = ((struct socket *)fp->f_data)->so_emuldata;
+
+	mtx_lock(&svr4_sockcache_lock);
+	TAILQ_FOREACH(e, &svr4_head, entries)
+		if (e->p == p && e->cookie == cookie) {
+			TAILQ_REMOVE(&svr4_head, e, entries);
+			mtx_unlock(&svr4_sockcache_lock);
+			DPRINTF(("svr4_delete_socket: %s [%p,%d,%d]\n",
+				 e->sock.sun_path, p, (int)e->dev, e->ino));
+			free(e, M_TEMP);
+			return;
+		}
+	mtx_unlock(&svr4_sockcache_lock);
+}
+
+void
+svr4_purge_sockcache(arg, p)
+	void *arg;
+	struct proc *p;
+{
+	struct svr4_sockcache_entry *e, *ne;
+
+	mtx_lock(&svr4_sockcache_lock);
+	TAILQ_FOREACH_SAFE(e, &svr4_head, entries, ne) {
+		if (e->p == p) {
+			TAILQ_REMOVE(&svr4_head, e, entries);
+			DPRINTF(("svr4_purge_sockcache: %s [%p,%d,%d]\n",
+				 e->sock.sun_path, p, (int)e->dev, e->ino));
+			free(e, M_TEMP);
+		}
+	}
+	mtx_unlock(&svr4_sockcache_lock);
+}
+
+void
+svr4_sockcache_init(void)
+{
+
+	TAILQ_INIT(&svr4_head);
+	mtx_init(&svr4_sockcache_lock, "svr4 socket cache", NULL, MTX_DEF);
+	svr4_sockcache_exit_tag = EVENTHANDLER_REGISTER(process_exit,
+	    svr4_purge_sockcache, NULL, EVENTHANDLER_PRI_ANY);
+	svr4_sockcache_exec_tag = EVENTHANDLER_REGISTER(process_exec,
+	    svr4_purge_sockcache, NULL, EVENTHANDLER_PRI_ANY);
+}
+
+void
+svr4_sockcache_destroy(void)
+{
+
+	KASSERT(TAILQ_EMPTY(&svr4_head),
+	    ("%s: sockcache entries still around", __func__));
+	EVENTHANDLER_DEREGISTER(process_exec, svr4_sockcache_exec_tag);
+	EVENTHANDLER_DEREGISTER(process_exit, svr4_sockcache_exit_tag);
+	mtx_destroy(&svr4_sockcache_lock);
+}
 
 int
 svr4_sys_socket(td, uap)
diff --git a/sys/compat/svr4/svr4_socket.h b/sys/compat/svr4/svr4_socket.h
index 26d20f0..cbe7401 100644
--- a/sys/compat/svr4/svr4_socket.h
+++ b/sys/compat/svr4/svr4_socket.h
@@ -48,22 +48,11 @@ struct svr4_sockaddr_in {
         u_char         sin_zero[8];
 };
 
-struct sockaddr_un *svr4_find_socket(struct thread *, struct file *,
-    dev_t, ino_t);
-void svr4_delete_socket(struct proc *, struct file *);
-int svr4_add_socket(struct thread *, const char *, struct stat *);
-
-struct svr4_sockcache_entry {
-	struct proc *p;		/* Process for the socket		*/
-	void *cookie;		/* Internal cookie used for matching	*/
-	struct sockaddr_un sock;/* Pathname for the socket		*/
-	dev_t dev;		/* Device where the socket lives on	*/
-	ino_t ino;		/* Inode where the socket lives on	*/
-	TAILQ_ENTRY(svr4_sockcache_entry) entries;
-};
-
-TAILQ_HEAD(svr4_sockcache_head, svr4_sockcache_entry);
-extern struct svr4_sockcache_head svr4_head;
-extern int svr4_str_initialized;
+int	svr4_add_socket(struct thread *, const char *, struct stat *);
+void	svr4_delete_socket(struct proc *, struct file *);
+int	svr4_find_socket(struct thread *, struct file *, dev_t, ino_t,
+    struct sockaddr_un *);
+void	svr4_sockcache_init(void);
+void	svr4_sockcache_destroy(void);
 
 #endif /* _SVR4_SOCKET_H_ */
diff --git a/sys/compat/svr4/svr4_stream.c b/sys/compat/svr4/svr4_stream.c
index 2d10cc9..5efb3b8 100644
--- a/sys/compat/svr4/svr4_stream.c
+++ b/sys/compat/svr4/svr4_stream.c
@@ -1554,13 +1554,11 @@ svr4_do_putmsg(td, uap, fp)
 			/* Maybe we've been given a device/inode pair */
 			dev_t *dev = SVR4_ADDROF(&sc);
 			ino_t *ino = (ino_t *) &dev[1];
-			sa = (struct sockaddr *)
-			    svr4_find_socket(td, fp, *dev, *ino);
-			if (sa == NULL) {
-				sa = (struct sockaddr *)&saun;
+			if (svr4_find_socket(td, fp, *dev, *ino, &saun) != 0) {
 				/* I guess we have it by name */
 				netaddr_to_sockaddr_un(&saun, &sc);
 			}
+			sa = (struct sockaddr *)&saun;
 			sasize = sizeof(saun);
 		}
 		break;
diff --git a/sys/compat/svr4/svr4_sysvec.c b/sys/compat/svr4/svr4_sysvec.c
index 9bfbd47..c4a2d8c 100644
--- a/sys/compat/svr4/svr4_sysvec.c
+++ b/sys/compat/svr4/svr4_sysvec.c
@@ -62,6 +62,7 @@ __FBSDID("$FreeBSD$");
 #include <compat/svr4/svr4_types.h>
 #include <compat/svr4/svr4_syscall.h>
 #include <compat/svr4/svr4_signal.h>
+#include <compat/svr4/svr4_socket.h>
 #include <compat/svr4/svr4_sockio.h>
 #include <compat/svr4/svr4_errno.h>
 #include <compat/svr4/svr4_proto.h>
@@ -269,12 +270,14 @@ svr4_elf_modevent(module_t mod, int type, void *data)
 
 	switch(type) {
 	case MOD_LOAD:
-		if (elf32_insert_brand_entry(&svr4_brand) < 0)
-			error = EINVAL;
-		if (error)
+		if (elf32_insert_brand_entry(&svr4_brand) < 0) {
 			printf("cannot insert svr4 elf brand handler\n");
-		else if (bootverbose)
+			error = EINVAL;
+			break;
+		}
+		if (bootverbose)
 			printf("svr4 ELF exec handler installed\n");
+		svr4_sockcache_init();
 		break;
 	case MOD_UNLOAD:
 		/* Only allow the emulator to be removed if it isn't in use. */
@@ -284,11 +287,14 @@ svr4_elf_modevent(module_t mod, int type, void *data)
 			error = EINVAL;
 		}
 
-		if (error)
+		if (error) {
 			printf("Could not deinstall ELF interpreter entry (error %d)\n",
 			       error);
-		else if (bootverbose)
+			break;
+		}
+		if (bootverbose)
 			printf("svr4 ELF exec handler removed\n");
+		svr4_sockcache_destroy();
 		break;
 	default:
 		return (EOPNOTSUPP);
diff --git a/sys/dev/streams/streams.c b/sys/dev/streams/streams.c
index 804d064..5c1377c 100644
--- a/sys/dev/streams/streams.c
+++ b/sys/dev/streams/streams.c
@@ -68,8 +68,6 @@ static int svr4_soo_close(struct file *, struct thread *);
 static int svr4_ptm_alloc(struct thread *);
 static  d_open_t	streamsopen;
 
-struct svr4_sockcache_head svr4_head;
-
 /*
  * Device minor numbers
  */
@@ -101,7 +99,6 @@ static struct fileops svr4_netops = {
  
 static struct cdevsw streams_cdevsw = {
 	.d_version =	D_VERSION,
-	.d_flags =	D_NEEDGIANT,
 	.d_open =	streamsopen,
 	.d_name =	"streams",
 };
@@ -119,7 +116,6 @@ streams_modevent(module_t mod, int type, void *unused)
 {
 	switch (type) {
 	case MOD_LOAD:
-		TAILQ_INIT(&svr4_head);
 		dt_ptm = make_dev(&streams_cdevsw, dev_ptm, 0, 0, 0666,
 			"ptm");
 		dt_arp = make_dev(&streams_cdevsw, dev_arp, 0, 0, 0666,
@@ -187,13 +183,12 @@ MODULE_VERSION(streams, 1);
 static  int
 streamsopen(struct cdev *dev, int oflags, int devtype, struct thread *td)
 {
-	int type, protocol;
-	int fd, extraref;
-	struct file *fp;
+	struct filedesc *fdp;
+	struct svr4_strm *st;
 	struct socket *so;
-	int error;
-	int family;
-	struct proc *p = td->td_proc;
+	struct file *fp;
+	int family, type, protocol;
+	int error, fd;
 	
 	if (td->td_dupfd >= 0)
 	  return ENODEV;
@@ -244,34 +239,40 @@ streamsopen(struct cdev *dev, int oflags, int devtype, struct thread *td)
 	  return EOPNOTSUPP;
 	}
 
+	fdp = td->td_proc->p_fd;
 	if ((error = falloc(td, &fp, &fd)) != 0)
 	  return error;
 	/* An extra reference on `fp' has been held for us by falloc(). */
 
-	if ((error = socreate(family, &so, type, protocol,
-	    td->td_ucred, td)) != 0) {
-	  FILEDESC_LOCK_FAST(p->p_fd);
-	  /* Check the fd table entry hasn't changed since we made it. */
-	  extraref = 0;
-	  if (p->p_fd->fd_ofiles[fd] == fp) {
-	    p->p_fd->fd_ofiles[fd] = NULL;
-	    extraref = 1;
-	  }
-	  FILEDESC_UNLOCK_FAST(p->p_fd);
-	  if (extraref)
-	    fdrop(fp, td);
-	  fdrop(fp, td);
-	  return error;
+	NET_LOCK_GIANT();
+	error = socreate(family, &so, type, protocol, td->td_ucred, td);
+	NET_UNLOCK_GIANT();
+	if (error) {
+	   fdclose(fdp, fp, fd, td);
+	   fdrop(fp, td);
+	   return error;
 	}
 
-	FILEDESC_LOCK_FAST(p->p_fd);
+	FILEDESC_LOCK_FAST(fdp);
 	fp->f_data = so;
 	fp->f_flag = FREAD|FWRITE;
 	fp->f_ops = &svr4_netops;
 	fp->f_type = DTYPE_SOCKET;
-	FILEDESC_UNLOCK_FAST(p->p_fd);
+	FILEDESC_UNLOCK_FAST(fdp);
+
+	/*
+	 * Allocate a stream structure and attach it to this socket.
+	 * We don't bother locking so_emuldata for SVR4 stream sockets as
+	 * its value is constant for the lifetime of the stream once it
+	 * is initialized here.
+	 */
+	st = malloc(sizeof(struct svr4_strm), M_TEMP, M_WAITOK);
+	st->s_family = so->so_proto->pr_domain->dom_family;
+	st->s_cmd = ~0;
+	st->s_afd = -1;
+	st->s_eventmask = 0;
+	so->so_emuldata = st;
 
-	(void)svr4_stream_get(fp);
 	fdrop(fp, td);
 	td->td_dupfd = fd;
 	return ENXIO;
@@ -334,59 +335,12 @@ svr4_stream_get(fp)
 	struct file *fp;
 {
 	struct socket *so;
-	struct svr4_strm *st;
 
 	if (fp == NULL || fp->f_type != DTYPE_SOCKET)
 		return NULL;
 
 	so = fp->f_data;
-
-	/*
-	 * mpfixme: lock socketbuffer here
-	 */
-	if (so->so_emuldata) {
-		return so->so_emuldata;
-	}
-
-	/* Allocate a new one. */
-	st = malloc(sizeof(struct svr4_strm), M_TEMP, M_WAITOK);
-	st->s_family = so->so_proto->pr_domain->dom_family;
-	st->s_cmd = ~0;
-	st->s_afd = -1;
-	st->s_eventmask = 0;
-	/*
-	 * avoid a race where we loose due to concurrancy issues
-	 * of two threads trying to allocate the so_emuldata.
-	 */
-	if (so->so_emuldata) {
-		/* lost the race, use the existing emuldata */
-		FREE(st, M_TEMP);
-		st = so->so_emuldata;
-	} else {
-		/* we won, or there was no race, use our copy */
-		so->so_emuldata = st;
-		fp->f_ops = &svr4_netops;
-	}
-
-	return st;
-}
-
-void
-svr4_delete_socket(p, fp)
-	struct proc *p;
-	struct file *fp;
-{
-	struct svr4_sockcache_entry *e;
-	void *cookie = ((struct socket *)fp->f_data)->so_emuldata;
-
-	TAILQ_FOREACH(e, &svr4_head, entries)
-		if (e->p == p && e->cookie == cookie) {
-			TAILQ_REMOVE(&svr4_head, e, entries);
-			DPRINTF(("svr4_delete_socket: %s [%p,%d,%d]\n",
-				 e->sock.sun_path, p, (int)e->dev, e->ino));
-			free(e, M_TEMP);
-			return;
-		}
+	return so->so_emuldata;
 }
 
 static int