You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ji...@apache.org on 2002/12/08 20:09:55 UTC
cvs commit: apache-1.3/src/main alloc.c http_log.c
jim 2002/12/08 11:09:55
Modified: src CHANGES
src/include ap_alloc.h ap_mmn.h
src/main alloc.c http_log.c
Log:
*) Added new ap_register_cleanup_ex() API function which allows
for a "magic" cleanup function to be run at register time
rather than at cleanup time. Also added the
ap_note_cleanups_for_(socket|fd|file)_ex() API functions
which allows for control over whether that magic cleanup
should be called or not. This does not change the default
behavior of the non-"ex" function (eg: ap_register_cleanup).
At present, the magic cleanup is simply code that performs
a CLOSEXEC, but that can be modified (hmmm... maybe an
API issue?)
Reviewed by: Martin
Revision Changes Path
1.1864 +9 -0 apache-1.3/src/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/apache-1.3/src/CHANGES,v
retrieving revision 1.1863
retrieving revision 1.1864
diff -u -r1.1863 -r1.1864
--- CHANGES 25 Nov 2002 10:31:30 -0000 1.1863
+++ CHANGES 8 Dec 2002 19:09:54 -0000 1.1864
@@ -1,5 +1,14 @@
Changes with Apache 1.3.28
+ *) Added new ap_register_cleanup_ex() API function which allows
+ for a "magic" cleanup function to be run at register time
+ rather than at cleanup time. Also added the
+ ap_note_cleanups_for_(socket|fd|file)_ex() API functions
+ which allows for control over whether that magic cleanup
+ should be called or not. This does not change the default
+ behavior of the non-"ex" function (eg: ap_register_cleanup).
+ [Jim Jagielski, concept by Ben Laurie]
+
*) PORT: Take advantage of OpenBSD's arc4random() function for the
initial secret [Henning Brauer <hb-apache-dev at bsws.de>]
1.80 +14 -2 apache-1.3/src/include/ap_alloc.h
Index: ap_alloc.h
===================================================================
RCS file: /home/cvs/apache-1.3/src/include/ap_alloc.h,v
retrieving revision 1.79
retrieving revision 1.80
diff -u -r1.79 -r1.80
--- ap_alloc.h 13 Mar 2002 21:05:28 -0000 1.79
+++ ap_alloc.h 8 Dec 2002 19:09:55 -0000 1.80
@@ -291,11 +291,19 @@
* NB any code which invokes register_cleanup or kill_cleanup directly
* is a critical section which should be guarded by block_alarms() and
* unblock_alarms() below...
+ *
+ * ap_register_cleanup_ex provided to allow for an optional "cleanup"
+ * to be run at call-time for things like setting CLOSEXEC flags
+ * on fd's or whatever else may make sense.
*/
API_EXPORT(void) ap_register_cleanup(pool *p, void *data,
- void (*plain_cleanup) (void *),
- void (*child_cleanup) (void *));
+ void (*plain_cleanup) (void *),
+ void (*child_cleanup) (void *));
+API_EXPORT(void) ap_register_cleanup_ex(pool *p, void *data,
+ void (*plain_cleanup) (void *),
+ void (*child_cleanup) (void *),
+ int (*magic_cleanup) (void *));
API_EXPORT(void) ap_kill_cleanup(pool *p, void *data, void (*plain_cleanup) (void *));
API_EXPORT(void) ap_run_cleanup(pool *p, void *data, void (*cleanup) (void *));
@@ -331,13 +339,16 @@
API_EXPORT(int) ap_popenf(struct pool *, const char *name, int flg, int mode);
API_EXPORT(void) ap_note_cleanups_for_file(pool *, FILE *);
+API_EXPORT(void) ap_note_cleanups_for_file_ex(pool *, FILE *, int);
API_EXPORT(void) ap_note_cleanups_for_fd(pool *, int);
+API_EXPORT(void) ap_note_cleanups_for_fd_ex(pool *, int, int);
#ifdef WIN32
API_EXPORT(void) ap_note_cleanups_for_h(pool *, HANDLE);
#endif
API_EXPORT(void) ap_kill_cleanups_for_fd(pool *p, int fd);
API_EXPORT(void) ap_note_cleanups_for_socket(pool *, int);
+API_EXPORT(void) ap_note_cleanups_for_socket_ex(pool *, int, int);
API_EXPORT(void) ap_kill_cleanups_for_socket(pool *p, int sock);
API_EXPORT(int) ap_psocket(pool *p, int, int, int);
API_EXPORT(int) ap_pclosesocket(pool *a, int sock);
@@ -384,6 +395,7 @@
void *, enum kill_conditions,
FILE **pipe_in, FILE **pipe_out,
FILE **pipe_err);
+int ap_close_fd_on_exec(int fd);
/* magic numbers --- min free bytes to consider a free pool block useable,
* and the min amount to allocate if we have to go to malloc() */
1.61 +5 -1 apache-1.3/src/include/ap_mmn.h
Index: ap_mmn.h
===================================================================
RCS file: /home/cvs/apache-1.3/src/include/ap_mmn.h,v
retrieving revision 1.60
retrieving revision 1.61
diff -u -r1.60 -r1.61
--- ap_mmn.h 18 Jun 2002 01:21:46 -0000 1.60
+++ ap_mmn.h 8 Dec 2002 19:09:55 -0000 1.61
@@ -237,6 +237,10 @@
* structure
* 19990320.12 - add ap_getline(), ap_get_chunk_size()
* 19990320.13 - add ap_strtol()
+ * 19990320.14 - add ap_register_cleanup_ex(),
+ * ap_note_cleanups_for_fd_ex(),
+ * ap_note_cleanups_for_socket_ex() and
+ * ap_note_cleanups_for_file_ex()
*/
#define MODULE_MAGIC_COOKIE 0x41503133UL /* "AP13" */
@@ -244,7 +248,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 19990320
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 13 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 14 /* 0...n */
/* Useful for testing for features. */
#define AP_MODULE_MAGIC_AT_LEAST(major,minor) \
1.131 +86 -8 apache-1.3/src/main/alloc.c
Index: alloc.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/main/alloc.c,v
retrieving revision 1.130
retrieving revision 1.131
diff -u -r1.130 -r1.131
--- alloc.c 14 Nov 2002 16:38:31 -0000 1.130
+++ alloc.c 8 Dec 2002 19:09:55 -0000 1.131
@@ -1682,8 +1682,10 @@
struct cleanup *next;
};
-API_EXPORT(void) ap_register_cleanup(pool *p, void *data, void (*plain_cleanup) (void *),
- void (*child_cleanup) (void *))
+API_EXPORT(void) ap_register_cleanup_ex(pool *p, void *data,
+ void (*plain_cleanup) (void *),
+ void (*child_cleanup) (void *),
+ int (*magic_cleanup) (void *))
{
struct cleanup *c = (struct cleanup *) ap_palloc(p, sizeof(struct cleanup));
c->data = data;
@@ -1691,6 +1693,18 @@
c->child_cleanup = child_cleanup;
c->next = p->cleanups;
p->cleanups = c;
+ if(magic_cleanup) {
+ if(!magic_cleanup(data))
+ ap_log_error(APLOG_MARK, APLOG_WARNING, NULL,
+ "exec() may not be safe");
+ }
+}
+
+API_EXPORT(void) ap_register_cleanup(pool *p, void *data,
+ void (*plain_cleanup) (void *),
+ void (*child_cleanup) (void *))
+{
+ ap_register_cleanup_ex(p, data, plain_cleanup, child_cleanup, NULL);
}
API_EXPORT(void) ap_kill_cleanup(pool *p, void *data, void (*cleanup) (void *))
@@ -1771,14 +1785,48 @@
* generic cleanup interface.
*/
+int ap_close_fd_on_exec(int fd)
+{
+#if defined(F_SETFD) && defined(FD_CLOEXEC)
+ /* Protect the fd so that it will not be inherited by child processes */
+ if(fcntl(fd, F_SETFD, FD_CLOEXEC) < 0) {
+ ap_log_error(APLOG_MARK, APLOG_ERR, NULL,
+ "fcntl(%d, F_SETFD, FD_CLOEXEC) failed", fd);
+ return 0;
+ }
+
+ return 1;
+#else
+ return 0;
+#endif
+}
+
static void fd_cleanup(void *fdv)
{
close((int) (long) fdv);
}
+static int fd_magic_cleanup(void *fdv)
+{
+ return ap_close_fd_on_exec((int) (long) fdv);
+}
+
+API_EXPORT(void) ap_note_cleanups_for_fd_ex(pool *p, int fd, int domagic)
+{
+ if (domagic) {
+ ap_register_cleanup_ex(p, (void *) (long) fd, fd_cleanup, fd_cleanup,
+ fd_magic_cleanup);
+ } else {
+ /* basically ap_register_cleanup but save the possible
+ overhead of an extraneous function call */
+ ap_register_cleanup_ex(p, (void *) (long) fd, fd_cleanup, fd_cleanup,
+ NULL);
+ }
+}
+
API_EXPORT(void) ap_note_cleanups_for_fd(pool *p, int fd)
{
- ap_register_cleanup(p, (void *) (long) fd, fd_cleanup, fd_cleanup);
+ ap_note_cleanups_for_fd_ex(p, fd, 0);
}
API_EXPORT(void) ap_kill_cleanups_for_fd(pool *p, int fd)
@@ -1860,10 +1908,25 @@
{
close(fileno((FILE *) fpv));
}
+static int file_magic_cleanup(void *fpv)
+{
+ return ap_close_fd_on_exec(fileno((FILE *) fpv));
+}
+
+API_EXPORT(void) ap_note_cleanups_for_file_ex(pool *p, FILE *fp, int domagic)
+{
+ if (domagic) {
+ ap_register_cleanup_ex(p, (void *) fp, file_cleanup, file_child_cleanup,
+ file_magic_cleanup);
+ } else {
+ ap_register_cleanup_ex(p, (void *) fp, file_cleanup, file_child_cleanup,
+ NULL);
+ }
+}
API_EXPORT(void) ap_note_cleanups_for_file(pool *p, FILE *fp)
{
- ap_register_cleanup(p, (void *) fp, file_cleanup, file_child_cleanup);
+ ap_note_cleanups_for_file_ex(p, fp, 0);
}
API_EXPORT(FILE *) ap_pfopen(pool *a, const char *name, const char *mode)
@@ -1974,10 +2037,25 @@
{
closesocket((int) (long) fdv);
}
+static int socket_magic_cleanup(void *fpv)
+{
+ return ap_close_fd_on_exec(fileno((FILE *) fpv));
+}
+
+API_EXPORT(void) ap_note_cleanups_for_socket_ex(pool *p, int fd, int domagic)
+{
+ if (domagic) {
+ ap_register_cleanup_ex(p, (void *) (long) fd, socket_cleanup,
+ socket_cleanup, socket_magic_cleanup);
+ } else {
+ ap_register_cleanup_ex(p, (void *) (long) fd, socket_cleanup,
+ socket_cleanup, NULL);
+ }
+}
API_EXPORT(void) ap_note_cleanups_for_socket(pool *p, int fd)
{
- ap_register_cleanup(p, (void *) (long) fd, socket_cleanup, socket_cleanup);
+ ap_note_cleanups_for_socket_ex(p, fd, 0);
}
API_EXPORT(void) ap_kill_cleanups_for_socket(pool *p, int sock)
@@ -2603,19 +2681,19 @@
if (pipe_out) {
*pipe_out = ap_bcreate(p, B_RD);
- ap_note_cleanups_for_fd(p, fd_out);
+ ap_note_cleanups_for_fd_ex(p, fd_out, 0);
ap_bpushfd(*pipe_out, fd_out, fd_out);
}
if (pipe_in) {
*pipe_in = ap_bcreate(p, B_WR);
- ap_note_cleanups_for_fd(p, fd_in);
+ ap_note_cleanups_for_fd_ex(p, fd_in, 0);
ap_bpushfd(*pipe_in, fd_in, fd_in);
}
if (pipe_err) {
*pipe_err = ap_bcreate(p, B_RD);
- ap_note_cleanups_for_fd(p, fd_err);
+ ap_note_cleanups_for_fd_ex(p, fd_err, 0);
ap_bpushfd(*pipe_err, fd_err, fd_err);
}
#endif
1.95 +9 -1 apache-1.3/src/main/http_log.c
Index: http_log.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/main/http_log.c,v
retrieving revision 1.94
retrieving revision 1.95
diff -u -r1.94 -r1.95
--- http_log.c 19 May 2002 04:55:38 -0000 1.94
+++ http_log.c 8 Dec 2002 19:09:55 -0000 1.95
@@ -691,6 +691,13 @@
close(pl->fds[1]);
}
+static int piped_log_magic_cleanup(void *data)
+{
+ piped_log *pl = data;
+
+ /* Yes, I _do_ mean a binary and */
+ return ap_close_fd_on_exec(pl->fds[0]) & ap_close_fd_on_exec(pl->fds[1]);
+}
API_EXPORT(piped_log *) ap_open_piped_log(pool *p, const char *program)
{
@@ -707,7 +714,8 @@
errno = save_errno;
return NULL;
}
- ap_register_cleanup(p, pl, piped_log_cleanup, piped_log_cleanup_for_exec);
+ ap_register_cleanup_ex(p, pl, piped_log_cleanup, piped_log_cleanup_for_exec,
+ piped_log_magic_cleanup);
if (piped_log_spawn(pl) == -1) {
int save_errno = errno;
ap_kill_cleanup(p, pl, piped_log_cleanup);
Re: cvs commit: apache-1.3/src/main alloc.c http_log.c
Posted by Justin Erenkrantz <je...@apache.org>.
--On Sunday, December 8, 2002 7:09 PM +0000 jim@apache.org wrote:
> @@ -1691,6 +1693,18 @@
> c->child_cleanup = child_cleanup;
> c->next = p->cleanups;
> p->cleanups = c;
> + if(magic_cleanup) {
> + if(!magic_cleanup(data))
> + ap_log_error(APLOG_MARK, APLOG_WARNING, NULL,
> + "exec() may not be safe");
> + }
> +}
Your editor seems to have been tab-happy.
> +API_EXPORT(void) ap_note_cleanups_for_fd_ex(pool *p, int fd, int
> domagic)
> +{
> + if (domagic) {
> + ap_register_cleanup_ex(p, (void *) (long) fd,
> fd_cleanup, fd_cleanup,
> + fd_magic_cleanup);
> + } else {
> + /* basically ap_register_cleanup but save the possible
> + overhead of an extraneous function call */
> + ap_register_cleanup_ex(p, (void *) (long) fd,
> fd_cleanup, fd_cleanup,
> + NULL);
> + }
> +}
Would it be better to simplify as:
ap_register_cleanup_ex(p, (void *) (long) fd, fd_cleanup, fd_cleanup,
domagic ? fd_magic_cleanup : NULL);
(Appears several other places, too.)
> API_EXPORT(void) ap_kill_cleanups_for_socket(pool *p, int sock)
> @@ -2603,19 +2681,19 @@
>
> if (pipe_out) {
> *pipe_out = ap_bcreate(p, B_RD);
> - ap_note_cleanups_for_fd(p, fd_out);
> + ap_note_cleanups_for_fd_ex(p, fd_out, 0);
Why switch this? Wouldn't it have been the same thing, or are you
trying to save extra function calls whereever possible? -- justin