Re: cvs commit: httpd-2.0/server/mpm/worker pod.c
On Thu, 20 Mar 2003, Aaron Bannert wrote: Hi, Log: SECURITY: Eliminated leaks of several file descriptors to child processes, such as CGI scripts. [...] apr_sockaddr_info_get((*pod)-sa, ap_listeners-bind_addr-hostname, APR_UNSPEC, ap_listeners-bind_addr-port, 0, p); +/* close these before exec. */ +apr_file_unset_inherit((*pod)-pod_in); +apr_file_unset_inherit((*pod)-pod_out); + return APR_SUCCESS; The PODs in the worker MPM are getting closed and the parent is then unable to kill its children when it needs to (don't you love how morbid that sounds?). I see one of these every second in the error log: [Thu Mar 20 18:09:25 2003] [warn] (32)Broken pipe: write pipe_of_death Since the unset_inherit() is being called from the open_logs hook, it's happening in the parent process, which means that the fork for the children is going to kill them off. We need to unset the inherit *after* we are running in the child. I am not really familiar with worker but what about this (untested) ? does a) pod work again and b) are the fd's still closed on exec ? --- httpd-2.0/server/mpm/worker/pod.c.orig Fri Mar 21 08:20:07 2003 +++ httpd-2.0/server/mpm/worker/pod.c Fri Mar 21 08:20:27 2003 @@ -75,10 +75,6 @@ apr_file_pipe_timeout_set((*pod)-pod_in, 0); */ (*pod)-p = p; - -/* close these before exec. */ -apr_file_unset_inherit((*pod)-pod_in); -apr_file_unset_inherit((*pod)-pod_out); return APR_SUCCESS; } --- httpd-2.0/server/mpm/worker/worker.c.orig Fri Mar 21 08:20:31 2003 +++ httpd-2.0/server/mpm/worker/worker.cFri Mar 21 08:21:48 2003 @@ -1387,6 +1387,10 @@ #endif RAISE_SIGSTOP(MAKE_CHILD); +/* close these before exec. */ +apr_file_unset_inherit((*pod)-pod_in); +apr_file_unset_inherit((*pod)-pod_out); + apr_signal(SIGTERM, just_die); child_main(slot); Else we would need s.th. in apr that only sets child_cleanup_fn and not both I think ... -- Bjoern A. Zeeb bzeeb at Zabbadoz dot NeT 56 69 73 69 74 http://www.zabbadoz.net/
Re: [PATCH] resend: fix fd leaks
On Thu, 20 Mar 2003, Joe Orton wrote: Hi, Submitted by: Christian Kratzer, Bjoern A. Zeeb +1 here. I have one comment; please *don't* simply delete those lines from server/log.c, modules/mappers/mod_rewrite.c and, of course, modules/loggers/mod_log_config.c. Please comment them out with /* XXX: this would be required in the Win32 parent */ I had just tested wrowes latest apr commits with httpd-2.0 cvs co and Joe's latest patch and the open fds are gone and apache seems to work fine. I think we should get this in now with the requested comments. -- Bjoern A. Zeeb bzeeb at Zabbadoz dot NeT 56 69 73 69 74 http://www.zabbadoz.net/
discussion on fd leak problematic
what I know. It is fully impractical to loop through fd 3..rlim.rlim_cur and close all of them in p.ex. suexec as some systems such as AIX have a huge per-process open file limit.[12] TODO: - APR: check FD_CLOEXEC and check if it would be possible to always register a child cleanup_fn[9] as this (yet) is only used for execs on unix. HTTPD: we need further discussion on wheter to explicitly call apr_file_inherit_unset() on those. This can be best decided together with apr people as they might better know if the default behavior will change/break somewhen in future again. check wether files etc. are opened with too many priviledges and if those orivs can be withdrwn w/o breaking the code in other places. See p.ex. [11]. Please have a look at the code as you all should know it far better than me and discuss those points. LAST: - some more comments/details might either be in the [EMAIL PROTECTED] account or are also archived on http://sources.zabbadoz.net/patches/apache/ along with some sample CGIs etc. that show some of the problematics described above. References: - [1] bug report by Christian Kratzer http://nagoya.apache.org/bugzilla/show_bug.cgi?id=17206 [2] vuln-dev posting by Steve Grubb http://www.securityfocus.com/archive/82/312747/2003-02-20/2003-02-26/0 or http://marc.theaimsgroup.com/?l=vuln-devm=104585997219471w=2 [3] apr 24^2 vs. 242 bug http://cvs.apache.org/viewcvs.cgi/apr/include/arch/unix/Attic/inherit.h.diff?r1=1.10r2=1.11diff_format=h [4] httpd inherit commits which cause problmes http://cvs.apache.org/viewcvs.cgi/httpd-2.0/server/log.c.diff?r1=1.94r2=1.95diff_format=h [5] redhat bugzilla report https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=82142 [6] apr patch by Joe Orton http://marc.theaimsgroup.com/?t=10468821818r=1w=2 [7] httpd diff by Joe Orton http://marc.theaimsgroup.com/?t=10469649342r=1w=2 resend: http://marc.theaimsgroup.com/?l=apache-httpd-devm=104755245104369w=2 [8] apr pipe inherit commit http://cvs.apache.org/viewcvs.cgi/apr/file_io/unix/pipe.c.diff?r1=1.61r2=1.62diff_format=h [9] suggestion for registering child cleanup_fn for pipes. http://sources.zabbadoz.net/patches/apache/pipe-20030302-01.diff [10]suggestion for a fix that works for me for fd leaks of logs included in http://sources.zabbadoz.net/patches/apache/bug-report.txt [11]suggestion for not opening error_logs readable http://marc.theaimsgroup.com/?l=apache-httpd-devm=104758986221626w=2 http://sources.zabbadoz.net/patches/apache/httpd-2.0-error-log-open-for-reading-20030313-01.diff [12]postfix master source code by Wietse Venema # End; - -- Bjoern A. Zeeb bzeeb at Zabbadoz dot NeT 56 69 73 69 74 http://www.zabbadoz.net/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.1 (FreeBSD) iD8DBQE+cllYIcUJFg5KeHURAnRgAJwOrTEmFyVaCEERYoWgV9kJk9bUJwCeK5kf 6f4FNCdWnC+3RnvzUHCKE+8= =rk5r -END PGP SIGNATURE-
need to read error_logs from httpd ?
Hi,
is there a need that we open error_log for reading from within httpd ?
--- httpd-2.0/server/log.c.orig Thu Mar 13 19:47:20 2003
+++ httpd-2.0/server/log.c Thu Mar 13 19:47:48 2003
@@ -313,7 +313,7 @@
return DONE;
}
if ((rc = apr_file_open(s-error_log, fname,
- APR_APPEND | APR_READ | APR_WRITE | APR_CREATE,
+ APR_APPEND | APR_WRITE | APR_CREATE,
APR_OS_DEFAULT, p)) != APR_SUCCESS) {
ap_log_error(APLOG_MARK, APLOG_STARTUP, rc, NULL,
%s: could not open error log file %s.,
--
Bjoern A. Zeeb bzeeb at Zabbadoz dot NeT
56 69 73 69 74 http://www.zabbadoz.net/
Re: [PATCH] fix fd leaks
On Thu, 6 Mar 2003, Joe Orton wrote:
Hi,
Submitted by: Christian Kratzer, Bjoern A. Zeeb
...
-
-apr_file_inherit_set(s-error_log);
}
so now we are back to that point that needs further discussing.
Should we simply remove apr_file_inherit_set or explicitly call
apr_file_unset_inherit ? Simply removing works as long as defaults in
APR remain but there is still p.ex.:
if (!(flag APR_FILE_NOCLEANUP)){
that can again prevent the child_cleanup_fn from being set (seems to
unused in open at the moment).
It seems to me that the apr_file_inherit_set had been in because up
to including httpd 2.0.39 a bug in apr prevented _set/_unset from
working as expected (the 2^24 vs. 224 flag check).
Thus _set did the correct thing that _unset should have done.
If the _sets had been in for some specific reason we sould perhaps
use _unset.
So there needs to be further tracking on why the _sets had been
committed to apache code.
Comments ?
--
Bjoern A. Zeeb bzeeb at Zabbadoz dot NeT
56 69 73 69 74 http://www.zabbadoz.net/
