VFS: better dupfrom(2) deadlock detection

Change-Id: I29f00075698888c7c8ca60b47ab82fba8c606f4e

diff --git a/servers/vfs/device.c b/servers/vfs/device.c
index 1e446fe1fd256cb8cee24b853568a02b47eeccc3..828048968abfc7e1c2fde62fbe45343dc6e60d00 100644 (file)
--- a/servers/vfs/device.c
+++ b/servers/vfs/device.c
@@ -560,9 +560,13 @@ int do_ioctl(message *UNUSED(m_out))
   if (r == OK) {
        dev = (dev_t) vp->v_sdev;
 
-       if (S_ISBLK(vp->v_mode))
+       if (S_ISBLK(vp->v_mode)) {
+               f->filp_ioctl_fp = fp;
+
                r = bdev_ioctl(dev, who_e, ioctlrequest, argx);
-       else
+
+               f->filp_ioctl_fp = NULL;
+       } else
                r = cdev_io(CDEV_IOCTL, dev, who_e, argx, 0, ioctlrequest,
                        f->filp_flags);
   }

diff --git a/servers/vfs/file.h b/servers/vfs/file.h
index 1bba747ab7d3778c348f30e50aa632cb6fedc6a1..efbe80d97584ba3f3dde7c48e883862fb24b8f11 100644 (file)
--- a/servers/vfs/file.h
+++ b/servers/vfs/file.h
@@ -15,6 +15,9 @@ EXTERN struct filp {
   struct fproc *filp_softlock; /* if not NULL; this filp didn't lock the
                                 * vnode. Another filp already holds a lock
                                 * for this thread */
+  struct fproc *filp_ioctl_fp; /* if not NULL, this filp is locked by the
+                                * process for a currently ongoing IOCTL call
+                                */
 
   /* the following fields are for select() and are owned by the generic
    * select() code (i.e., fd-type-specific select() code can't touch these).

diff --git a/servers/vfs/filedes.c b/servers/vfs/filedes.c
index 79f2d3ffffb90b70a0bb5dadb2beb7888fbb05a9..4387b209b59ee4df07516ef8ffa6a3f848eafddd 100644 (file)
--- a/servers/vfs/filedes.c
+++ b/servers/vfs/filedes.c
@@ -131,6 +131,7 @@ int get_fd(struct fproc *rfp, int start, mode_t bits, int *k, struct filp **fpt)
                f->filp_flags = 0;
                f->filp_select_flags = 0;
                f->filp_softlock = NULL;
+               f->filp_ioctl_fp = NULL;
                *fpt = f;
                return(OK);
        }
@@ -628,7 +629,6 @@ int do_dupfrom(message *UNUSED(m_out))
  */
   struct fproc *rfp;
   struct filp *rfilp;
-  struct vnode *vp;
   endpoint_t endpt;
   int r, fd, slot;
 
@@ -647,19 +647,15 @@ int do_dupfrom(message *UNUSED(m_out))
   if ((rfilp = get_filp2(rfp, fd, VNODE_NONE)) == NULL)
        return(err_code);
 
-  /* For now, we do not allow remote duplication of device nodes.  In practice,
-   * only a block-special file can cause a deadlock for the caller (currently
-   * only the VND driver).  This would happen if a user process passes in the
-   * file descriptor to the device node on which it is performing the IOCTL.
-   * This would cause two VFS threads to deadlock on the same filp.  Since the
-   * VND driver does not allow device nodes to be used anyway, this somewhat
-   * rudimentary check eliminates such deadlocks.  A better solution would be
-   * to check if the given endpoint holds a lock to the target filp, but we
-   * currently do not have this information within VFS.
+  /* If the filp is involved in an IOCTL by the user process, locking the filp
+   * here would result in a deadlock.  This would happen if a user process
+   * passes in the file descriptor to the device node on which it is performing
+   * the IOCTL.  We do not allow manipulation of such device nodes.  In
+   * practice, this only applies to block-special files (and thus VND), because
+   * character-special files (as used by UDS) are unlocked during the IOCTL.
    */
-  vp = rfilp->filp_vno;
-  if (S_ISCHR(vp->v_mode) || S_ISBLK(vp->v_mode))
-       return(EINVAL);
+  if (rfilp->filp_ioctl_fp == rfp)
+       return(EBADF);
 
   /* Now we can safely lock the filp, copy it, and unlock it again. */
   lock_filp(rfilp, VNODE_READ);

diff --git a/test/tvnd.c b/test/tvnd.c
index 68b76b26d5d2c470ae74357dc86dfb9b5724b5be..0f6c8333153e952a25af869e87dd6e72755a43a2 100644 (file)
--- a/test/tvnd.c
+++ b/test/tvnd.c
@@ -42,7 +42,7 @@ main(int argc, char **argv)
                return EXIT_FAILURE;
        }
 
-       if (errno != EINVAL) {
+       if (errno != EBADF) {
                perror("ioctl(VNDIOCSET)");
                return EXIT_FAILURE;
        }