#29915 [Opn->Fbk]: allocate_new_resource() dangling pointer

2005-04-12 Thread sniper
 ID:   29915
 Updated by:   [EMAIL PROTECTED]
 Reported By:  test dot 007 at seznam dot cz
-Status:   Open
+Status:   Feedback
 Bug Type: Reproducible crash
 Operating System: any
 PHP Version:  5CVS-2004-08-31 (dev)
 New Comment:

Please provide any patches in unified diff format (diff -u)
and also, put them online somewhere where we can download them. 

Can you also give some short example script which illustrates the
problem this patch supposedly fixes?



Previous Comments:


[2005-02-16 16:48:31] test dot 007 at seznam dot cz

Hello Tony,

I found the bug in 4.3.8, but I've checked sources of PHP5 (a link you
provided), and the bug is still present in both 4.x and 5.x.

The bug only occurs if there are more PHP threads running, and you have
a bad luck. It is unrelated to a PHP script, it can crash with any
script(s) => I can't provide you with a particular "crashing" or even
"crashes here" script.

To sum up the problem, allocate_new_resource() uses a resource a
microsecond after it has unlocked a mutex. If another thread removes
the resource during the microsecond, crash.

tsrm_mutex_unlock(tsmm_mutex);
if (tsrm_new_thread_end_handler) {
tsrm_new_thread_end_handler(thread_id,
&((*thread_resources_ptr)->storage));

My patch is obviously harmless, and, believe me, it helps :-)



[2005-02-10 00:24:12] [EMAIL PROTECTED]

Please try using this CVS snapshot:

  http://snaps.php.net/php5-STABLE-latest.tar.gz
 
For Windows:
 
  http://snaps.php.net/win32/php5.0-win32-latest.zip

Please provide a short but complete reproduce script if you still
expirience this issue.
Also, I don't quite understand: you're talking about 5.0.x-CVS, but the
patch you provided is against 4.3.x-CVS.



[2004-08-31 15:30:07] test dot 007 at seznam dot cz

Description:

There's a dangling pointer in TSRM.c: allocate_new_resource(),  the
last command.
After 
tsrm_mutex_unlock(tsmm_mutex);
the (*thread_resources_ptr) is invalid if you're really unlucky.

Fix:
diff ..\php-4.3.8.orig\TSRM\TSRM.c TSRM\TSRM.c
258a259
> tsrm_tls_entry *new_resource;
261a263
> new_resource = (*thread_resources_ptr);
291c293
<   tsrm_new_thread_end_handler(thread_id,
&((*thread_resources_ptr)->storage));
---
>   tsrm_new_thread_end_handler(thread_id, 
> &((new_resource)->storage));

