php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #39648 Implementation of PHP functions chown() and chgrp() not thread safe.
Submitted: 2006-11-27 17:05 UTC Modified: 2006-11-30 11:27 UTC
From: wharmby at uk dot ibm dot com Assigned: iliaa (profile)
Status: Closed Package: Unknown/Other Function
PHP Version: 5CVS-2006-11-27 (snap) OS: Linux RHEL4
Private report: No CVE-ID: None
View Developer Edit
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: wharmby at uk dot ibm dot com
New email:
PHP Version: OS:

 

 [2006-11-27 17:05 UTC] wharmby at uk dot ibm dot com 
Description:
------------
The current implementation of the chown() and chgrp() 
functions on Linux use the non-reentrant getpwnam() and 
getgrnam() C library calls respectively rather than the
reentrant getpwnam_r() and getgrnam_r(). Therefore using
either chown() or chgrp() on Linux in a ZTS enabled build
could lead to unpredictable/undesirable results.

The following patch, which was built against the latest snapshot (Nov 27th, 2006, 0730 GMT) modifies the code in 
ext/standard/filestat.c to use the reentrant versions of
these functions and so make these 2 functions thread safe:

            http://pastebin.ca/259657

However, I am concerned that this patch relies on the C
library supporting the POSIX.1 functions getpwnam_r(), 
getgrnam_r() and sysconf(). These are all implemented by 
GNU libc but are there other C libraries used to build PHP 
which may not have the necessary support ?  Or is it 
reasonable to assume that all C libraries used when buiding 
PHP will be POSIX.1 compliant ? 

N.B There are other uses of these non-reentrant functions in
the PHP code base (e.g posix.c and fopen_wrappers.c) and I 
m happy to produce the necessary patches to fix these uses 
if this fix proves satisfactory.

Andy Wharmby
IBM United Kingdom Limited

Reproduce code:
---------------
---------------
Problem found by code inspection. As with most thread safety issues difficult to produce a simple testcase which will show a reproducible crash but current Linux executable is clearly
not reentrant.

Expected result:
----------------
N/A

Actual result:
--------------
N/A

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2006-11-29 23:42 UTC] iliaa@php.net 
This bug has been fixed in CVS.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.
 [2006-11-30 11:27 UTC] wharmby at uk dot ibm dot com 
First thanks for taking a look at this one for me Ilia. 

One question about the fix though. Rather than 
unconditionally calling getpwnam_r and getgrnam_r as my
patch did you have modified the autoconf input to cause the 
necessary "HAVE_" variables to be set and #if def'd the code
to check these variables. This was the bit I was unsure 
about as I have not worked with autoconf files much before 
but that all looks straightforward and will help me construct a fix for a similar issue I have identified in the
PHP code.

My only concern is that as the code stands if the reentrant 
(_r) version of the function is NOT available then the
standard function is called instead. This assumes that if a
library does not implement a "_r" version of the function 
that the basic function is reentrant. Whilst I don't know
of an instance when this is not true I wondered if this was 
a known safe assumption ? It would sure make life easier if 
it was true but I have not seen this documented anywhere. 
You have probably already done all the necessary research 
and found this to be a safe assumption but for my own future
information it would be very useful if you could confirm 
this please. 

I half expected to see code similar to that in reentrancy.c 
where if the "_r" version is not available the standard 
function is called wrapped in a mutex. I realize that 
assuming that if a "_r" version of a function is not  
available that assuming that the standard function must NOT 
be reentrant could also be a flawed assumption and without 
extra #if def's in the code could lead to unnecessary single
threading of the code but at least it ensures thread safety.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Tue Dec 03 17:01:29 2024 UTC