This is the mail archive of the cygwin@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: bug in pthread_mutex_init (cygwin-src-20010813)


Robert Collins writes:
> Try this David... if it works please confirm via the mailing list and
> I'll fixup the cond_init as well.

This won't fix it.  When I applied your patch, the result in thread.cc
was this:

 1|  if ((((pshared_mutex *)(mutex))->flags & SYS_BASE == SYS_BASE))
 2|    {
 3|      // a pshared mutex or random data?
 4|      pshared_mutex *pmutex=(pshared_mutex *)(mutex);
 5|      if ((MT_INTERFACE->pshared_mutexs[pmutex->id]) != NULL)
 6|        return EBUSY;
 7|      // not a pshared mutex.
 8|      *mutex = NULL;
 9|    }
10|  if (attr && !verifyable_object_isvalid (*attr, PTHREAD_MUTEXATTR_MAGIC))
11|    return EINVAL;
12|
13|  if (verifyable_object_isvalid (*mutex, PTHREAD_MUTEX_MAGIC))
14|    return EBUSY;

If 'mutex' really is random data, then the program will segfault on
either line 5 (because "pmutex->id" is a random integer that is
outside the bounds of the "pshared_mutexs" array) or line 13 (I'm not
sure why, but it does).  Running my test program inside gdb shows this
quickly.

Worse, if the random integer in pmutex->id -is- a valid entry in your
array, then the random data provided to pthread_mutex_init will
masquerdade as some other shared mutex!

I'm sorry I haven't come up with a better suggestion myself.

I do have more unambiguous bugs to fix, though!  And they are also in
thread.cc, so you may want to handle them at the same time.

These 2 pthread functions:

  __pthread_setcancelstate (int state, int *oldstate)
  __pthread_setcanceltype (int type, int *oldtype)

...are supposed to permit the caller to pass NULL in for the second
parameter, meaning "don't bother telling me what the original state
was."

The cygwin implementation derefernces these pointers even if they are
NULL, resulting in a crash.  The obvious fix is to wrap the
dereferences inside "if( oldstate != NULL) { ... }" checks.

				     -Dave

-- 
 David Lum                              713-868-6950 (Houston)
 dlum@alum.mit.edu                      703-318-1266 (Dulles)

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]