From: Simon Wilkinson <sxw@your-file-system.com>
Date: Wed, 27 Feb 2013 10:11:21 +0000 (+0000)
Subject: libafscp: Can't unlock something we've freed
X-Git-Tag: upstream/1.6.10_pre1^2~165
X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=206d6d9271504cad16326bcb717146ea5b3eed35;p=packages%2Fo%2Fopenafs.git

libafscp: Can't unlock something we've freed

When we call _StatCleanup on a stored statent structure, it
deletes the mutex, and frees the structure itself. This means it
can't be called with a locked structure as the mutex deletion
will fail, and then we'll try to reference freed memory when we
later unlock that mutex.

Fix this by unlocking the mutex before calling _StatCleanup. This
is safe because the only reference to the structure visible to other
threads must have been deleted by the time we reach this point.

Caught by coverity (#986058, #986059)

Reviewed-on: http://gerrit.openafs.org/9297
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit ce20f1f15103226667bc872378cf9b2e4b3e8cd7)

Change-Id: Id89df6302002224ec2f871f18711e781990f73d3
Reviewed-on: http://gerrit.openafs.org/11024
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
---

diff --git a/src/libafscp/afscp_fid.c b/src/libafscp/afscp_fid.c
index 69e38b164..2aad96267 100644
--- a/src/libafscp/afscp_fid.c
+++ b/src/libafscp/afscp_fid.c
@@ -146,11 +146,13 @@ afscp_WaitForCallback(const struct afscp_venusfid *fid, int seconds)
 	    code = pthread_cond_timedwait(&(stored->cv), &(stored->mtx), &ts);
 	else
 	    pthread_cond_wait(&(stored->cv), &(stored->mtx));
-	if ((stored->nwaiters == 1) && stored->cleanup)
+	if ((stored->nwaiters == 1) && stored->cleanup) {
+	    pthread_mutex_unlock(&(stored->mtx));
 	    _StatCleanup(stored);
-	else
+	} else {
 	    stored->nwaiters--;
-        pthread_mutex_unlock(&(stored->mtx));
+	    pthread_mutex_unlock(&(stored->mtx));
+	}
     }
     if ((code == EINTR) || (code == ETIMEDOUT)) {
 	afscp_errno = code;
@@ -314,9 +316,11 @@ _StatInvalidate(const struct afscp_venusfid *fid)
 	    /* avoid blocking callback thread */
 	    pthread_cond_broadcast(&(stored->cv));
 	    stored->cleanup = 1;
-	} else
+	    pthread_mutex_unlock(&(stored->mtx));
+	} else {
+	    pthread_mutex_unlock(&(stored->mtx));
 	    _StatCleanup(stored);
-	pthread_mutex_unlock(&(stored->mtx));
+	}
     }
     return 0;
 }