*) 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?)

PR:
Obtained from:
Submitted by:
Reviewed by:	Martin

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/1.3.x@97798 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/CHANGES b/src/CHANGES
index a05c482d66082c78994b2ed6642552265cd7ff74..6d561273bf36be46b6a7ec2d255496c21fe03eac 100644 (file)
--- a/src/CHANGES
+++ b/src/CHANGES
@@ -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>]
 

diff --git a/src/include/ap_alloc.h b/src/include/ap_alloc.h
index 1e944f8ec982a3aac9bfe07c81ec6acfbc5b73a9..738f9d441bb2b154d8ec0042d55bdb265efedfa9 100644 (file)
--- a/src/include/ap_alloc.h
+++ b/src/include/ap_alloc.h
@@ -291,11 +291,19 @@ API_EXPORT(void) ap_overlap_tables(table *a, const table *b, unsigned flags);
  * 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(FILE *) ap_pfdopen(struct pool *, int fd, const char *fmode);
 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 @@ API_EXPORT(int) ap_spawn_child(pool *, int (*)(void *, child_info *),
                                   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() */

diff --git a/src/include/ap_mmn.h b/src/include/ap_mmn.h
index e034392024ddfdc8bd8dfe9ca5f944b6a2cdd336..1043634d0f1b3473209d8cd3d112bdf28f967d48 100644 (file)
--- a/src/include/ap_mmn.h
+++ b/src/include/ap_mmn.h
@@ -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)          \

diff --git a/src/main/alloc.c b/src/main/alloc.c
index fcadb6421f5a18c551eb8e20aebff83cfb57bcec..55ebe111364259f6f9aeff1c2e9bd55f10109f2f 100644 (file)
--- a/src/main/alloc.c
+++ b/src/main/alloc.c
@@ -1682,8 +1682,10 @@ struct cleanup {
     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 @@ API_EXPORT(void) ap_register_cleanup(pool *p, void *data, void (*plain_cleanup)
     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 @@ API_EXPORT_NONSTD(void) ap_null_cleanup(void *data)
  * 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 @@ static void file_child_cleanup(void *fpv)
 {
     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 @@ static void socket_cleanup(void *fdv)
 {
     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 @@ API_EXPORT(int) ap_bspawn_child(pool *p, int (*func) (void *, child_info *), voi
 
     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

diff --git a/src/main/http_log.c b/src/main/http_log.c
index 174cc5466b0a0f5ffe0423a77c6ee38018bfbc49..de0023512156d5e68bae81760244c4af9200175c 100644 (file)
--- a/src/main/http_log.c
+++ b/src/main/http_log.c
@@ -691,6 +691,13 @@ static void piped_log_cleanup_for_exec(void *data)
     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 @@ API_EXPORT(piped_log *) ap_open_piped_log(pool *p, const char *program)
        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);