From f635d1746140d0f4915af3adb8314f4759377d3b Mon Sep 17 00:00:00 2001 From: Sepherosa Ziehau Date: Thu, 24 Sep 2015 21:50:45 +0800 Subject: [PATCH] revoke(2): Use unix socket externalize code to handle revoke. Use revoke token in shared mode in unix socket code and in exclusive mode in fdrevoke(); mainly to make sure that all fps externalized and marked FREVOKED will be picked up by later allproc_scan() called by fdrevoke(). This one greatly simplies the code on unix socket side. The original unix socket revoke handling was also kinda broken: it tried to hold socket reception buffer token w/ all filedesc spin lock being held. Go-Ahead-by: dillon@ --- sys/kern/kern_descrip.c | 19 +++--- sys/kern/uipc_usrreq.c | 127 ++-------------------------------------- sys/sys/thread.h | 1 + sys/sys/un.h | 1 - 4 files changed, 16 insertions(+), 132 deletions(-) diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c index 583415ea46..f39c096e90 100644 --- a/sys/kern/kern_descrip.c +++ b/sys/kern/kern_descrip.c @@ -130,6 +130,8 @@ static struct spinlock filehead_spin = SPINLOCK_INITIALIZER(&filehead_spin, "fil static int nfiles; /* actual number of open files */ extern int cmask; +struct lwkt_token revoke_token = LWKT_TOKEN_INITIALIZER(revoke_token); + /* * Fixup fd_freefile and fd_lastfile after a descriptor has been cleared. * @@ -1339,7 +1341,6 @@ struct fdrevoke_info { short type; short unused; int found; - int intransit; struct ucred *cred; struct file *nfp; }; @@ -1365,23 +1366,23 @@ fdrevoke(void *f_data, short f_type, struct ucred *cred) * Scan the file pointer table once. dups do not dup file pointers, * only descriptors, so there is no leak. Set FREVOKED on the fps * being revoked. + * + * Any fps sent over unix-domain sockets will be revoked by the + * socket code checking for FREVOKED when the fps are externialized. + * revoke_token is used to make sure that fps marked FREVOKED and + * externalized will be picked up by the following allproc_scan(). */ + lwkt_gettoken(&revoke_token); allfiles_scan_exclusive(fdrevoke_check_callback, &info); + lwkt_reltoken(&revoke_token); /* * If any fps were marked track down the related descriptors * and close them. Any dup()s at this point will notice * the FREVOKED already set in the fp and do the right thing. - * - * Any fps with non-zero msgcounts (aka sent over a unix-domain - * socket) bumped the intransit counter and will require a - * scan. Races against fps leaving the socket are closed by - * the socket code checking for FREVOKED. */ if (info.found) allproc_scan(fdrevoke_proc_callback, &info); - if (info.intransit) - unp_revoke_gc(info.nfp); fdrop(info.nfp); return(0); } @@ -1421,8 +1422,6 @@ fdrevoke_check_callback(struct file *fp, void *vinfo) if (info->data == fp->f_data && info->type == fp->f_type) { atomic_set_int(&fp->f_flag, FREVOKED); info->found = 1; - if (fp->f_msgcount) - ++info->intransit; } return(0); } diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c index 95ee1a2a56..86e443ebca 100644 --- a/sys/kern/uipc_usrreq.c +++ b/sys/kern/uipc_usrreq.c @@ -123,7 +123,6 @@ static void unp_gc (void); static int unp_gc_clearmarks(struct file *, void *); static int unp_gc_checkmarks(struct file *, void *); static int unp_gc_checkrefs(struct file *, void *); -static int unp_revoke_gc_check(struct file *, void *); static void unp_scan (struct mbuf *, void (*)(struct file *, void *), void *data); static void unp_mark (struct file *, void *data); @@ -1435,8 +1434,6 @@ unp_externalize(struct mbuf *rights) / sizeof(struct file *); int f; - lwkt_gettoken(&unp_token); - /* * if the new FD's will not fit, then we free them all */ @@ -1451,7 +1448,6 @@ unp_externalize(struct mbuf *rights) *rp++ = NULL; unp_discard(fp, NULL); } - lwkt_reltoken(&unp_token); return (EMSGSIZE); } @@ -1465,7 +1461,11 @@ unp_externalize(struct mbuf *rights) * struct file pointer. * If sizeof (struct file *) is smaller than sizeof int, then * do it in reverse order. + * + * Hold revoke_token in 'shared' mode, so that we won't miss + * the FREVOKED update on fps being externalized (fsetfd). */ + lwkt_gettoken_shared(&revoke_token); if (sizeof(struct file *) >= sizeof(int)) { fdp = (int *)CMSG_DATA(cm); rp = (struct file **)CMSG_DATA(cm); @@ -1490,7 +1490,7 @@ unp_externalize(struct mbuf *rights) for (i = 0; i < newfds; i++) rp[i] = NULL; - lwkt_reltoken(&unp_token); + lwkt_reltoken(&revoke_token); return (EMSGSIZE); } fp = rp[i]; @@ -1513,6 +1513,7 @@ unp_externalize(struct mbuf *rights) *fdp-- = f; } } + lwkt_reltoken(&revoke_token); /* * Adjust length, in case sizeof(struct file *) and sizeof(int) @@ -1521,15 +1522,12 @@ unp_externalize(struct mbuf *rights) cm->cmsg_len = CMSG_LEN(newfds * sizeof(int)); rights->m_len = cm->cmsg_len; - lwkt_reltoken(&unp_token); return (0); } static void unp_fp_externalize(struct lwp *lp, struct file *fp, int fd) { - lwkt_gettoken(&unp_token); - if (lp) { KKASSERT(fd >= 0); if (fp->f_flag & FREVOKED) { @@ -1553,8 +1551,6 @@ unp_fp_externalize(struct lwp *lp, struct file *fp, int fd) unp_rights--; spin_unlock(&unp_spin); fdrop(fp); - - lwkt_reltoken(&unp_token); } void @@ -1958,117 +1954,6 @@ unp_gc_checkmarks(struct file *fp, void *data) return (0); } -/* - * Scan all unix domain sockets and replace any revoked file pointers - * found with the dummy file pointer fx. We don't worry about races - * against file pointers being read out as those are handled in the - * externalize code. - */ - -#define REVOKE_GC_MAXFILES 32 - -struct unp_revoke_gc_info { - struct file *fx; - struct file *fary[REVOKE_GC_MAXFILES]; - int fcount; -}; - -void -unp_revoke_gc(struct file *fx) -{ - struct unp_revoke_gc_info info; - int i; - - lwkt_gettoken(&unp_token); - info.fx = fx; - do { - info.fcount = 0; - allfiles_scan_exclusive(unp_revoke_gc_check, &info); - for (i = 0; i < info.fcount; ++i) - unp_fp_externalize(NULL, info.fary[i], -1); - } while (info.fcount == REVOKE_GC_MAXFILES); - lwkt_reltoken(&unp_token); -} - -/* - * Check for and replace revoked descriptors. - * - * WARNING: This routine is not allowed to block. - */ -static int -unp_revoke_gc_check(struct file *fps, void *vinfo) -{ - struct unp_revoke_gc_info *info = vinfo; - struct file *fp; - struct socket *so; - struct mbuf *m0; - struct mbuf *m; - struct file **rp; - struct cmsghdr *cm; - int i; - int qfds; - - /* - * Is this a unix domain socket with rights-passing abilities? - */ - if (fps->f_type != DTYPE_SOCKET) - return (0); - if ((so = (struct socket *)fps->f_data) == NULL) - return(0); - if (so->so_proto->pr_domain != &localdomain) - return(0); - if ((so->so_proto->pr_flags & PR_RIGHTS) == 0) - return(0); - - /* - * Scan the mbufs for control messages and replace any revoked - * descriptors we find. - */ - lwkt_gettoken(&so->so_rcv.ssb_token); - m0 = so->so_rcv.ssb_mb; - while (m0) { - for (m = m0; m; m = m->m_next) { - if (m->m_type != MT_CONTROL) - continue; - if (m->m_len < sizeof(*cm)) - continue; - cm = mtod(m, struct cmsghdr *); - if (cm->cmsg_level != SOL_SOCKET || - cm->cmsg_type != SCM_RIGHTS) { - continue; - } - qfds = (cm->cmsg_len - CMSG_LEN(0)) / sizeof(void *); - rp = (struct file **)CMSG_DATA(cm); - for (i = 0; i < qfds; i++) { - fp = rp[i]; - if (fp->f_flag & FREVOKED) { - kprintf("Warning: Removing revoked fp from unix domain socket queue\n"); - fhold(info->fx); - info->fx->f_msgcount++; - unp_rights++; - rp[i] = info->fx; - info->fary[info->fcount++] = fp; - } - if (info->fcount == REVOKE_GC_MAXFILES) - break; - } - if (info->fcount == REVOKE_GC_MAXFILES) - break; - } - m0 = m0->m_nextpkt; - if (info->fcount == REVOKE_GC_MAXFILES) - break; - } - lwkt_reltoken(&so->so_rcv.ssb_token); - - /* - * Stop the scan if we filled up our array. - */ - if (info->fcount == REVOKE_GC_MAXFILES) - return(-1); - return(0); -} - /* * Dispose of the fp's stored in a mbuf. * diff --git a/sys/sys/thread.h b/sys/sys/thread.h index c5a201e268..2aa72fadd4 100644 --- a/sys/sys/thread.h +++ b/sys/sys/thread.h @@ -427,6 +427,7 @@ extern struct lwkt_token kvm_token; extern struct lwkt_token sigio_token; extern struct lwkt_token tty_token; extern struct lwkt_token vnode_token; +extern struct lwkt_token revoke_token; /* * Procedures diff --git a/sys/sys/un.h b/sys/sys/un.h index f6717b70b5..5778c7fbb6 100644 --- a/sys/sys/un.h +++ b/sys/sys/un.h @@ -72,7 +72,6 @@ int uipc_usrreq (struct socket *so, int req, struct mbuf *m, void uipc_ctloutput (union netmsg *msg); int unp_connect2 (struct socket *so, struct socket *so2); void unp_dispose (struct mbuf *m); -void unp_revoke_gc (struct file *fx); int unp_externalize (struct mbuf *rights); void unp_init (void); extern struct pr_usrreqs uipc_usrreqs; -- 2.41.0