From 206d6d9271504cad16326bcb717146ea5b3eed35 Mon Sep 17 00:00:00 2001 From: Simon Wilkinson Date: Wed, 27 Feb 2013 10:11:21 +0000 Subject: [PATCH] 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 Tested-by: BuildBot Reviewed-by: Jeffrey Altman (cherry picked from commit ce20f1f15103226667bc872378cf9b2e4b3e8cd7) Change-Id: Id89df6302002224ec2f871f18711e781990f73d3 Reviewed-on: http://gerrit.openafs.org/11024 Reviewed-by: Chas Williams - CONTRACTOR Tested-by: BuildBot Reviewed-by: Andrew Deason Reviewed-by: Stephan Wiesand --- src/libafscp/afscp_fid.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) 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; } -- 2.39.5