[Openais] [PATCH]: openais/trunk: Check for valid lock_id before invoking saLckLockGrantCallback

Steven Dake sdake at redhat.com
Wed Jun 10 09:55:04 PDT 2009


looks good

regards
-steve

On Thu, 2009-06-11 at 09:51 -0500, Ryan O'Hara wrote:
> Yet another small change. Move the hdb_handle_put below the callback.
> 
> 
> On Thu, Jun 11, 2009 at 09:24:31AM -0500, Ryan O'Hara wrote:
> > 
> > Same patch but with a hdb_handle_put if hdb_handle_get returned OK.
> > 
> > Ryan
> > 
> > 
> > On Wed, Jun 10, 2009 at 11:57:31AM -0500, Ryan O'Hara wrote:
> > > 
> > > If we request a lock via saLckLockRequestAsync, which is granted, an
> > > saLckLockGrantCallback will be generated. The callback request is sent
> > > to the dispatcher, but it is possible to unlock this lock before the
> > > callback is invoked. The specification clearly states that this should
> > > result in saLckLockGrantCallback returning SA_AIS_ERR_NOT_EXIST.
> > > 
> > > The best way to handle this is to add a check to saLckDispatch that
> > > verifies that the lock_id is valid before invoking
> > > saLckLockGrantCallback. This add this check.
> > > 
> > > Thanks again to Honzaf for finding this bug.
> > > 
> > > Ryan
> > > 
> > 
> > > Index: include/ipc_lck.h
> > > ===================================================================
> > > --- include/ipc_lck.h	(revision 1951)
> > > +++ include/ipc_lck.h	(working copy)
> > > @@ -192,6 +192,7 @@
> > >  	coroipc_response_header_t header __attribute__((aligned(8)));
> > >  	mar_invocation_t invocation __attribute__((aligned(8)));
> > >  	mar_uint32_t lock_status __attribute__((aligned(8)));
> > > +	mar_uint64_t lock_id __attribute__((aligned(8)));
> > >  } __attribute__((aligned(8)));
> > >  
> > >  struct res_lib_lck_lockwaiter_callback {
> > > Index: services/lck.c
> > > ===================================================================
> > > --- services/lck.c	(revision 1951)
> > > +++ services/lck.c	(working copy)
> > > @@ -1674,6 +1674,8 @@
> > >  		if (resource_lock != NULL) {
> > >  			res_lib_lck_lockgrant_callback.lock_status =
> > >  				resource_lock->lock_status;
> > > +			res_lib_lck_lockgrant_callback.lock_id =
> > > +				resource_lock->lock_id;
> > >  			res_lib_lck_lockgrant_callback.invocation =
> > >  				resource_lock->invocation;
> > >  		}
> > > Index: lib/lck.c
> > > ===================================================================
> > > --- lib/lck.c	(revision 1951)
> > > +++ lib/lck.c	(working copy)
> > > @@ -288,6 +288,7 @@
> > >  	SaDispatchFlagsT dispatchFlags)
> > >  {
> > >  	struct lckInstance *lckInstance;
> > > +	struct lckLockIdInstance *lckLockIdInstance;
> > >  	struct res_lib_lck_resourceopen_callback *res_lib_lck_resourceopen_callback;
> > >  	struct res_lib_lck_lockgrant_callback *res_lib_lck_lockgrant_callback;
> > >  	struct res_lib_lck_lockwaiter_callback *res_lib_lck_lockwaiter_callback;
> > > @@ -363,6 +364,19 @@
> > >  			res_lib_lck_lockgrant_callback =
> > >  				(struct res_lib_lck_lockgrant_callback *)dispatch_data;
> > >  
> > > +			/*
> > > +			 * Check that the lock_id is still valid before invoking
> > > +			 * the callback. This is needed to handle the case where
> > > +			 * a lock was unlocked before the callback was processed.
> > > +			 */
> > > +			error = hdb_error_to_sa (hdb_handle_get (&lckLockIdHandleDatabase,
> > > +				res_lib_lck_lockgrant_callback->lock_id,
> > > +				(void *)&lckLockIdInstance));
> > > +			if (error != SA_AIS_OK) {
> > > +				res_lib_lck_lockgrant_callback->header.error =
> > > +					SA_AIS_ERR_NOT_EXIST;
> > > +			}
> > > +
> > >  			callbacks.saLckLockGrantCallback (
> > >  				res_lib_lck_lockgrant_callback->invocation,
> > >  				res_lib_lck_lockgrant_callback->lock_status,
> > 
> > > _______________________________________________
> > > Openais mailing list
> > > Openais at lists.linux-foundation.org
> > > https://lists.linux-foundation.org/mailman/listinfo/openais
> 
> > Index: include/ipc_lck.h
> > ===================================================================
> > --- include/ipc_lck.h	(revision 1951)
> > +++ include/ipc_lck.h	(working copy)
> > @@ -192,6 +192,7 @@
> >  	coroipc_response_header_t header __attribute__((aligned(8)));
> >  	mar_invocation_t invocation __attribute__((aligned(8)));
> >  	mar_uint32_t lock_status __attribute__((aligned(8)));
> > +	mar_uint64_t lock_id __attribute__((aligned(8)));
> >  } __attribute__((aligned(8)));
> >  
> >  struct res_lib_lck_lockwaiter_callback {
> > Index: services/lck.c
> > ===================================================================
> > --- services/lck.c	(revision 1951)
> > +++ services/lck.c	(working copy)
> > @@ -1674,6 +1674,8 @@
> >  		if (resource_lock != NULL) {
> >  			res_lib_lck_lockgrant_callback.lock_status =
> >  				resource_lock->lock_status;
> > +			res_lib_lck_lockgrant_callback.lock_id =
> > +				resource_lock->lock_id;
> >  			res_lib_lck_lockgrant_callback.invocation =
> >  				resource_lock->invocation;
> >  		}
> > Index: lib/lck.c
> > ===================================================================
> > --- lib/lck.c	(revision 1951)
> > +++ lib/lck.c	(working copy)
> > @@ -288,6 +288,7 @@
> >  	SaDispatchFlagsT dispatchFlags)
> >  {
> >  	struct lckInstance *lckInstance;
> > +	struct lckLockIdInstance *lckLockIdInstance;
> >  	struct res_lib_lck_resourceopen_callback *res_lib_lck_resourceopen_callback;
> >  	struct res_lib_lck_lockgrant_callback *res_lib_lck_lockgrant_callback;
> >  	struct res_lib_lck_lockwaiter_callback *res_lib_lck_lockwaiter_callback;
> > @@ -363,6 +364,23 @@
> >  			res_lib_lck_lockgrant_callback =
> >  				(struct res_lib_lck_lockgrant_callback *)dispatch_data;
> >  
> > +			/*
> > +			 * Check that the lock_id is still valid before invoking
> > +			 * the callback. This is needed to handle the case where
> > +			 * a lock was unlocked before the callback was processed.
> > +			 */
> > +			error = hdb_error_to_sa (hdb_handle_get (&lckLockIdHandleDatabase,
> > +				res_lib_lck_lockgrant_callback->lock_id,
> > +				(void *)&lckLockIdInstance));
> > +			if (error != SA_AIS_OK) {
> > +				res_lib_lck_lockgrant_callback->header.error =
> > +					SA_AIS_ERR_NOT_EXIST;
> > +			}
> > +			else {
> > +				hdb_handle_put (&lckLockIdHandleDatabase,
> > +					res_lib_lck_lockgrant_callback->lock_id);
> > +			}
> > +
> >  			callbacks.saLckLockGrantCallback (
> >  				res_lib_lck_lockgrant_callback->invocation,
> >  				res_lib_lck_lockgrant_callback->lock_status,
> 
> > _______________________________________________
> > Openais mailing list
> > Openais at lists.linux-foundation.org
> > https://lists.linux-foundation.org/mailman/listinfo/openais
> _______________________________________________
> Openais mailing list
> Openais at lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/openais



More information about the Openais mailing list