libafscp: Can't unlock something we've freed
authorSimon Wilkinson <sxw@your-file-system.com>
Wed, 27 Feb 2013 10:11:21 +0000 (10:11 +0000)
committerJeffrey Altman <jaltman@your-file-system.com>
Wed, 27 Feb 2013 20:43:59 +0000 (12:43 -0800)
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)

Change-Id: I346d4c8a7cd478db044af919662c1cf1c093e205
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>

src/libafscp/afscp_fid.c

index c7582c9..7c3ca65 100644 (file)
@@ -144,11 +144,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;
@@ -312,9 +314,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;
 }