From: David van Moolenbroek <david@minix3.org>
Date: Thu, 4 Aug 2016 16:17:09 +0000 (+0000)
Subject: VFS: fix race condition in select(2)
X-Git-Url: http://zhaoyanbai.com/repos/%22http:/www.isc.org/icons/readme1st.txt?a=commitdiff_plain;h=2ff64318e25700b5f55a95b9feecb542e1653175;p=minix.git

VFS: fix race condition in select(2)

A select query could deadlock if..

- it was querying a character or socket device that, at the start of
  the select query, was not known to be ready for the requested
  operations;
- this device could not be checked immediately, due to another ongoing
  query to the same character or socket driver;
- the select query had a timer that triggered before the device could
  be checked, thereby changing the select query to non-blocking.

In this situation, a missing flag check would cause the select code to
conclude erroneously that the operations which it flagged for later,
were satisfied.  At the same time, the same flag remained set, so that
the select query would continue to wait for that device.  This
resulted in a deadlock.  The same bug could most likely be triggered
through other scenarios that were even less likely to occur.

This patch fixes the race condition and puts in a hopefully slightly
more informative comment for the affected block of code.

In practice, the bug could be triggered fairly reliably by generating
lots of output in tmux.

Change-Id: I1c909255dcf552e6c7cef08b0cf5cbc41294b99c
---

diff --git a/minix/servers/vfs/select.c b/minix/servers/vfs/select.c
index 5b834b231..ac6e34044 100644
--- a/minix/servers/vfs/select.c
+++ b/minix/servers/vfs/select.c
@@ -412,20 +412,27 @@ static int select_request_char(struct filp *f, int *ops, int block,
   /* By default, nothing to do */
   *ops = 0;
 
-  if (!block && (f->filp_select_flags & FSF_BLOCKED)) {
-	/* This filp is blocked waiting for a reply, but we don't want to
-	 * block ourselves. Unless we're awaiting the initial reply, these
-	 * operations won't be ready */
-	if (!(f->filp_select_flags & FSF_BUSY)) {
-		if ((rops & SEL_RD) && (f->filp_select_flags & FSF_RD_BLOCK))
-			rops &= ~SEL_RD;
-		if ((rops & SEL_WR) && (f->filp_select_flags & FSF_WR_BLOCK))
-			rops &= ~SEL_WR;
-		if ((rops & SEL_ERR) && (f->filp_select_flags & FSF_ERR_BLOCK))
-			rops &= ~SEL_ERR;
-		if (!(rops & (SEL_RD|SEL_WR|SEL_ERR)))
-			return(OK);
-	}
+  /*
+   * If we have previously asked the driver to notify us about certain ready
+   * operations, but it has not notified us yet, then we can safely assume that
+   * those operations are not ready right now.  Therefore, if this call is not
+   * supposed to block, we can disregard the pending operations as not ready.
+   * We must make absolutely sure that the flags are "stable" right now though:
+   * we are neither waiting to query the driver about them (FSF_UPDATE) nor
+   * querying the driver about them right now (FSF_BUSY).  This is a dangerous
+   * case of premature optimization and may be removed altogether if it proves
+   * to continue to be a source of bugs.
+   */
+  if (!block && !(f->filp_select_flags & (FSF_UPDATE | FSF_BUSY)) &&
+      (f->filp_select_flags & FSF_BLOCKED)) {
+	if ((rops & SEL_RD) && (f->filp_select_flags & FSF_RD_BLOCK))
+		rops &= ~SEL_RD;
+	if ((rops & SEL_WR) && (f->filp_select_flags & FSF_WR_BLOCK))
+		rops &= ~SEL_WR;
+	if ((rops & SEL_ERR) && (f->filp_select_flags & FSF_ERR_BLOCK))
+		rops &= ~SEL_ERR;
+	if (!(rops & (SEL_RD|SEL_WR|SEL_ERR)))
+		return(OK);
   }
 
   f->filp_select_flags |= FSF_UPDATE;