Author: rmacklem
Date: Mon Apr 15 01:27:15 2019
New Revision: 346217
URL: https://svnweb.freebsd.org/changeset/base/346217

Log:
  Fix the NFSv4 client to safely find processes.
  
  r340744 broke the NFSv4 client, because it replaced pfind_locked() with a
  call to pfind(), since pfind() acquires the sx lock for the pid hash and
  the NFSv4 already holds a mutex when it does the call.
  The patch fixes the problem by recreating a pfind_any_locked() and adding the
  functions pidhash_slockall() and pidhash_sunlockall to acquire/release
  all of the pid hash locks.
  These functions are then used by the NFSv4 client instead of acquiring
  the allproc_lock and calling pfind().
  
  Reviewed by:  kib, mjg
  MFC after:    2 weeks
  Differential Revision:        https://reviews.freebsd.org/D19887

Modified:
  head/sys/fs/nfs/nfsport.h
  head/sys/fs/nfsclient/nfs_clport.c
  head/sys/fs/nfsclient/nfs_clstate.c
  head/sys/kern/kern_proc.c
  head/sys/sys/proc.h

Modified: head/sys/fs/nfs/nfsport.h
==============================================================================
--- head/sys/fs/nfs/nfsport.h   Sun Apr 14 18:04:53 2019        (r346216)
+++ head/sys/fs/nfs/nfsport.h   Mon Apr 15 01:27:15 2019        (r346217)
@@ -692,8 +692,6 @@ void nfsrvd_rcv(struct socket *, void *, int);
 #define        NFSUNLOCKMNT(m)         mtx_unlock(&((m)->nm_mtx))
 #define        NFSLOCKREQUEST(r)       mtx_lock(&((r)->r_mtx))
 #define        NFSUNLOCKREQUEST(r)     mtx_unlock(&((r)->r_mtx))
-#define        NFSPROCLISTLOCK()       sx_slock(&allproc_lock)
-#define        NFSPROCLISTUNLOCK()     sx_sunlock(&allproc_lock)
 #define        NFSLOCKSOCKREQ(r)       mtx_lock(&((r)->nr_mtx))
 #define        NFSUNLOCKSOCKREQ(r)     mtx_unlock(&((r)->nr_mtx))
 #define        NFSLOCKDS(d)            mtx_lock(&((d)->nfsclds_mtx))

Modified: head/sys/fs/nfsclient/nfs_clport.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clport.c  Sun Apr 14 18:04:53 2019        
(r346216)
+++ head/sys/fs/nfsclient/nfs_clport.c  Mon Apr 15 01:27:15 2019        
(r346217)
@@ -1156,7 +1156,7 @@ nfscl_procdoesntexist(u_int8_t *own)
        tl.cval[2] = *own++;
        tl.cval[3] = *own++;
        pid = tl.lval;
-       p = pfind(pid);
+       p = pfind_any_locked(pid);
        if (p == NULL)
                return (1);
        if (p->p_stats == NULL) {

Modified: head/sys/fs/nfsclient/nfs_clstate.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clstate.c Sun Apr 14 18:04:53 2019        
(r346216)
+++ head/sys/fs/nfsclient/nfs_clstate.c Mon Apr 15 01:27:15 2019        
(r346217)
@@ -1789,7 +1789,13 @@ nfscl_cleanupkext(struct nfsclclient *clp, struct nfsc
        struct nfscllockowner *lp, *nlp;
        struct nfscldeleg *dp;
 
-       NFSPROCLISTLOCK();
+       /*
+        * All the pidhash locks must be acquired, since they are sx locks
+        * and must be acquired before the mutexes.  The pid(s) that will
+        * be used aren't known yet, so all the locks need to be acquired.
+        * Fortunately, this function is only performed once/sec.
+        */
+       pidhash_slockall();
        NFSLOCKCLSTATE();
        LIST_FOREACH_SAFE(owp, &clp->nfsc_owner, nfsow_list, nowp) {
                LIST_FOREACH(op, &owp->nfsow_open, nfso_list) {
@@ -1816,7 +1822,7 @@ nfscl_cleanupkext(struct nfsclclient *clp, struct nfsc
                }
        }
        NFSUNLOCKCLSTATE();
-       NFSPROCLISTUNLOCK();
+       pidhash_sunlockall();
 }
 
 /*

Modified: head/sys/kern/kern_proc.c
==============================================================================
--- head/sys/kern/kern_proc.c   Sun Apr 14 18:04:53 2019        (r346216)
+++ head/sys/kern/kern_proc.c   Mon Apr 15 01:27:15 2019        (r346217)
@@ -193,7 +193,7 @@ procinit(void)
        pidhashtbl_lock = malloc(sizeof(*pidhashtbl_lock) * (pidhashlock + 1),
            M_PROC, M_WAITOK | M_ZERO);
        for (i = 0; i < pidhashlock + 1; i++)
-               sx_init(&pidhashtbl_lock[i], "pidhash");
+               sx_init_flags(&pidhashtbl_lock[i], "pidhash", SX_DUPOK);
        pgrphashtbl = hashinit(maxproc / 4, M_PROC, &pgrphash);
        proc_zone = uma_zcreate("PROC", sched_sizeof_proc(),
            proc_ctor, proc_dtor, proc_init, proc_fini,
@@ -365,6 +365,52 @@ inferior(struct proc *p)
                        return (0);
        }
        return (1);
+}
+
+/*
+ * Shared lock all the pid hash lists.
+ */
+void
+pidhash_slockall(void)
+{
+       u_long i;
+
+       for (i = 0; i < pidhashlock + 1; i++)
+               sx_slock(&pidhashtbl_lock[i]);
+}
+
+/*
+ * Shared unlock all the pid hash lists.
+ */
+void
+pidhash_sunlockall(void)
+{
+       u_long i;
+
+       for (i = 0; i < pidhashlock + 1; i++)
+               sx_sunlock(&pidhashtbl_lock[i]);
+}
+
+/*
+ * Similar to pfind_any(), this function finds zombies.
+ */
+struct proc *
+pfind_any_locked(pid_t pid)
+{
+       struct proc *p;
+
+       sx_assert(PIDHASHLOCK(pid), SX_LOCKED);
+       LIST_FOREACH(p, PIDHASH(pid), p_hash) {
+               if (p->p_pid == pid) {
+                       PROC_LOCK(p);
+                       if (p->p_state == PRS_NEW) {
+                               PROC_UNLOCK(p);
+                               p = NULL;
+                       }
+                       break;
+               }
+       }
+       return (p);
 }
 
 /*

Modified: head/sys/sys/proc.h
==============================================================================
--- head/sys/sys/proc.h Sun Apr 14 18:04:53 2019        (r346216)
+++ head/sys/sys/proc.h Mon Apr 15 01:27:15 2019        (r346217)
@@ -989,8 +989,11 @@ extern struct uma_zone *proc_zone;
 
 struct proc *pfind(pid_t);             /* Find process by id. */
 struct proc *pfind_any(pid_t);         /* Find (zombie) process by id. */
+struct proc *pfind_any_locked(pid_t pid); /* Find process by id, locked. */
 struct pgrp *pgfind(pid_t);            /* Find process group by id. */
 struct proc *zpfind(pid_t);            /* Find zombie process by id. */
+void   pidhash_slockall(void);         /* Shared lock all pid hash lists. */
+void   pidhash_sunlockall(void);       /* Shared unlock all pid hash lists. */
 
 struct fork_req {
        int             fr_flags;


_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to