From 8a1f79ab85c00574b27570ea0a8fc13018f8d7f4 Mon Sep 17 00:00:00 2001 From: jhb Date: Fri, 21 Jul 2006 20:40:13 +0000 Subject: [PATCH] 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. --- sys/compat/svr4/svr4_socket.c | 103 ++++++++++++++++++++++++++++----- sys/compat/svr4/svr4_socket.h | 23 ++------ sys/compat/svr4/svr4_stream.c | 6 +- sys/compat/svr4/svr4_sysvec.c | 18 ++++-- sys/dev/streams/streams.c | 104 ++++++++++------------------------ 5 files changed, 139 insertions(+), 115 deletions(-) diff --git a/sys/compat/svr4/svr4_socket.c b/sys/compat/svr4/svr4_socket.c index e8dd79ba5e64..1714bf1d3378 100644 --- a/sys/compat/svr4/svr4_socket.c +++ b/sys/compat/svr4/svr4_socket.c @@ -46,7 +46,9 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include +#include #include #include #include @@ -65,17 +67,34 @@ __FBSDID("$FreeBSD$"); #include #include -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 26d20f0ae548..cbe7401f5693 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 2d10cc984a48..5efb3b818114 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 9bfbd4790148..c4a2d8c1825a 100644 --- a/sys/compat/svr4/svr4_sysvec.c +++ b/sys/compat/svr4/svr4_sysvec.c @@ -62,6 +62,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -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 804d0640fbeb..5c1377cc83d3 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