httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Kraemer <mar...@apache.org>
Subject Re: [PATCH] Have logfiles closed on exec
Date Wed, 11 Dec 2002 16:00:13 GMT
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

Mime
View raw message