RE: More PTHREAD_PROCESS_SHARED(Storing pointers into shared memory?)

I submit this patch to you to show what I did to fix the problem.  I
hesitate to recommend the patch, because it creates invalid pointers that
must be checked before they are used.  Basically, when allocating a shared
mutex, return array index of the mutex + 1(so you can still have NULL
pointers).  Then, before actually de-referencing the pointer, check to see
if the mutex pointer is really an index and fix the pointer.  I have been
running this patch for a couple of weeks without any problems...

Thanks for the reply,
Chris McFarlen
T-NETIX, Inc.

diff -urNp ngpt-2.2.0/pth_sync.c ngpt-2.2.0-my/pth_sync.c
--- ngpt-2.2.0/pth_sync.c	2002-12-03 10:21:56.000000000 -0600
+++ ngpt-2.2.0-my/pth_sync.c	2003-01-16 13:32:57.000000000 -0600
@@ -82,6 +82,24 @@ intern int		    pth_shared_fd = 0;
 intern pth_shared_area_t    *pth_shared_area;
 intern int		    PTH_SHARED_SIZE = (PTH_MAX_SHARED_OBJECTS *
sizeof(struct pth_shared_sync_st) + sizeof(struct pth_qlock_st));
 
+pth_mutex_t *pth_fix_mutex_ptr(pth_mutex_t *mtx)
+{
+   if (((int)mtx) <= PTH_MAX_SHARED_OBJECTS) {
+      //fprintf(stderr, "fixup mutex: ptr: %p indx: %i\n",
&pth_shared_area->o[((int)mtx)-1].u.mx, mtx-1);
+      return &pth_shared_area->o[((int)mtx)-1].u.mx;
+   }
+   return mtx;
+}
+
+pth_cond_t *pth_fix_cond_ptr(pth_cond_t *cn)
+{
+   if (((int)cn) <= PTH_MAX_SHARED_OBJECTS) {
+      return &pth_shared_area->o[((int)cn)-1].u.cn;
+   }
+   return cn;
+}
+
+
 /*
 **  Mutual Exclusion Locks
 */
