revoke(2): Use unix socket externalize code to handle revoke.
authorSepherosa Ziehau <sephe@dragonflybsd.org>
Thu, 24 Sep 2015 13:50:45 +0000 (21:50 +0800)
committerSepherosa Ziehau <sephe@dragonflybsd.org>
Sat, 26 Sep 2015 02:15:05 +0000 (10:15 +0800)
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
sys/kern/uipc_usrreq.c
sys/sys/thread.h
sys/sys/un.h

index 583415e..f39c096 100644 (file)
@@ -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);
 }
index 95ee1a2..86e443e 100644 (file)
@@ -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.
  *
index c5a201e..2aa72fa 100644 (file)
@@ -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
index f6717b7..5778c7f 100644 (file)
@@ -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;