VFS: fix deadlock when out of worker threads

There is a deadlock vulnerability when there are no worker threads
available and all of them blocked on a worker thread that's waiting for a
reply from a driver or a reply from an FS that needs to make a back call. In
these cases the deadlock resolver thread should kick in, but didn't in all
cases. Moreover, POSIX calls from File Servers weren't handled properly
anymore, which also could lead to deadlocks.

diff --git a/servers/vfs/dmap.c b/servers/vfs/dmap.c
index deccc8d361464a0821549a0a4207a02191b198d2..887c67291b90d046a38dcc087951e49f03bfccb4 100644 (file)
--- a/servers/vfs/dmap.c
+++ b/servers/vfs/dmap.c
@@ -79,6 +79,7 @@ int do_mapdriver()
   vir_bytes label_vir;
   size_t label_len;
   char label[LABEL_MAX];
+  struct fproc *rfp;
 
   /* Only RS can map drivers. */
   if (who_e != RS_PROC_NR) return(EPERM);
@@ -108,6 +109,10 @@ int do_mapdriver()
        return(EINVAL);
   }
 
+  /* Process is a service */
+  rfp = &fproc[_ENDPOINT_P(endpoint)];
+  rfp->fp_flags |= FP_SRV_PROC;
+
   /* Try to update device mapping. */
   return map_driver(label, major, endpoint, style, flags);
 }
@@ -129,7 +134,6 @@ int flags;                  /* device flags */
   int slot, s;
   size_t len;
   struct dmap *dp;
-  struct fproc *rfp;
 
   /* Get pointer to device entry in the dmap table. */
   if (major < 0 || major >= NR_DEVICES) return(ENODEV);
@@ -159,9 +163,6 @@ int flags;                  /* device flags */
        /* This is not a problem only when we force this driver mapping */
        if (! (flags & DRV_FORCED))
                return(EINVAL);
