From ef4b9e13ef6a79d7e5f540297e486189fdedf085 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Fri, 5 Nov 2010 14:34:05 -0500 Subject: [PATCH] DAFS: Do not let VScheduleSalvage_r free vp In VScheduleSalvage_r, we were calling VCancelReservation_r, which has the possibility of free'ing vp. Since we still use vp after doing this (and since we're already inside VCancelReservation_r to begin with), we must not free vp. Instead, just decrement nWaiters without triggering any of the dtor code in VCancelReservation_r. This is safe as long as all VScheduleSalvage_r callers ensure that they check to free the vp if necessary, which they all do. Thanks to Derrick Brashear. Change-Id: Iebdbdf47c6307cd7c036b88ad4dbc25bc7a3279a Reviewed-on: http://gerrit.openafs.org/3272 Tested-by: Andrew Deason Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- src/vol/volume.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/vol/volume.c b/src/vol/volume.c index 1dc1597bd6..6686bf45e9 100644 --- a/src/vol/volume.c +++ b/src/vol/volume.c @@ -4963,6 +4963,10 @@ VOfflineForSalvage_r(struct Volume *vp) * * @note this is one of the event handlers called by VCancelReservation_r * + * @note the caller must check if the volume needs to be freed after calling + * this; the volume may not have any references or be on any lists after + * we return, and we do not free it + * * @see VCancelReservation_r * * @internal volume package internal use only. @@ -5237,6 +5241,10 @@ try_FSSYNC(Volume *vp, char *partName, int *code) { * server over SALVSYNC. If we are not the fileserver, the request will be * sent to the fileserver over FSSYNC (FSYNC_VOL_FORCE_ERROR/FSYNC_SALVAGE). * + * @note the caller must check if the volume needs to be freed after calling + * this; the volume may not have any references or be on any lists after + * we return, and we do not free it + * * @note DAFS only * * @internal volume package internal use only. @@ -5329,7 +5337,14 @@ VScheduleSalvage_r(Volume * vp) } } } - VCancelReservation_r(vp); + + /* NB: this is cancelling the reservation we obtained above, but we do + * not call VCancelReservation_r, since that may trigger the vp dtor, + * possibly free'ing the vp. We need to keep the vp around after + * this, as the caller may reference vp without any refs. Instead, it + * is the duty of the caller to inspect 'vp' after we return to see if + * needs to be freed. */ + osi_Assert(--vp->nWaiters >= 0); return ret; } #endif /* SALVSYNC_BUILD_CLIENT || FSSYNC_BUILD_CLIENT */