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

Ryan O'Hara rohara at redhat.com
Thu Jun 11 07:51:15 PDT 2009


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
-------------- next part --------------
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,11 +364,29 @@
 			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,
 				res_lib_lck_lockgrant_callback->header.error);
 
+			if (error == SA_AIS_OK) {
+				hdb_handle_put (&lckLockIdHandleDatabase,
+					res_lib_lck_lockgrant_callback->lock_id);
+			}
+
 			break;
 
 		case MESSAGE_RES_LCK_LOCKWAITER_CALLBACK:


More information about the Openais mailing list