-  } else {
-       rfp = &fproc[slot];
-       rfp->fp_flags |= FP_SYS_PROC;   /* Process is a driver */
   }
 
   if (label != NULL) {
@@ -228,16 +229,21 @@ void dmap_unmap_by_endpt(endpoint_t proc_e)
 }
 
 /*===========================================================================*
- *                            map_service                                   *
+ *                            map_service                                   *
  *===========================================================================*/
 int map_service(struct rprocpub *rpub)
 {
 /* Map a new service by storing its device driver properties. */
   int r;
   struct dmap *fdp, *sdp;
+  struct fproc *rfp;
+
+  /* Process is a service */
+  rfp = &fproc[_ENDPOINT_P(rpub->endpoint)];
+  rfp->fp_flags |= FP_SRV_PROC;
 
   /* Not a driver, nothing more to do. */
-  if(rpub->dev_nr == NO_DEV) return(OK);
+  if (rpub->dev_nr == NO_DEV) return(OK);
 
   /* Map driver. */
   r = map_driver(rpub->label, rpub->dev_nr, rpub->endpoint, rpub->dev_style,

diff --git a/servers/vfs/fproc.h b/servers/vfs/fproc.h
index 49fa2f890e86151b94cfcb362a16b0b580fa7bcd..9ef7acbd03ebe62accbce25b23124fff2229aab3 100644 (file)
--- a/servers/vfs/fproc.h
+++ b/servers/vfs/fproc.h
@@ -62,7 +62,7 @@ EXTERN struct fproc {
 #define FP_PENDING      0010   /* Set if process has pending work */
 #define FP_EXITING      0020   /* Set if process is exiting */
 #define FP_PM_PENDING   0040   /* Set if process has pending PM request */
-#define FP_SYS_PROC     0100   /* Set if process is a driver or FS */
+#define FP_SRV_PROC     0100   /* Set if process is a service */
 #define FP_DROP_WORK    0200   /* Set if process won't accept new work */
 
 /* Field values. */

diff --git a/servers/vfs/main.c b/servers/vfs/main.c
index 3e4eb3dad1c1ec72ad5828b5f1ad8344ca779aae..adece2af84f879f636b6ecf030cbf8571e92587b 100644 (file)
--- a/servers/vfs/main.c
+++ b/servers/vfs/main.c
@@ -42,6 +42,7 @@ EXTERN unsigned long calls_stats[NCALLS];
 /* Thread related prototypes */
 static void *do_async_dev_result(void *arg);
 static void *do_control_msgs(void *arg);
+static void *do_dev_event(void *arg);
 static void *do_fs_reply(struct job *job);
 static void *do_work(void *arg);
 static void *do_pm(void *arg);
@@ -110,7 +111,9 @@ int main(void)
        } else if (is_notify(call_nr)) {
                /* A task notify()ed us */
                if (who_e == DS_PROC_NR)
-                       worker_start(ds_event);
+                       handle_work(ds_event);
+               else if (fp != NULL && (fp->fp_flags & FP_SRV_PROC))
+                       handle_work(do_dev_event);
                else
                        sys_worker_start(do_control_msgs);
                continue;
@@ -170,37 +173,37 @@ static void handle_work(void *(*func)(void *arg))
 
   proc_e = m_in.m_source;
 
-  if (fp->fp_flags & FP_SYS_PROC) {
+  if (fp->fp_flags & FP_SRV_PROC) {
+       vmp = find_vmnt(proc_e);
+       if (vmp != NULL) {
+               /* A call back or dev result from an FS
+                * endpoint. Set call back flag. Can do only
+                * one call back at a time.
+                */
+               if (vmp->m_flags & VMNT_CALLBACK) {
+                       reply(proc_e, EAGAIN);
+                       return;
+               }
+               vmp->m_flags |= VMNT_CALLBACK;
+               if (vmp->m_flags & VMNT_MOUNTING) {
+                       vmp->m_flags |= VMNT_FORCEROOTBSF;
+               }
+       }
+
        if (worker_available() == 0) {
                if (!deadlock_resolving) {
-                       if ((vmp = find_vmnt(proc_e)) != NULL) {
-                               /* A call back or dev result from an FS
-                                * endpoint. Set call back flag. Can do only
-                                * one call back at a time.
-                                */
-                               if (vmp->m_flags & VMNT_CALLBACK) {
-                                       reply(proc_e, EAGAIN);
-                                       return;
-                               }
-                               vmp->m_flags |= VMNT_CALLBACK;
-
-                               /* When an FS endpoint has to make a call back
-                                * in order to mount, force its device to a
-                                * "none device" so block reads/writes will be
-                                * handled by ROOT_FS_E.
-                                */
-                               if (vmp->m_flags & VMNT_MOUNTING)
-                                       vmp->m_flags |= VMNT_FORCEROOTBSF;
-                       }
                        deadlock_resolving = 1;
                        dl_worker_start(func);
                        return;
                }
-               /* Already trying to resolve a deadlock, can't
-                * handle more, sorry */
 
-               reply(proc_e, EAGAIN);
-               return;
+               if (vmp != NULL) {
+                       /* Already trying to resolve a deadlock, can't
+                        * handle more, sorry */
+
+                       reply(proc_e, EAGAIN);
+                       return;
+               }
        }
   }
 
@@ -243,19 +246,7 @@ static void *do_async_dev_result(void *arg)
        select_reply2(job_m_in.m_source, job_m_in.DEV_MINOR,
                      job_m_in.DEV_SEL_OPS);
 
-  if (deadlock_resolving) {
-       if (fp != NULL && fp->fp_wtid == dl_worker.w_tid)
-               deadlock_resolving = 0;
-  }
-
-  if (fp != NULL && (fp->fp_flags & FP_SYS_PROC)) {
-       struct vmnt *vmp;
-
-       if ((vmp = find_vmnt(fp->fp_endpoint)) != NULL)
-               vmp->m_flags &= ~VMNT_CALLBACK;
-  }
-
-  thread_cleanup(NULL);
+  thread_cleanup(fp);
   return(NULL);
 }
 
@@ -273,15 +264,29 @@ static void *do_control_msgs(void *arg)
   if (job_m_in.m_source == CLOCK) {
        /* Alarm timer expired. Used only for select(). Check it. */
        expire_timers(job_m_in.NOTIFY_TIMESTAMP);
-  } else {
-       /* Device notifies us of an event. */
-       dev_status(job_m_in.m_source);
   }
 
   thread_cleanup(NULL);
   return(NULL);
 }
 
+/*===========================================================================*
+ *                            do_dev_event                                  *
+ *===========================================================================*/
+static void *do_dev_event(void *arg)
+{
+/* Device notifies us of an event. */
+  struct job my_job;
+
+  my_job = *((struct job *) arg);
+  fp = my_job.j_fp;
+
+  dev_status(job_m_in.m_source);
+
+  thread_cleanup(fp);
+  return(NULL);
+}
+
 /*===========================================================================*
  *                            do_fs_reply                                   *
  *===========================================================================*/
@@ -384,8 +389,8 @@ static void *do_pending_pipe(void *arg)
        reply(fp->fp_endpoint, r);
 
   unlock_filp(f);
-
   thread_cleanup(fp);
+  unlock_proc(fp);
   return(NULL);
 }
 
@@ -402,6 +407,7 @@ void *do_dummy(void *arg)
 
   if ((r = mutex_trylock(&fp->fp_lock)) == 0) {
        thread_cleanup(fp);
+       unlock_proc(fp);
   } else {
        /* Proc is busy, let that worker thread carry out the work */
        thread_cleanup(NULL);
@@ -456,24 +462,10 @@ static void *do_work(void *arg)
   }
 
   /* Copy the results back to the user and send reply. */
-  if (error != SUSPEND) {
-
-       if ((fp->fp_flags & FP_SYS_PROC)) {
-               struct vmnt *vmp;
-
-               if ((vmp = find_vmnt(fp->fp_endpoint)) != NULL)
-                       vmp->m_flags &= ~VMNT_CALLBACK;
-       }
-
-       if (deadlock_resolving) {
-               if (fp->fp_wtid == dl_worker.w_tid)
-                       deadlock_resolving = 0;
-       }
-
-       reply(fp->fp_endpoint, error);
-  }
+  if (error != SUSPEND) reply(fp->fp_endpoint, error);
 
   thread_cleanup(fp);
+  unlock_proc(fp);
   return(NULL);
 }
 