Long description:
I believe there's a bug even in latest PHP 5, which crashes more
probably in debug build, see TSRM/tsrm.c:
ts_resource_ex() calls while owning a mutex:
allocate_new_resource(&thread_resources->next, thread_id);
That should add a new tsrm_tls_entry* thread_resources to the end of
the linked list.
allocate_new_resource(tsrm_tls_entry **thread_resources_ptr, THREAD_T
thread_id) adds a new linked entry (i.e. sets the list tail's next)
(*thread_resources_ptr) = (tsrm_tls_entry *)
malloc(sizeof(tsrm_tls_entry));
unlock the mutex;
tsrm_new_thread_end_handler(thread_id,
&((*thread_resources_ptr)->storage)); /* 2nd arg equals to
(*thread_resources_ptr) */
If another thread, by accident exactly the thread which has been the
last one in the linked list before the allocate_new_resource() called
malloc(), and whose "next" entry was set there, calls ts_free_thread(),
it free()s the thread_resources of ts_resource_ex() -- 
in debug build, the (thread_resources->next) vulgo
(*thread_resources_ptr) is set to 0xfeeefeee, and the call
tsrm_new_thread_end_handler(0xfeeefeee) crashes; 
in release build, the memory can be used by another malloc() or
decommited, but probably, in the very short period this dangling
pointer is used, it's not overwritten, thus the code executes almost
always without crash. 

The solution is to store (*thread_resources_ptr) on stack anytimes
between malloc() and unlock.

Reproduce code:
---
You need to run multiple threads and call ts_free_thread() from them
regularly (is it correct?). 
You need the thread hash table to be too small so that hash collisions
occur. To crash you need the tail of a linked list to call
ts_free_thread() at the same time a new tail is added.






-- 
Edit this bug report at http://bugs.php.net/?id=29915&edit=1


#29915 [Opn->Fbk]: allocate_new_resource() dangling pointer

2005-02-09 Thread tony2001
 ID:   29915
 Updated by:   [EMAIL PROTECTED]
 Reported By:  test dot 007 at seznam dot cz
-Status:   Open
+Status:   Feedback
 Bug Type: Reproducible crash
 Operating System: any
 PHP Version:  5CVS-2004-08-31 (dev)
 New Comment:

Please try using this CVS snapshot:

  http://snaps.php.net/php5-STABLE-latest.tar.gz
 
For Windows:
 
  http://snaps.php.net/win32/php5.0-win32-latest.zip

Please provide a short but complete reproduce script if you still
expirience this issue.
Also, I don't quite understand: you're talking about 5.0.x-CVS, but the
patch you provided is against 4.3.x-CVS.


Previous Comments:


[2004-08-31 15:30:07] test dot 007 at seznam dot cz

Description:

There's a dangling pointer in TSRM.c: allocate_new_resource(),  the
last command.
After 
tsrm_mutex_unlock(tsmm_mutex);
the (*thread_resources_ptr) is invalid if you're really unlucky.

Fix:
diff ..\php-4.3.8.orig\TSRM\TSRM.c TSRM\TSRM.c
258a259
> tsrm_tls_entry *new_resource;
261a263
> new_resource = (*thread_resources_ptr);
291c293
<   tsrm_new_thread_end_handler(thread_id,
&((*thread_resources_ptr)->storage));
---
>   tsrm_new_thread_end_handler(thread_id, 
> &((new_resource)->storage));

Long description:
I believe there's a bug even in latest PHP 5, which crashes more
probably in debug build, see TSRM/tsrm.c:
ts_resource_ex() calls while owning a mutex:
allocate_new_resource(&thread_resources->next, thread_id);
That should add a new tsrm_tls_entry* thread_resources to the end of
the linked list.
allocate_new_resource(tsrm_tls_entry **thread_resources_ptr, THREAD_T
thread_id) adds a new linked entry (i.e. sets the list tail's next)
(*thread_resources_ptr) = (tsrm_tls_entry *)
malloc(sizeof(tsrm_tls_entry));
unlock the mutex;
tsrm_new_thread_end_handler(thread_id,
&((*thread_resources_ptr)->storage)); /* 2nd arg equals to
(*thread_resources_ptr) */
If another thread, by accident exactly the thread which has been the
last one in the linked list before the allocate_new_resource() called
malloc(), and whose "next" entry was set there, calls ts_free_thread(),
it free()s the thread_resources of ts_resource_ex() -- 
in debug build, the (thread_resources->next) vulgo
(*thread_resources_ptr) is set to 0xfeeefeee, and the call
tsrm_new_thread_end_handler(0xfeeefeee) crashes; 
in release build, the memory can be used by another malloc() or
decommited, but probably, in the very short period this dangling
pointer is used, it's not overwritten, thus the code executes almost
always without crash. 

The solution is to store (*thread_resources_ptr) on stack anytimes
between malloc() and unlock.

Reproduce code:
---
You need to run multiple threads and call ts_free_thread() from them
regularly (is it correct?). 
You need the thread hash table to be too small so that hash collisions
occur. To crash you need the tail of a linked list to call
ts_free_thread() at the same time a new tail is added.






-- 
Edit this bug report at http://bugs.php.net/?id=29915&edit=1