@@ -115,6 +133,8 @@ intern int pth_find_shared_mutex(pth_mut
 {
     int indx = 0;
 
+    mutex = pth_fix_mutex_ptr(mutex);
+
     pth_acquire_lock(&(pth_shared_area->lock));
     while (pth_shared_area->o[indx].used == TRUE) {
 	if (mutex == &pth_shared_area->o[indx].u.mx) {
@@ -152,7 +172,8 @@ intern pth_mutex_t *pth_alloc_shared_mut
 
     pth_release_lock(&(pth_shared_area->lock));
 
-    return rmx;
+    //fprintf(stderr, "alloc shared: ptr: %p index: %i\n", rmx, indx);
+    return (pth_mutex_t *)indx+1;
 }
 
 intern pth_cond_t *pth_alloc_shared_cond(void)
@@ -180,11 +201,13 @@ intern pth_cond_t *pth_alloc_shared_cond
 
     pth_release_lock(&(pth_shared_area->lock));
 
-    return rcn;
+    return (pth_cond_t *)indx+1;
 }
 
 int pth_mutex_init(pth_mutex_t *mutex, pth_mutexattr_t *pattr)
 {
+    mutex = pth_fix_mutex_ptr(mutex);
+
     int pshared = (pattr != NULL) ? pattr->pshared : FALSE;
     if (mutex == NULL)
 	return FALSE;
@@ -231,6 +254,8 @@ int pth_mutex_acquire(pth_mutex_t *mutex
     pth_event_t ev;
     int retcode = 0;
 
+    mutex = pth_fix_mutex_ptr(mutex);
+
     if (mutex->mx_state & PTH_MUTEX_INTERNAL_LOCKED) {
 	_pth_acquire_lock(&(mutex->mx_lock), 0);
 	return 0;
@@ -561,6 +586,8 @@ int pth_mutex_release(pth_mutex_t *mutex
     pth_descr_t descr = pth_get_native_descr();
     pth_list_t *mq;
 
+    mutex = pth_fix_mutex_ptr(mutex);
+
     if (!(mutex->mx_state & PTH_MUTEX_LOCKED)) {
 	if (mutex->mx_state & PTH_MUTEX_INTERNAL_LOCKED) {
 	    _pth_release_lock(&(mutex->mx_lock), 0);
@@ -643,7 +670,7 @@ intern void pth_mutex_releaseall(pth_t t
 	pth_mutex_release(thread->mutex_owned);
     rn = rnf = pth_ring_first(&(thread->mutexring));
     while (rn != NULL) {
-	m = (pth_mutex_t *)((char *)rn - (int)&((pth_mutex_t *)0)->mx_node);
+	m = pth_fix_mutex_ptr((pth_mutex_t *)((char *)rn -
(int)&((pth_mutex_t *)0)->mx_node));
         pth_mutex_release(m);
 
 	/* 
@@ -664,6 +691,8 @@ intern void pth_mutex_releaseall(pth_t t
 
 int pth_mutex_destroy(pth_mutex_t *mutex)
 {
+    mutex = pth_fix_mutex_ptr(mutex);
+
     /* 
      * Sanity check... 
      *	This only valid for shared mutexes.
@@ -810,6 +839,8 @@ int pth_rwlock_release(pth_rwlock_t *rwl
 
 int _pth_cond_init(pth_cond_t *cond, int pshared)
 {
+    cond = pth_fix_cond_ptr(cond);
+
     rfutex_init(&cond->cn_shared, cond, pshared);
     if (pshared != TRUE)
 	cond->cn_index = -1;
@@ -825,6 +856,8 @@ int _pth_cond_init(pth_cond_t *cond, int
 
 int pth_cond_destroy(pth_cond_t *cond)
 {
+    cond = pth_fix_cond_ptr(cond);
+
     /*
      * Sanity check...
      * This only valid for shared cond variables.
@@ -884,6 +917,8 @@ int pth_cond_await(pth_cond_t *cond, pth
     pth_t current= descr->current;
     int futx_fd = 0;
 
+    cond = pth_fix_cond_ptr(cond);
+
     /* consistency checks */
     if ((!(cond->cn_state & PTH_COND_INITIALIZED)) || (mutex->mx_owner !=
current))
         return FALSE;
@@ -1010,6 +1045,8 @@ static int pth_check_waiters(void *arg)
 {
     pth_cond_t *cond = (pth_cond_t *)arg;
 
+    cond = pth_fix_cond_ptr(cond);
+
     if (!cond->cn_waiters)
 	return TRUE;
     else
@@ -1024,6 +1061,8 @@ int pth_cond_notify(pth_cond_t *cond, in
     char c = (int)1;
     pth_descr_t ds = NULL;
     
+    cond = pth_fix_cond_ptr(cond);
+
     /* consistency checks */
     if (!(cond->cn_state & PTH_COND_INITIALIZED))
         return FALSE;



> -----Original Message-----
> From: Howell, David P [mailto:[email protected]] 
> Sent: Friday, January 24, 2003 5:08 PM
> To: [email protected]; [email protected]
> Subject: RE: [pthreads-devel] More 
> PTHREAD_PROCESS_SHARED(Storing pointers into shared memory?)
> 
> 
> This is clearly a bug, seems like we have to nail the NGPT 
> shared area to a 
> known virtual address that all processes using shared mutexes 
> would use. It 
> is unfortunate but necessary to add restrictions like this 
> with the current implementation, we should investigate other 
> ways to implement this.
> 
> I just modified pth_sync.c in the call to mmap to use a start 
> address of a 
> constant 0x48000000, this should keep it at the same virtual 
> address for all processes that use the ngpt runtime. Is there 
> a better place to put this to keep it out of the way of other 
> mmaps, or just to place it better for efficiency? Also, the 
> man page says that 'start' is a hint only, that's scary for a 
> solution like this. 
> 
> But this may help folks who are stuck on this to make 
> progress. We could for the short term do this and make it a 
> configuration time tuneable.
> 
> Any thoughts? 
>  
> Dave Howell
> 
> -----Original Message-----
> From: [email protected] [mailto:[email protected]] 
> Sent: Tuesday, January 07, 2003 3:34 PM
> To: [email protected]
> Subject: Re: [pthreads-devel] More 
> PTHREAD_PROCESS_SHARED(Storing pointers into shared memory?)
> 
> 
> 
> I have written code that demonstrates the problem.  This 
> scenario involves two programs(proga, progb) executing the 
> same code that manipulates some shared data protected with a 
> PTHREAD_PROCESS_SHARED mutex. Both programs use the same 
> initialization code to map its shared memory and, if 
> necessary, initialize the mutex.  The only difference between 
> the two is that progb has another library linked in 
> (libbreak). This library doesn't do anything but allocate 
> some memory, but takes up address space causing the internal 
> NGPT shared memory to be mapped at a different address than 
> proga.  The source to everything is in the attached tarball.
> 
> After initialization, each program acquires the mutex, prints 
> out the shared data, then updates it to be the programs name 
> (argv[0]).  Before each step it asks the user to press enter 
> (this way you can get the other program started).
> 
> If you run two instances of proga, or two instances of progb, 
> everything works as expected.  Running proga with progb 
> causes the locks not to work (seemingly ignored).
> 
> I found it useful to add this code to the bottom of
> pth_initialize_shared():
> 
>     fprintf(stderr, "NGPT: shared area pointer: %p\n", 
> pth_shared_area);
> 
> You can see then, that the memory is mapped to a different 
> address in progb then proga.
> 
> to test:
>    untar ngptproblem.tar.gz
>    ./configure --with-ngptdir=/your/ngpt/path
>    make
>    ./proga
>    (on another tty)
>    ./progb (or proga if you want to see it work)
> 
>    The first <enter> locks the mutex, prints out the data, 
> then updates it.  The second <enter> unlocks the mutex.  So 
> hit enter on proga, then hit enter on progb.  The lock wont 
> be honored and the critical section executes(because the 
> pointer in the shared mutex is wrong from progb's perspective).
> 
> 
> Here is the output from proga:
> 
> NGPT: shared area pointer: 0x40195000
> internal mutex pointer: 0x40198200
> press a key to obtain the lock and update data
> 
> locking mutex...
> locked
> shared data is ''
> press a key to unlock the mutex
> 
> unlocking mutex...
> unlocked
> 
> And here from progb:
> 
> NGPT: shared area pointer: 0x40197000
> Initializing library
> press a key to obtain the lock and update data
> 
> locking mutex...
> locked
> shared data is './proga'
> press a key to unlock the mutex
> 
> unlocking mutex...
> unlocked
> 
> Thanks for taking a look,
> Chris McFarlen
> _______________________________________________
> pthreads-devel mailing list
> [email protected] 
> http://www-124.ibm.com/developerworks/oss/mailman/listinfo/pth
reads-devel