You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jim Jagielski <ji...@jaguNET.com> on 1997/11/05 01:54:38 UTC

[PATCH] Make FLOCK mutex work PR#1056

This should close out that PR... The child does the open() before
it gives up root, thus making a DoS easier to guard against
(the lockfile should be located only where root can read/write
and not on a NFS point).

Index: src/main/http_main.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_main.c,v
retrieving revision 1.243
diff -u -r1.243 http_main.c
--- http_main.c	1997/11/03 10:11:42	1.243
+++ http_main.c	1997/11/05 00:52:35
@@ -276,6 +276,8 @@
 
 #define accept_mutex_cleanup()
 
+#define accept_mutex_child_init(x)
+
 static void accept_mutex_init(pool *p)
 {
     ptrdiff_t old;
@@ -347,6 +349,8 @@
     }
 }
 
+#define accept_mutex_child_init(x)
+
 static void accept_mutex_init(pool *p)
 {
     pthread_mutexattr_t mattr;
@@ -435,6 +439,7 @@
     semctl(sem_id, 0, IPC_RMID, ick);
 }
 
+#define accept_mutex_child_init(x)
 
 static void accept_mutex_init(pool *p)
 {
@@ -508,6 +513,8 @@
 
 #define accept_mutex_cleanup()
 
+#define accept_mutex_child_init(x)
+
 /*
  * Initialize mutex lock.
  * Must be safe to call this on a restart.
@@ -563,22 +570,33 @@
 
 static int lock_fd = -1;
 
-#define accept_mutex_cleanup()
+static void accept_mutex_cleanup(void)
+{
+    unlink(lock_fname);
+}
 
 /*
  * Initialize mutex lock.
- * Must be safe to call this on a restart.
+ * Done by each child at it's birth
  */
-static void accept_mutex_init(pool *p)
+static void accept_mutex_child_init(pool *p)
 {
 
-    expand_lock_fname(p);
-    lock_fd = popenf(p, lock_fname, O_CREAT | O_WRONLY | O_EXCL, 0644);
+    lock_fd = popenf(p, lock_fname, O_CREAT | O_WRONLY, 0600);
     if (lock_fd == -1) {
 	aplog_error(APLOG_MARK, APLOG_EMERG, server_conf,
 		    "Cannot open lock file: %s\n", lock_fname);
 	exit(1);
     }
+}
+/*
+ * Initialize mutex lock.
+ * Must be safe to call this on a restart.
+ */
+static void accept_mutex_init(pool *p)
+{
+
+    expand_lock_fname(p);
     unlink(lock_fname);
 }
 
@@ -614,6 +632,7 @@
  * the sockets. */
 #define NO_SERIALIZED_ACCEPT
 #define accept_mutex_cleanup()
+#define accept_mutex_child_init(x)
 #define accept_mutex_init(x)
 #define accept_mutex_on()
 #define accept_mutex_off()
@@ -2623,6 +2642,7 @@
 
     /* needs to be done before we switch UIDs so we have permissions */
     reopen_scoreboard(pconf);
+    SAFE_ACCEPT(accept_mutex_child_init(pconf));
 
 #ifdef MPE
     /* Only try to switch if we're running as MANAGER.SYS */
-- 
====================================================================
      Jim Jagielski            |       jaguNET Access Services
     jim@jaguNET.com           |       http://www.jaguNET.com/
            "Look at me! I'm wearing a cardboard belt!"

Re: [PATCH] Make FLOCK mutex work PR#1056

Posted by Marc Slemko <ma...@worldgate.com>.
On Tue, 4 Nov 1997, Dean Gaudet wrote:

> 
> 
> On Tue, 4 Nov 1997, Jim Jagielski wrote:
> 
> > -    lock_fd = popenf(p, lock_fname, O_CREAT | O_WRONLY | O_EXCL, 0644);
> > +    lock_fd = popenf(p, lock_fname, O_CREAT | O_WRONLY, 0600);
> 
> Make the parent do an O_CREAT to create the file, and remove O_CREAT from
> the children ... to make the window in which an attacker can stuff a
> symlink in the way smaller... also helps make sure we notice if some dolt
> removes the lock file (the children would all exit, the server would chew
> lots of cpu spawning new children :).  It looks fine either way though. 

The parent should also have a O_EXCL.


Re: [PATCH] Make FLOCK mutex work PR#1056

Posted by Dean Gaudet <dg...@arctic.org>.

On Tue, 4 Nov 1997, Jim Jagielski wrote:

> -    lock_fd = popenf(p, lock_fname, O_CREAT | O_WRONLY | O_EXCL, 0644);
> +    lock_fd = popenf(p, lock_fname, O_CREAT | O_WRONLY, 0600);

Make the parent do an O_CREAT to create the file, and remove O_CREAT from
the children ... to make the window in which an attacker can stuff a
symlink in the way smaller... also helps make sure we notice if some dolt
removes the lock file (the children would all exit, the server would chew
lots of cpu spawning new children :).  It looks fine either way though. 

Dean