# Everyone on the road?
- Thread: https://externals.io/message/11529
- Original archive: https://news-web.php.net/php.internals/11529
- Started by Andi Gutmans on 2004-07-24T07:35:25+00:00
- Messages: 15
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt
---
## #11529 — Andi Gutmans — 2004-07-24T07:35:25+00:00
URL: https://externals.io/message/11529#11529
Hey,
It's a bit quiet here lately :)
Has anyone had a chance to try, test and benchmark the `realpath()` patch I
send to the list?
Andi
---
## #11540 — Rasmus Lerdorf — 2004-07-24T15:06:24+00:00
URL: https://externals.io/message/11529#11540 · In reply to: #11529
> Has anyone had a chance to try, test and benchmark the `realpath()` patch I
> send to the list?
I had a look the other day, but here is a more detailed look at the system
calls involved in a php5 request. I have stripped out most of the stuff
we can't do anything about. The script I am testing is
/var/home/rasmus/php5/o which contains:
The system calls required to open and read the script:
getcwd("/var/home/rasmus/php5", 4096) = 22
lstat64("/var/home/rasmus/php5/o", {st_mode=S_IFREG|0644, st_size=156, ...}) = 0
setitimer(ITIMER_PROF, {it_interval={0, 0}, it_value={0, 0}}, NULL) = 0
rt_sigaction(SIGPROF, {0x4067a7a0, [PROF], SA_RESTORER|SA_RESTART, 0x406ea658}, {0x4067a7a0, [PROF], SA_RESTORER|SA_RESTART, 0x406ea658}, 8) = 0
rt_sigprocmask(SIG_UNBLOCK, [PROF], NULL, 8) = 0
read(3, " I'd love to see a `realpath()` replacement function which makes use of the
> stat cache for partial paths as well. Chances are that on a busy ISP
> server, for example, you will have thousands of scripts and include files
> served up from the same base path and you are going to be statting your
> way along that tree every time you hit a file that isn't in the cache.
It is a very good idea to replace `realpath()` with an own function.
Realpath is not only slow, it is also "dangerous" on some systems like
linux where it also works on paths that are not really existing.
example: include "./foo/bar/index.php/../../../../../../etc/passwd";
On the BSDs this should be invalid, but linux happily accepts it as
/etc/passwd. This is just one thing that would largely benefit from an
own `realpath()`.
Stefan
---
## #11542 — Christian Schneider — 2004-07-24T15:42:07+00:00
URL: https://externals.io/message/11529#11542 · In reply to: #11541
Stefan Esser wrote:
> Realpath is not only slow, it is also "dangerous" on some systems like
> linux where it also works on paths that are not really existing.
>
> example: include "./foo/bar/index.php/../../../../../../etc/passwd";
This is a completely legal unix path as .. in / is / again. And from a
security point of view I'd say the only thing you gain by disallowing
this is that the number of .. in the path have to be guessed (pretty
easily) by the attacker.
Or am I missing something here?
Not that I can see the benefit of a home-grown `realpath()` :-)
- Chris
---
## #11543 — Stefan Esser — 2004-07-24T15:54:19+00:00
URL: https://externals.io/message/11529#11543 · In reply to: #11542
Christian Schneider wrote:
> > […]
> This is a completely legal unix path as .. in / is / again. And from a
> security point of view I'd say the only thing you gain by disallowing
> this is that the number of .. in the path have to be guessed (pretty
> easily) by the attacker.
This is no legal unix path, because index.php is a file and not a directory.
And to understand the security impact:
include "./foo/bar/template_".$userinput;
By setting $userinput to "validtemplate.php/../../../../etc/passwd" you
get to the /etc/passwd file, altough the path is completely illegal.
Stefan
---
## #11545 — Christian Schneider — 2004-07-24T16:30:49+00:00
URL: https://externals.io/message/11529#11545 · In reply to: #11543
Stefan Esser wrote:
> This is no legal unix path, because index.php is a file and not a
Oops, missed that part of the path, just looked at the .. :-)
> And to understand the security impact:
> include "./foo/bar/template_".$userinput;
... which I'd consider bad practice anyway but that's another story :-)
- Chris
---
## #11556 — Andi Gutmans — 2004-07-25T17:59:12+00:00
URL: https://externals.io/message/11529#11556 · In reply to: #11529
Hi,
Thanks for the in-depth analysis.
I agree that we should nuke that extra `fstat()`. We need to track down where
it is (probably streams :). I don't think it's a good idea to cache the
`stat()` itself because usually you don't `stat()` the same file twice in the
same request, and caching those in between requests could lead to weird
bugglets such as Last-Modified: not changing even though you changed the
script. I think the `realpath()` is the main performance problem and easiest
to deal with as very rarely will the real path change so caching them can't
do too much damage.
The `time()` call is part of the patch to check if we should refresh the
realpath cache for this specific path. If it becomes a problem then we
might need to think of a less deterministic approach where we call `time()`
only every few requests. Another idea I had was to call `time()` once at the
beginning of the request and always use the same one for reference. I don't
see any major problem with this as far as realpath is concerned. It might
even be useful in other places where not the exact time is needed but the
execution start time is good enough.
I think it's going to be quite hard and not really worthwhile to add
support for partial `realpath()`. In this case I like the kiss approach. It
gives a great bang for the buck and is very simple.
By the way, if we use the VIRTUAL_DIR support that `getcwd()` is also saved.
Not sure if we want to convert (it might be a bit too dangerous) but it's
something to keep in the back of the mind.
Andi
At 08:06 AM 7/24/2004 -0700, Rasmus Lerdorf wrote:
>
> > […]
>
>I had a look the other day, but here is a more detailed look at the system
>calls involved in a php5 request. I have stripped out most of the stuff
>we can't do anything about. The script I am testing is
>/var/home/rasmus/php5/o which contains:
> include '/var/www/lerdorf.com/abc';
> include '/var/www/lerdorf.com/abc';
> include '/var/www/lerdorf.com/def';
> include '/var/www/phpics.com/ghi';
>?>
>
>The system calls required to open and read the script:
>
>getcwd("/var/home/rasmus/php5", 4096) = 22
>lstat64("/var/home/rasmus/php5/o", {st_mode=S_IFREG|0644, st_size=156,
>...}) = 0
>setitimer(ITIMER_PROF, {it_interval={0, 0}, it_value={0, 0}}, NULL) = 0
>rt_sigaction(SIGPROF, {0x4067a7a0, [PROF], SA_RESTORER|SA_RESTART,
>0x406ea658}, {0x4067a7a0, [PROF], SA_RESTORER|SA_RESTART, 0x406ea658}, 8) = 0
>rt_sigprocmask(SIG_UNBLOCK, [PROF], NULL, 8) = 0
>read(3, "read(3, "", 4096) = 0
>read(3, "", 8192) = 0
>close(3) = 0
>
>ok so far. Now the first include:
>
>getcwd("/var/home/rasmus/php5", 4096) = 22
>lstat64("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
>lstat64("/var/www", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
>lstat64("/var/www/lerdorf.com", {st_mode=S_IFDIR|0777, st_size=16384,
>...}) = 0
>lstat64("/var/www/lerdorf.com/abc", {st_mode=S_IFREG|0644, st_size=4,
>...}) = 0
>open("/var/www/lerdorf.com/abc", O_RDONLY) = 3
>fstat64(3, {st_mode=S_IFREG|0644, st_size=4, ...}) = 0
>fstat64(3, {st_mode=S_IFREG|0644, st_size=4, ...}) = 0
>lseek(3, 0, SEEK_CUR) = 0
>read(3, "abc\n", 8192) = 4
>read(3, "", 8192) = 0
>read(3, "", 8192) = 0
>close(3) = 0
>
>Since nothing is cached yet we see `realpath()` statting the entire tree
>leading to the abc include file. There is an obvious double fstat on the
>opened descriptor there that we should track down.
>
>Then we get:
>
>fstat64(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 1), ...}) = 0
>old_mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
>0) = 0x40018000
>write(1, "abc\n", 4) = 4
>
>To write out the results.
>
>The second time we include the same file the stat cache kicks in and we
>have:
>
>getcwd("/var/home/rasmus/php5", 4096) = 22
>time(NULL) = 1090679437
>open("/var/www/lerdorf.com/abc", O_RDONLY) = 3
>fstat64(3, {st_mode=S_IFREG|0644, st_size=4, ...}) = 0
>fstat64(3, {st_mode=S_IFREG|0644, st_size=4, ...}) = 0
>lseek(3, 0, SEEK_CUR) = 0
>read(3, "abc\n", 8192) = 4
>read(3, "", 8192) = 0
>read(3, "", 8192) = 0
>close(3) = 0
>write(1, "abc\n", 4) = 4
>
>Not sure where that `time()` call came from here, and again we see the
>double fstat. And since we have already started writing the output the
>results are written out immediately in a single write.
>
>Now the /var/www/lerdorf.com/def include:
>
>getcwd("/var/home/rasmus/php5", 4096) = 22
>lstat64("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
>lstat64("/var/www", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
>lstat64("/var/www/lerdorf.com", {st_mode=S_IFDIR|0777, st_size=16384,
>...}) = 0
>lstat64("/var/www/lerdorf.com/def", {st_mode=S_IFREG|0644, st_size=4,
>...}) = 0
>time(NULL) = 1090679437
>open("/var/www/lerdorf.com/def", O_RDONLY) = 3
>fstat64(3, {st_mode=S_IFREG|0644, st_size=4, ...}) = 0
>fstat64(3, {st_mode=S_IFREG|0644, st_size=4, ...}) = 0
>lseek(3, 0, SEEK_CUR) = 0
>read(3, "def\n", 8192) = 4
>read(3, "", 8192) = 0
>read(3, "", 8192) = 0
>close(3) = 0
>write(1, "def\n", 4) = 4
>
>We get what we expect here too. Since the stat cache only caches full
>paths and we are now including a different file from the same location we
>have to stat the whole tree again.
>
>And the final include:
>
>getcwd("/var/home/rasmus/php5", 4096) = 22
>lstat64("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
>lstat64("/var/www", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
>lstat64("/var/www/phpics.com", {st_mode=S_IFDIR|0755, st_size=8192, ...}) = 0
>lstat64("/var/www/phpics.com/ghi", {st_mode=S_IFREG|0644, st_size=4, ...}) = 0
>time(NULL) = 1090679437
>open("/var/www/phpics.com/ghi", O_RDONLY) = 3
>fstat64(3, {st_mode=S_IFREG|0644, st_size=4, ...}) = 0
>fstat64(3, {st_mode=S_IFREG|0644, st_size=4, ...}) = 0
>lseek(3, 0, SEEK_CUR) = 0
>read(3, "ghi\n", 8192) = 4
>read(3, "", 8192) = 0
>read(3, "", 8192) = 0
>close(3) = 0
>write(1, "ghi\n", 4) = 4
>
>No surprises there either.
>
>Shutting down (cli version):
>
>close(1) = 0
>close(0) = 0
>setitimer(ITIMER_PROF, {it_interval={0, 0}, it_value={0, 0}}, NULL) = 0
>setitimer(ITIMER_PROF, {it_interval={0, 0}, it_value={30, 0}}, NULL) = 0
>rt_sigaction(SIGPROF, {0x4067a7a0, [PROF], SA_RESTORER|SA_RESTART,
>0x406ea658}, {0x4067a7a0, [PROF], SA_RESTORER|SA_RESTART, 0x406ea658}, 8) = 0
>rt_sigprocmask(SIG_UNBLOCK, [PROF], NULL, 8) = 0
>setitimer(ITIMER_PROF, {it_interval={0, 0}, it_value={0, 0}}, NULL) = 0
>munmap(0x40018000, 4096) = 0
>exit_group(0) = ?
>
>
>I'd love to see a `realpath()` replacement function which makes use of the
>stat cache for partial paths as well. Chances are that on a busy ISP
>server, for example, you will have thousands of scripts and include files
>served up from the same base path and you are going to be statting your
>way along that tree every time you hit a file that isn't in the cache.
>
>And that double-fstat needs to go away. A very quick look for it seems to
>show one checking if the opened file is a regular file in
>fopen_wrappers.c. Ideally we would simply use the stat struct from our
>own `realpath()` implementation and neither of these `fstat()` calls would be
>needed. The goal should be to average at most 1 stat per file, whether it
>be the main script or include files. For the main script, if we are
>running under Apache, Apache has already done this stat and passed us the
>stat struct
>
>-Rasmus
---
## #11557 — Rasmus Lerdorf — 2004-07-25T18:19:05+00:00
URL: https://externals.io/message/11529#11557 · In reply to: #11556
> I think it's going to be quite hard and not really worthwhile to add
> support for partial `realpath()`. In this case I like the kiss approach. It
> gives a great bang for the buck and is very simple.
Well, there is another approach here. There are only 2 places where we
need the full paths. The first is for the included_files list to be able
to do the require_once/include_once check and the second is for the
open_basedir mechanism. There are a lot of people out there who do not
use open_basedir and I would guess most people looking for maximum
performance wouldn't be in a situation where they needed the open_basedir
feature. So the only real feature in question here is the *_once
checking and even there you don't necessarily need the full path although
without it you could potentially run into problems if you include the same
file through 2 different relative paths or more obscurely if you
deliberately change your cwd halfway through your script and include a
file of the same name through the same relative path but this is not
actually the same file. In both cases you are basically going out of your
way to trick the _once check. At Yahoo I have been running everything
with relative paths for well over a year now without any problems.
So, some approaches:
a. compile-time switch to disable full paths completely
b. ini switch to disable full paths completely unless open_basedir is enabled
c. ini switch to disable full paths from include/require but enable them
for include_once/require_once. That is, if you want *_once protection
you use include_once/require_once. If you don't, you just use
include/require and include/require would not populate the
included_files list.
-Rasmus
---
## #11966 — Rasmus Lerdorf — 2004-08-06T15:06:23+00:00
URL: https://externals.io/message/11529#11966 · In reply to: #11556
> The `time()` call is part of the patch to check if we should refresh the
> realpath cache for this specific path. If it becomes a problem then we
> might need to think of a less deterministic approach where we call `time()`
> only every few requests. Another idea I had was to call `time()` once at the
> beginning of the request and always use the same one for reference. I don't
> see any major problem with this as far as realpath is concerned. It might
> even be useful in other places where not the exact time is needed but the
> execution start time is good enough.
I meant to mention this a while ago, but it slipped my mind. Most web
servers do that `time()` call for us at the beginning of the request because
they need it for logging purposes. I think the right approach here is to
add a SAPI call to expose this. For Apache-1.3 it is right in the
request_rec in the request_time field. Obviously the SAPI call would
simply do the `time()` call itself if it can't fetch it from somewhere
internally.
-Rasmus
---
## #11968 — Sara Golemon — 2004-08-06T15:59:29+00:00
URL: https://externals.io/message/11529#11968 · In reply to: #11966
> I meant to mention this a while ago, but it slipped my mind. Most web
> servers do that `time()` call for us at the beginning of the request because
> they need it for logging purposes. I think the right approach here is to
> add a SAPI call to expose this. For Apache-1.3 it is right in the
> request_rec in the request_time field. Obviously the SAPI call would
> simply do the `time()` call itself if it can't fetch it from somewhere
> internally.
>
It sounds good enough, but I'd worry about people using this in
incompletely-thought-out benchmarking scripts.
A SAPI which supports the request time record would appear to take slightly
longer than a SAPI which does not (the time between processing the request
in the web server and passing off to PHP {plus compile, plus execute down to
that instructuion})
Do NOT use this method for benchmarking between
different webservers, inconsistent implementations could give unexpected
results.
-Sara
---
## #11969 — Rasmus Lerdorf — 2004-08-06T16:37:45+00:00
URL: https://externals.io/message/11529#11969 · In reply to: #11968
> > […]
> It sounds good enough, but I'd worry about people using this in
> incompletely-thought-out benchmarking scripts.
Adding a SAPI call doesn't mean exposure to scripts, so I don't really see
how this is relevant. Sure, someone could write a PHP extension to
benchmark different servers, but what are the chances of that? And if
they are advanced enough to write an extension, they are smart enough to
normalize the request start times.
-Rasmus
---
## #11970 — Sara Golemon — 2004-08-06T18:21:09+00:00
URL: https://externals.io/message/11529#11970 · In reply to: #11969
> Adding a SAPI call doesn't mean exposure to scripts, so I don't really see
> how this is relevant. Sure, someone could write a PHP extension to
> benchmark different servers, but what are the chances of that? And if
> they are advanced enough to write an extension, they are smart enough to
> normalize the request start times.
>
Sorry, I misread... I thought you were talking about exposing this call to
userspace.
-Sara
---
## #11975 — Andi Gutmans — 2004-08-06T21:01:39+00:00
URL: https://externals.io/message/11529#11975 · In reply to: #11966
At 08:59 AM 8/6/2004 -0700, Sara Golemon wrote:
> > […]
>It sounds good enough, but I'd worry about people using this in
>incompletely-thought-out benchmarking scripts.
>
>A SAPI which supports the request time record would appear to take slightly
>longer than a SAPI which does not (the time between processing the request
>in the web server and passing off to PHP {plus compile, plus execute down to
>that instructuion})
>
>
> Do NOT use this method for benchmarking between
>different webservers, inconsistent implementations could give unexpected
>results.
>
If you call it get_request_start_time() it should make it clearer what it
means.
I guess we could put it into SAPI I don't see any disadvantage although in
any case, doing it one more time per-request wouldn't be that bad either.
I hope to have more time towards the end of next week and then I can
hopefully make this improvement.
Andi
---
## #11977 — Rasmus Lerdorf — 2004-08-06T21:12:45+00:00
URL: https://externals.io/message/11529#11977 · In reply to: #11975
> If you call it get_request_start_time() it should make it clearer what it
> means.
> I guess we could put it into SAPI I don't see any disadvantage although in
> any case, doing it one more time per-request wouldn't be that bad either.
> I hope to have more time towards the end of next week and then I can
> hopefully make this improvement.
Sure, it's a single syscall, but this entire effort is about removing
syscalls, so adding one that isn't necessary because it has already been
done during the request makes sense.
I am just cleaning up some other code where a sapi call for this would
make things a lot cleaner. Without the sapi call you end up needing to do
this:
#if HAVE_APACHE
#include "httpd.h"
#endif
...
#if HAVE_APACHE
t = ((request_rec *)SG(server_context))->request_time;
#else
t = time(0);
#endif
-Rasmus
---
## #11979 — George Schlossnagle — 2004-08-06T21:21:50+00:00
URL: https://externals.io/message/11529#11979 · In reply to: #11977
>
> > […]
>
> Sure, it's a single syscall, but this entire effort is about removing
> syscalls, so adding one that isn't necessary because it has already
> been
> done during the request makes sense.
I didn't imagine this would be so controversial. I'm all for this
added callout.
George