From 6421a4261e8c99d268f3002709275b3861b368a4 Mon Sep 17 00:00:00 2001 From: Michael Meffie Date: Mon, 9 Nov 2009 16:28:33 -0500 Subject: [PATCH] cm: address race condition in afs_QueueVCB Access the vcache callback member after taking the xvcb lock to avoid the server object from being freed in FlushServer on another thread. Eventually, we should have a ref count on avc->server. FIXES 125596 Change-Id: I9328a0b3e372353b7812d6e9f2cbea44655a3d58 Reviewed-on: http://gerrit.openafs.org/817 Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- src/afs/afs_vcache.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/afs/afs_vcache.c b/src/afs/afs_vcache.c index 47ec45be9..66032c967 100644 --- a/src/afs/afs_vcache.c +++ b/src/afs/afs_vcache.c @@ -210,14 +210,7 @@ afs_FlushVCache(struct vcache *avc, int *slept) vn_reinit(AFSTOV(avc)); #endif afs_FreeAllAxs(&(avc->Access)); - - /* we can't really give back callbacks on RO files, since the - * server only tracks them on a per-volume basis, and we don't - * know whether we still have some other files from the same - * volume. */ - if ((avc->states & CRO) == 0 && avc->callback) { - afs_QueueVCB(avc); - } + afs_QueueVCB(avc); ObtainWriteLock(&afs_xcbhash, 460); afs_DequeueCallback(avc); /* remove it from queued callbacks list */ avc->states &= ~(CStatd | CUnique); @@ -507,22 +500,36 @@ afs_FlushVCBs(afs_int32 lockit) * Environment: * Locks the xvcb lock. * Called when the xvcache lock is already held. + * + * \param avc vcache entry + * \return 1 if queued, 0 otherwise */ static afs_int32 afs_QueueVCB(struct vcache *avc) { + int queued = 0; struct server *tsp; struct afs_cbr *tcbp; AFS_STATCNT(afs_QueueVCB); + + MObtainWriteLock(&afs_xvcb, 274); + + /* we can't really give back callbacks on RO files, since the + * server only tracks them on a per-volume basis, and we don't + * know whether we still have some other files from the same + * volume. */ + if (!((avc->states & CRO) == 0 && avc->callback)) { + goto done; + } + /* The callback is really just a struct server ptr. */ tsp = (struct server *)(avc->callback); /* we now have a pointer to the server, so we just allocate * a queue entry and queue it. */ - MObtainWriteLock(&afs_xvcb, 274); tcbp = afs_AllocCBR(); tcbp->fid = avc->fid.Fid; @@ -534,10 +541,12 @@ afs_QueueVCB(struct vcache *avc) tcbp->pprev = &tsp->cbrs; afs_InsertHashCBR(tcbp); + queued = 1; + done: /* now release locks and return */ MReleaseWriteLock(&afs_xvcb); - return 0; + return queued; } -- 2.39.5