From: Andrew Deason <adeason@sinenomine.net>
Date: Wed, 24 Aug 2011 17:48:19 +0000 (-0500)
Subject: ihandle: Fix IH_REALLYCLOSE for positional I/O
X-Git-Tag: upstream/1.6.1.pre1^2~232
X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=f4dd94edf7384dee912a09354f500d08421f94d4;p=packages%2Fo%2Fopenafs.git

ihandle: Fix IH_REALLYCLOSE for positional I/O

Currently, ih_fdclose (which is called by IH_REALLYCLOSE), goes
through every FD_HANDLE_OPEN FdHandle_t and closes it. If it finds
handles that are FD_HANDLE_INUSE, it skips those and sets a flag on
the parent IHandle_t. For non-positional I/O, any future opens cannot
use these _INUSE handles, since _INUSE handles cannot be reused, and
the handle will be actually closed when it is FDH_CLOSE'd.

For positional I/O, the situation is different. Multiple threads can
use the same _INUSE FdHandle_t, and so there is nothing currently
stopping a thread from IH_OPEN'ing an ihandle that has been
IH_REALLYCLOSE'd, and getting back an FdHandle_t that existed before
the IH_REALLYCLOSE was issued. This is important, since IH_REALLYCLOSE
is used on files that are deleted, and future IH_OPENs for the same
inode must not use the cached file descriptor. Getting this wrong can
cause data loss, since it can cause us to read from or write to a file
descriptor referring to a deleted file, when we instead should open a
new copy of that file.

To fix this, we create a new FdHandle_t state called
FD_HANDLE_CLOSING, which is set in IH_REALLYCLOSE if we encounter an
FD_HANDLE_INUSE FdHandle_t. In IH_OPEN, we always skip
FD_HANDLE_CLOSING handles, so we can never get back a cached file
descriptor from before an IH_REALLYCLOSE call.

Reviewed-on: http://gerrit.openafs.org/5308
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
(cherry picked from commit 597de25969ebdeaafb7390984b5ce2c8782fd557)

Change-Id: I4ac2e4d10ce20f8575b35385e324b637dffd0671
Reviewed-on: http://gerrit.openafs.org/5404
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
---

diff --git a/src/vol/ihandle.c b/src/vol/ihandle.c
index b57929d31..6bfa80152 100644
--- a/src/vol/ihandle.c
+++ b/src/vol/ihandle.c
@@ -343,6 +343,11 @@ ih_open(IHandle_t * ihP)
 
     /* Do we already have an open file handle for this Inode? */
     for (fdP = ihP->ih_fdtail; fdP != NULL; fdP = fdP->fd_ihprev) {
+	if (fdP->fd_status == FD_HANDLE_CLOSING) {
+	    /* The handle was open when an IH_REALLYCLOSE was issued, so we
+	     * cannot reuse it; it will be closed soon. */
+	    continue;
+	}
 #ifndef HAVE_PIO
 	/*
 	 * If we don't have positional i/o, don't try to share fds, since
@@ -445,7 +450,8 @@ fd_close(FdHandle_t * fdP)
     IH_LOCK;
     osi_Assert(ih_Inited);
     osi_Assert(fdInUseCount > 0);
-    osi_Assert(fdP->fd_status == FD_HANDLE_INUSE);
+    osi_Assert(fdP->fd_status == FD_HANDLE_INUSE ||
+               fdP->fd_status == FD_HANDLE_CLOSING);
 
     ihP = fdP->fd_ih;
 
@@ -454,7 +460,8 @@ fd_close(FdHandle_t * fdP)
      * failed (this is determined by checking the ihandle for the flag
      * IH_REALLY_CLOSED) or we have too many open files.
      */
-    if (ihP->ih_flags & IH_REALLY_CLOSED || fdInUseCount > fdCacheSize) {
+    if (fdP->fd_status == FD_HANDLE_CLOSING ||
+        ihP->ih_flags & IH_REALLY_CLOSED || fdInUseCount > fdCacheSize) {
 	IH_UNLOCK;
 	return fd_reallyclose(fdP);
     }
@@ -496,7 +503,8 @@ fd_reallyclose(FdHandle_t * fdP)
     IH_LOCK;
     osi_Assert(ih_Inited);
     osi_Assert(fdInUseCount > 0);
-    osi_Assert(fdP->fd_status == FD_HANDLE_INUSE);
+    osi_Assert(fdP->fd_status == FD_HANDLE_INUSE ||
+               fdP->fd_status == FD_HANDLE_CLOSING);
 
     ihP = fdP->fd_ih;
     closeFd = fdP->fd_fd;
@@ -801,7 +809,8 @@ ih_fdclose(IHandle_t * ihP)
 	next = fdP->fd_ihnext;
 	osi_Assert(fdP->fd_ih == ihP);
 	osi_Assert(fdP->fd_status == FD_HANDLE_OPEN
-	       || fdP->fd_status == FD_HANDLE_INUSE);
+	       || fdP->fd_status == FD_HANDLE_INUSE
+	       || fdP->fd_status == FD_HANDLE_CLOSING);
 	if (fdP->fd_status == FD_HANDLE_OPEN) {
 	    DLL_DELETE(fdP, ihP->ih_fdhead, ihP->ih_fdtail, fd_ihnext,
 		       fd_ihprev);
@@ -809,6 +818,7 @@ ih_fdclose(IHandle_t * ihP)
 	    DLL_INSERT_TAIL(fdP, head, tail, fd_next, fd_prev);
 	} else {
 	    closedAll = 0;
+	    fdP->fd_status = FD_HANDLE_CLOSING;
 	    ihP->ih_flags |= IH_REALLY_CLOSED;
 	}
     }
diff --git a/src/vol/ihandle.h b/src/vol/ihandle.h
index 3f863cd2e..3e093c42e 100644
--- a/src/vol/ihandle.h
+++ b/src/vol/ihandle.h
@@ -171,6 +171,9 @@ typedef struct FdHandle_s {
 #define FD_HANDLE_AVAIL		1	/* handle is not open and available */
 #define FD_HANDLE_OPEN		2	/* handle is open and not in use */
 #define FD_HANDLE_INUSE		3	/* handle is open and in use */
+#define FD_HANDLE_CLOSING	4	/* handle is open, in use, and has been
+					 * IH_REALLYCLOSE'd. It should not be
+					 * used for subsequent opens. */
 
 /* buffered file descriptor handle */
 #define STREAM_HANDLE_BUFSIZE	2048	/* buffer size for STR_READ/STR_WRITE */