don't release Volume lightweight ref too early
authorTom Keiser <tkeiser@sinenomine.net>
Wed, 13 Oct 2010 05:10:09 +0000 (01:10 -0400)
committerDerrick Brashear <shadow@dementia.org>
Thu, 14 Oct 2010 04:03:36 +0000 (21:03 -0700)
FSYNC_com_VolOff was releasing its lightweight ref before the error handling
code for VGetVolumeByVp_r was executed; this code needs to dereference the
Volume pointer for some of its logic.  This was unsafe since
VCancelReservation_r() could have resulted in the Volume object being freed.
Move VCancelReservation_r() below the error handling block.  NB: the error
handling block now relies upon the goto done/deny to cancel its lightweight
ref.

Change-Id: Ic920e45d55f7d1773585a5c0b004c722355b5725
Reviewed-on: http://gerrit.openafs.org/2968
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Derrick Brashear <shadow@dementia.org>

src/vol/fssync-server.c

index 98f30c7..4dfbb6f 100644 (file)
@@ -976,9 +976,6 @@ FSYNC_com_VolOff(FSSYNC_VolOp_command * vcom, SYNC_response * res)
 
        /* convert to heavyweight ref */
        nvp = VGetVolumeByVp_r(&error, vp);
-       VCancelReservation_r(rvp);
-       rvp = NULL;
-
        if (!nvp) {
             /*
              * It's possible for VGetVolumeByVp_r to have dropped and
@@ -1014,6 +1011,10 @@ FSYNC_com_VolOff(FSSYNC_VolOp_command * vcom, SYNC_response * res)
            vp = nvp;
        }
 
+       /* kill off lightweight ref to ensure we can't deadlock against ourselves later... */
+       VCancelReservation_r(rvp);
+       rvp = NULL;
+
        /* register the volume operation metadata with the volume */
        VRegisterVolOp_r(vp, &info);