[Issue 126669] Possible null pointer dereference

[bu...@apache.org]

https://bz.apache.org/ooo/show_bug.cgi?id=126669

orcmid <or...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |CONFIRMED
     Ever confirmed|0                           |1

--- Comment #2 from orcmid <or...@apache.org> ---
(In reply to AppChecker from comment #0)
> In file main/sal/osl/unx/profile.c at line 318 there is if-statement: 
>    
>     if ( pProfile == 0 )
>     {
> 		pthread_mutex_unlock(&(pProfile->m_AccessLock));
> #ifdef TRACE_OSL_PROFILE
>         OSL_TRACE("Out osl_closeProfile [pProfile==0]\n");
> #endif
>         return sal_False;
>     }

The immediate problem can be solved simply by deleting the pthread_mutex_unlock
invocation.  


This particular pProfile value can be only be NULL and this code executed only
if the immediately preceding 

  pProfile = acquireProfile(Profile, ...);

operation returns NULL.

Prior to that point, pPointer pointed to a correct Profile on which the mutex
was locked by this code.  The second problem has to do with not leaving that
mutex locked just in case some thread is waiting on it.  

The convention in this code is that after obtaining the lock, users of the
profile will check to see if the Profile is marked invalid.  If it is, they
release their lock on it and do not attempt to use the profile.

That means it is safe to unlock the Profile once it is set invalid at line 300,

    pProfile->m_bIsValid=sal_False; 

The unlock can be placed immediately after that line.  

The new problem is that if the current thread, which sets the profile invalid,
releases the memory that pProfile points to, there is a prospective use after
free situation when one of the waiting threads finally takes a look at the
profile's validity.

If we can establish that the data structure having the mutex_t and m_bIsValid
entries is not freed by any code that unlocks a successful lock on it, this
code is safe.  Any "memory leak" is presumably temporary.

Unfortunately, at lines 366-370, the non-NULL-pProfile  structure is freed. It
is also unlocked. This is the pProfile returned by one of the two acquire
profile calls.

Destroying the mutex and freeing the pProfile-referenced data structure can be
a problem though.

To have this code work, with the previous corrections, it is necessary to lock
the mutex of the acquireProfile-returned pProfile, if acquireProfile does not
assure that.

That needs to be explored.

-- 
You are receiving this mail because:
You are the assignee for the issue.