On Wed, Dec 11, 2002 at 09:25:19AM -0500, Jim Jagielski wrote:
>
> Comments??
What a coincidence. I was just about to post this mail:
---snip--
Hi,
This patch takes Jim's ap_note_cleanups_for_*_ex() functions and
plugs the open fd's being passed to children of, say, PHP.
With this patch, I tested an apache_1.3.28-dev with php3 on FreeBSD
which was configured
* to listen on 2 sockets (*:8080 and localhost:8081)
Reason: some lock files are only created when more than one
listening socket is used
* to use piped logs (cronolog) for its access_log
Reason: though I want a log _file_ to be closed on exec, the
specific log fd for the piped logger process must survive the
exec() for the logger's invocation.
in the following scenarios (using Apache-1.3.28-dev from today):
1) Unchanged source (patch NOT aplied):
* exec cgi program: /cgi-bin/openfds.cgi
-> p--------- 0 root wheel 0 /dev/fd/0
-> p--------- 0 root wheel 0 /dev/fd/1
-> -rw-r----- 1 martin kraemer 7971 /dev/fd/2 == /tmp/apa13/logs/error_log
* <!--#exec cmd="openfds" --> from an .shtml page
-> crw-rw-rw- 1 root wheel 2, 2 /dev/fd/0
-> p--------- 0 root wheel 0 /dev/fd/1
-> -rw-r----- 1 martin kraemer 7971 /dev/fd/2 == /tmp/apa13/logs/error_log
* <?php system("openfds"); ?>
-> crw-rw-rw- 1 root wheel 2, 2 /dev/fd/0
-> p--------- 0 root wheel 0 /dev/fd/1
-> -rw-r----- 1 martin kraemer 7971 /dev/fd/2 == /tmp/apa13/logs/error_log
-> srw-rw-rw- 0 martin kraemer 0 /dev/fd/5 lcl:[deejai.mch.fsc.net:8080]
rmt:[deejai2.mch.fsc.net:3902]
-> -rw-r----- 1 martin kraemer 7971 /dev/fd/15 == /tmp/apa13/logs/error_log
-> srw-rw-rw- 0 martin kraemer 0 /dev/fd/16 lcl:[localhost:8081] (Socket
is not connected)
-> srw-rw-rw- 0 martin kraemer 0 /dev/fd/17 lcl:[0.0.0.0:8080] (Socket
is not connected)
-> -rw------- 1 martin kraemer 0 /dev/fd/18 == /tmp/apa13/logs/httpd.lock.28043
-> -rw------- 1 martin kraemer 0 /dev/fd/19 == /tmp/apa13/logs/httpd.lock.28043
-> -rw-r----- 1 martin kraemer 209 /dev/fd/20 == /tmp/apa13/htdocs/openfds.php
When CustomLog refers to a file, not a pipe, then this becomes:
-> crw-rw-rw- 1 root wheel 2, 2 /dev/fd/0
-> p--------- 0 root wheel 0 /dev/fd/1
-> -rw-r----- 1 martin kraemer 8308 /dev/fd/2 == /tmp/apa13/logs/error_log
-> srw-rw-rw- 0 martin kraemer 0 /dev/fd/3 lcl:[deejai.mch.fsc.net:8080]
rmt:[deejai2.mch.fsc.net:3914]
-> -rw-r----- 1 martin kraemer 8308 /dev/fd/15 == /tmp/apa13/logs/error_log
-> srw-rw-rw- 0 martin kraemer 0 /dev/fd/16 lcl:[localhost:8081] (Socket
is not connected)
-> srw-rw-rw- 0 martin kraemer 0 /dev/fd/17 lcl:[0.0.0.0:8080] (Socket
is not connected)
-> -rw-r----- 1 martin kraemer 5130 /dev/fd/18 == /tmp/apa13/logs/access_log-2002-12-11
-> -rw------- 1 martin kraemer 0 /dev/fd/19 == /tmp/apa13/logs/httpd.lock.28043
-> -rw------- 1 martin kraemer 0 /dev/fd/20 == /tmp/apa13/logs/httpd.lock.28043
-> -rw-r----- 1 martin kraemer 209 /dev/fd/21 == /tmp/apa13/htdocs/openfds.php
(i.e., the access_log is leaked as well).
Note especially that PHP allows full access for child programs
not only to the requesting socket, but also to all listener sockets.
(here: 8080 and 8081)
* piped logs work.
2) With appended patch applied:
* exec cgi program: /cgi-bin/openfds.cgi
-> p--------- 0 root wheel 0 /dev/fd/0
-> p--------- 0 root wheel 0 /dev/fd/1
-> -rw-r----- 1 martin kraemer 7719 /dev/fd/2 == /tmp/apa13/logs/error_log
* <!--#exec cmd="openfds" --> from an .shtml page
-> crw-rw-rw- 1 root wheel 2, 2 /dev/fd/0
-> p--------- 0 root wheel 0 /dev/fd/1
-> -rw-r----- 1 martin kraemer 7719 /dev/fd/2 == /tmp/apa13/logs/error_log
* <?php system("openfds"); ?>
-> crw-rw-rw- 1 root wheel 2, 2 /dev/fd/0
-> p--------- 0 root wheel 0 /dev/fd/1
-> -rw-r----- 1 martin kraemer 7719 /dev/fd/2 == /tmp/apa13/logs/error_log
-> -rw-r----- 1 martin kraemer 7719 /dev/fd/15 == /tmp/apa13/logs/error_log
* piped logs work too.
==================================================================================
To recap,
* Apache by itself does not leak fd's, because its ap_cleanup_for_exec() API
has made sure (for ages already) that nothing is passed to exec'ed processes.
* "certain 3rd party modules" do not use the API correctly (either because
they don't know better, or because they wanted top use existing unsafe
functions like system() and popen()) and do not invoke ap_cleanup_for_exec(),
thus leaking fd's.
The leaked fd's include the server's listener sockets, the accept()ed
request sockets, the access_log file, several lock files.
* By using FD_CLOEXEC with the new ap_note_cleanups_for_*_ex() functions,
we can proactively plug these leakages, even though it would be better if
the 3rd party module authors would "do it right" in _their_ code.
* By showing that is actually works, I want to encourage the maintainers
of the other OS platforms (Win32, Netware, TPF, OS/2, Cygwin) to test
their platform for a feature analogous to POSIX's FD_CLOEXEC, and add it
to the function main/alloc.c::ap_close_fd_on_exec().
CAVEAT
The appended patch changes the semantics of the two functions ap_popenf()
and ap_psocket(), by defaulting to creating fd's which have the FD_CLOEXEC
feature set (where the OS supports it). I checked the occurrences of these
two functions, and inside the Apache source distro they are only used
in a context where this behavior is desired.
However, there are many 3rd party modules out there, and if they use
these functions they may break.
So: how to proceed?
- create a new set of ap_popenf_autoclosing() and ap_psocket_autoclosing()?
- change the API and update the new_features_1_3 doc file? ;-)
- other proposals?
Have fun,
Martin
PS: I append the code for the openfds cgi program as well. It is based on
a very ancient hack, so bear with me if the code is not very readable.
What it does: it loops over all open fd's and tries to describe their
properties in a format similar to 'ls -l'. For regular files, it also tries
to determine the path name of the file by invoking mount(1) and find(1).
That makes it very easy to understand to what open file the fd refers.
---snip---
Now your patch answers my main question, since you created a new
ap_popenf_ex() (== ap_popenf_autoclosing()).
But in addition to ap_popenf_ex(), my patch, which I'll have to
rework of course, also plugs the socket fd leakage.
So, do you suggest we should do the same with ap_psocket_ex()?
Martin
--
<Martin.Kraemer@Fujitsu-Siemens.com> | Fujitsu Siemens
Fon: +49-89-636-46021, FAX: +49-89-636-47655 | 81730 Munich, Germany
|