@@ -635,6 +627,7 @@ static void *do_init_root(void *arg)
 
   unlock_pm();
   thread_cleanup(fp);
+  unlock_proc(fp);
   return(NULL);
 }
 
@@ -710,7 +703,18 @@ void thread_cleanup(struct fproc *rfp)
 
   if (rfp != NULL) {
        rfp->fp_flags &= ~FP_DROP_WORK;
-       unlock_proc(rfp);
+       if (rfp->fp_flags & FP_SRV_PROC) {
+               struct vmnt *vmp;
+
+               if ((vmp = find_vmnt(rfp->fp_endpoint)) != NULL) {
+                       vmp->m_flags &= ~VMNT_CALLBACK;
+               }
+       }
+  }
+
+  if (deadlock_resolving) {
+       if (self->w_tid == dl_worker.w_tid)
+               deadlock_resolving = 0;
   }
 }
 

diff --git a/servers/vfs/mount.c b/servers/vfs/mount.c
index 217dba891622f761f3ac1243097a702534eef0e6..7956c24fb240256b892a955eec5efda47c24743c 100644 (file)
--- a/servers/vfs/mount.c
+++ b/servers/vfs/mount.c
@@ -266,7 +266,7 @@ char mount_label[LABEL_MAX] )
        return(EINVAL);
   }
   rfp = &fproc[slot];
-  rfp->fp_flags |= FP_SYS_PROC;        /* Process is an FS */
+  rfp->fp_flags |= FP_SRV_PROC;        /* File Servers are also services */
 
   /* Store some essential vmnt data first */
   new_vmp->m_fs_e = fs_e;
@@ -407,7 +407,6 @@ void mount_pfs(void)
   strlcpy(vmp->m_label, "pfs", LABEL_MAX);
 
   rfp = &fproc[_ENDPOINT_P(PFS_PROC_NR)];
-  rfp->fp_flags |= FP_SYS_PROC;        /* PFS is a driver and an FS */
 }
 
 /*===========================================================================*