Edit report at https://bugs.php.net/bug.php?id=52312&edit=1
ID: 52312
Comment by: Terry at ellisons dot org dot uk
Reported by: v dot damore at gmail dot com
Summary: PHP safe_mode/open_basedir - lstat performance
problem
Status: Analyzed
Type: Bug
Package: Safe Mode/open_basedir
Operating System: Linux
PHP Version: 5.2.13
Block user comment: N
Private report: N
New Comment:
Yes Rasmus. We both know that; but this won't be address without something like an LPC-style
file-based cache to preserve context across image activations, but all this isn't that relevant
to #52312 -- "PHP safe_mode/open_basedir - lstat performance problem".
What is relevant are my points about a require_once 6 sub-directories down taking 13 stats and 1
open with open_basedir unset and 60 stats and 1 open if it is set, and that the security requirement
could still be implemented within the former stat number if done correctly.
This isn't a material problem for single source file scripts, but MediaWiki, Wordpress and the
like typically load in ~100 modules generating ~6K stats per request. And this does become a
problem.
Previous Comments:
------------------------------------------------------------------------
[2013-02-23 15:22:59] rasmus@php.net
First-request cli isn't going to have a populated realpath cache no matter what
we do since this cache is per-process and in no way shared nor persistent across
different processes.
------------------------------------------------------------------------
[2013-02-23 13:20:03] Terry at ellisons dot org dot uk
The above discussion was largely about the I/O overheads with open_basedir specified, so my figures
where in that context, and dd2.inc is just an empty class, but this isnt relevant to its inclusion.
Try:
echo "">dummy.inc
pwd
strace -tt -o /tmp/strace.log \
php -d realpath_cache_ttl=9999 -d open_basedir=$(pwd) \
-r '$x="./dummy.inc"; require_once($x);'
to see what I mean (I did this from 6 dir levels down which is pretty typical of web set ups.) since
you can do this yourself I won't dump the log output but:
for vb in lstat fstat stat open; do
echo -n "$vb "
sed -ne '/dummy.inc/,/close(/p' /tmp/strace | grep -c " $vb"
done
lstat 54
fstat 5
stat 1
open 1 (*) I removed the open for "/etc/ld.so.cache" which isn't relevant here
whereas dropping the open_basedir directive and repeating gives:
lstat 8
fstat 5
stat 1
open 1
so the open_basedir can have a severe impact on performance. I am still at loss as to why the PHP
cache is disabled if open_basedir is set. Surely the security objectives would be entirely achieved
by doing the real walk only during the open routine itself?
I realise that NFS tuning can mitigate these issues -- e.g. using noatime and setting the
actimeo,... parameters or even using a local fscache (in Ubuntu this is the cachefilesd package).
I agree that running mod_php5 with APC or O+ help a lot here, but as we've discussed in the
past, APC and O+ do not support php-cgi or php-cli. (I know that APC has apc.enable_cli, but this
seems to be a functional no-op.) We should have acceptable PHP performance over all common
usecases.
------------------------------------------------------------------------
[2013-02-23 03:44:04] rasmus@php.net
I don't know what your dd2.inc is doing, but I went through and optimized syscalls a
couple of years ago, and in normal no-openbasedir mode things are pretty clean. Here
are a few scenarios:
a.php does a require on ./b.php and a require_once on ./c.php
PHP 5.4/APC-HEAD/apc.stat=1/apc.include_once_override=0/open_basedir=off
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
stat("/var/www/./b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
open("/var/www/c.php", O_RDONLY) = 29
fstat(29, {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
fstat(29, {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
fstat(29, {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
stat("/var/www/c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
The initial stat on a.php is actually done by Apache and we ask Apache for the stat
struct to avoid an extra stat there. You can see the effect of the open and then the
(quick) fstat calls on fd=29. This is where APC tries to optimize things a bit using
the include_once_override. If we turn that on we get:
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
stat("/var/www/./b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
stat("/var/www/./c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
stat("/var/www/./c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
Not ideal since there is still an extra stat for the require_once case, but the open()
is gone. And you can eliminate those stats by turning off apc.stat which means only
the initial top-page stat from Apache on a.php will be done.
Now, of course, if we turn on open_basedir we get:
www-data 1799 0.8 0.2 310232 22560 pts/6 S+ 19:26 0:00 /usr/sbin/apache2 -X
root 1887 0.0 0.0 10892 916 pts/12 R+ 19:26 0:00 grep apache
7:26pm x220:~> strace -o out -p 1799
Process 1799 attached - interrupt to quit
^CProcess 1799 detached
7:26pm x220:~> egrep "stat|open" out
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
lstat("/var/www/./b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lstat("/var/www/b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
stat("/var/www/./b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www/./c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lstat("/var/www/c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
stat("/var/www/./c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www/./c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lstat("/var/www/c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
stat("/var/www/./c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
which is far from ideal, of course, because we are not hitting our realpath cache at
all and there is an oddity there with ./ getting resolved and re-statted and this re-
realpathed. Not sure I see where your 7 times thing is though.
With ZO+ with this config
zend_optimizerplus.revalidate_freq=0
zend_optimizerplus.enable_file_override=0
open_basedir=
we see this:
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
stat("/var/www/b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
stat("/var/www/c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
Here we see that ZO+ doesn't have the Apache stat optimization that APC has, which is
something we should obviously add, but it handles the require_once better and the
open() call and fstats are gone.
When we turn on open_basedir we get:
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
lstat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
lstat("/var/www/./b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
stat("/var/www/b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
stat("/var/www/c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
Better than APC, still not great, but there are no open() calls here, so the NFS
getattr issue should be gone.
And finally, ZO+ has a TTL version of apc.stat so we can set it to only stat every 60
seconds, so even with open_basedir enabled, with this config:
zend_optimizerplus.revalidate_freq=60
zend_optimizerplus.enable_file_override=1
we get this:
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
which is just the initial stat done by Apache which we can't do anything about. So, in
general, assuming the 5.5 integrated opcode cache is based on ZO+ and we add the one
missing Apache stat optimization, I really don't think the situation is all that dire,
even for NFS-mounted scripts.
------------------------------------------------------------------------
[2013-02-23 00:49:33] Terry at ellisons dot org dot uk
Incidentally I've just straced opening a require_once on
/home/terry/work/ext/lpc/tests/dd2.inc
and PHP 5.3.17 does the 7 x lstat walk that you indicate is necessary to meet CVE-2006-5178, but it
then repeats this another 5 times and does another 3 fstats after the file is opened 45 f/lstats all
in 2.5μS (on a local disk clearly).
PHP 5.4.6 is worse, it does 5x7 walks, opens the file then does 4 fstatsand reads the file, before
doing another 3x7 lstat walks and a last stat for luck, giving a grand total of 60 f/lstats in
4μS though this one includes mmapping the contents from the file cache.
OK, I can understand -- though question -- mandating one walk but why do it seven times ????
------------------------------------------------------------------------
[2013-02-23 00:08:51] Terry at ellisons dot org dot uk
For the purposes of this bug, let's document the advisory which triggered this change:
CVE-2006-5178 see http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2006-5178.
Let's be honest, even with this patch there is still a minuscule chance that the exploit
described could succeed, because we can't go into kernel mode during the recursive descent and
prevent other processes with a higher nice being scheduled in and doing the dirty. So it replaces
"very small" by "even smaller" at the same time killing NFS performance.
As I said don't remove this change, just make it elective by adding an extra variant of the
open_basedir parameter, and openly documenting the issues so that sysadmins can make their own
judgement call on security vs performance. Let's face it on a typical shared service I can
think of easier ways to do this exploit. E.g. if exec and the proc functions aren't disabled
then there is no point in having a strong open_basedir. If you are an enterprise and just want to
lock down your apps programmers from breaching good practice / infrastructure standards over a NAS
infrastructure then the weak form is perfectly appropriate.
Also IMO, PHP is trying to do a better job here than the standard Linux realpath function and
failing.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=52312
--
Edit this bug report at https://bugs.php.net/bug.php?id=52312&edit=1