From 50685cbec34c4010c01a9edd5cc3344519aaac55 Mon Sep 17 00:00:00 2001
From: David van Moolenbroek <david@minix3.org>
Date: Sat, 5 Oct 2013 12:50:44 +0200
Subject: [PATCH] VFS: better dupfrom(2) deadlock detection

Change-Id: I29f00075698888c7c8ca60b47ab82fba8c606f4e
---
 servers/vfs/device.c  |  8 ++++++--
 servers/vfs/file.h    |  3 +++
 servers/vfs/filedes.c | 22 +++++++++-------------
 test/tvnd.c           |  2 +-
 4 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/servers/vfs/device.c b/servers/vfs/device.c
index 1e446fe1f..828048968 100644
--- 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 1bba747ab..efbe80d97 100644
--- 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 79f2d3fff..4387b209b 100644
--- 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 68b76b26d..0f6c83331 100644
--- 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;
 	}
-- 
2.44.0