r17569: Make 'max smbd processes' more robust. Counting on the child to decrement a

tdb entry is not the most reliable way to count children correctly.

This increments the number of children after a fork and decrements it upon
SIGCLD. I'm keeping a list of children just for consistency checks, so that we
at least get a debug level 0 message if something goes wrong.

Volker
(This used to be commit eb45de167d24d07a218307ec5a48c0029ec097c6)

4 files changed, 78 insertions, 88 deletions

diff --git a/source3/lib/dummysmbd.c b/source3/lib/dummysmbd.c
index 087de2fe25..5bb71e120e 100644
--- a/source3/lib/dummysmbd.c
+++ b/source3/lib/dummysmbd.c
@@ -24,11 +24,6 @@
 
 #include "includes.h"
 
-void decrement_smbd_process_count( void )
-{
-	return;
-}
-
 int find_service(fstring service)
 {
 	return -1;
diff --git a/source3/lib/util.c b/source3/lib/util.c
index f985c57ed9..ef954015d6 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -1594,9 +1594,6 @@ void smb_panic(const char *const why)
 		    (unsigned long long)sys_getpid(), why));
 	log_stack_trace();
 
-	/* only smbd needs to decrement the smbd counter in connections.tdb */
-	decrement_smbd_process_count();
-
 	cmd = lp_panic_action();
 	if (cmd && *cmd) {
 		DEBUG(0, ("smb_panic(): calling panic action [%s]\n", cmd));
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index f8c66d93ea..a202c1fa87 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -1032,60 +1032,6 @@ static int construct_reply(char *inbuf,char *outbuf,int size,int bufsize)
 }
 
 /****************************************************************************
- Keep track of the number of running smbd's. This functionality is used to
- 'hard' limit Samba overhead on resource constrained systems. 
-****************************************************************************/
-
-static BOOL process_count_update_successful = False;
-
-static int32 increment_smbd_process_count(void)
-{
-	int32 total_smbds;
-
-	if (lp_max_smbd_processes()) {
-		total_smbds = 0;
-		if (tdb_change_int32_atomic(conn_tdb_ctx(), "INFO/total_smbds", &total_smbds, 1) == -1)
-			return 1;
-		process_count_update_successful = True;
-		return total_smbds + 1;
-	}
-	return 1;
-}
-
-void decrement_smbd_process_count(void)
-{
-	int32 total_smbds;
-
-	if (lp_max_smbd_processes() && process_count_update_successful) {
-		total_smbds = 1;
-		tdb_change_int32_atomic(conn_tdb_ctx(), "INFO/total_smbds", &total_smbds, -1);
-	}
-}
-
-static BOOL smbd_process_limit(void)
-{
-	int32  total_smbds;
-	
-	if (lp_max_smbd_processes()) {
-
-		/* Always add one to the smbd process count, as exit_server() always
-		 * subtracts one.
-		 */
-
-		if (!conn_tdb_ctx()) {
-			DEBUG(0,("smbd_process_limit: max smbd processes parameter set with status parameter not \
-set. Ignoring max smbd restriction.\n"));
-			return False;
-		}
-
-		total_smbds = increment_smbd_process_count();
-		return total_smbds > lp_max_smbd_processes();
-	}
-	else
-		return False;
-}
-
-/****************************************************************************
  Process an smb from the client
 ****************************************************************************/
 
@@ -1103,8 +1049,8 @@ static void process_smb(char *inbuf, char *outbuf)
 		deny parameters before doing any parsing of the packet
 		passed to us by the client.  This prevents attacks on our
 		parsing code from hosts not in the hosts allow list */
-		if (smbd_process_limit() ||
-				!check_access(smbd_server_fd(), lp_hostsallow(-1), lp_hostsdeny(-1))) {
+		if (!check_access(smbd_server_fd(), lp_hostsallow(-1),
+				  lp_hostsdeny(-1))) {
 			/* send a negative session response "not listening on calling name" */
 			static unsigned char buf[5] = {0x83, 0, 0, 1, 0x81};
 			DEBUG( 1, ( "Connection denied from %s\n", client_addr() ) );
diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index edde12465f..460e2cc44b 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -36,6 +36,7 @@ extern struct auth_context *negprot_global_auth_context;
 extern pstring user_socket_options;
 extern SIG_ATOMIC_T got_sig_term;
 extern SIG_ATOMIC_T reload_after_sighup;
+static SIG_ATOMIC_T got_sig_cld;
 
 #ifdef WITH_DFS
 extern int dcelogin_atmost_once;
@@ -93,6 +94,15 @@ static void sig_hup(int sig)
 }
 
 /****************************************************************************
+ Catch a sigcld
+****************************************************************************/
+static void sig_cld(int sig)
+{
+	got_sig_cld = 1;
+	sys_select_signal(SIGCLD);
+}
+
+/****************************************************************************
   Send a SIGTERM to our process group.
 *****************************************************************************/
 
@@ -189,6 +199,54 @@ static void msg_inject_fault(int msg_type, struct process_id src,
 }
 #endif /* DEVELOPER */
 
+struct child_pid {
+	struct child_pid *prev, *next;
+	pid_t pid;
+};
+
+static struct child_pid *children;
+static int num_children;
+
+static void add_child_pid(pid_t pid)
+{
+	struct child_pid *child;
+
+	if (lp_max_smbd_processes() == 0) {
+		/* Don't bother with the child list if we don't care anyway */
+		return;
+	}
+
+	child = SMB_MALLOC_P(struct child_pid);
+	if (child == NULL) {
+		DEBUG(0, ("Could not add child struct -- malloc failed\n"));
+		return;
+	}
+	child->pid = pid;
+	DLIST_ADD(children, child);
+	num_children += 1;
+}
+
+static void remove_child_pid(pid_t pid)
+{
+	struct child_pid *child;
+
+	if (lp_max_smbd_processes() == 0) {
+		/* Don't bother with the child list if we don't care anyway */
+		return;
+	}
+
+	for (child = children; child != NULL; child = child->next) {
+		if (child->pid == pid) {
+			struct child_pid *tmp = child;
+			DLIST_REMOVE(children, child);
+			SAFE_FREE(tmp);
+			num_children -= 1;
+			return;
+		}
+	}
+
+	DEBUG(0, ("Could not find child %d -- ignoring\n", (int)pid));
+}
 
 /****************************************************************************
  Have we reached the process limit ?
@@ -201,27 +259,7 @@ static BOOL allowable_number_of_smbd_processes(void)
 	if (!max_processes)
 		return True;
 
-	{
-		TDB_CONTEXT *tdb = conn_tdb_ctx();
-		int32 val;
-		if (!tdb) {
-			DEBUG(0,("allowable_number_of_smbd_processes: can't open connection tdb.\n" ));
-			return False;
-		}
-
-		val = tdb_fetch_int32(tdb, "INFO/total_smbds");
-		if (val == -1 && (tdb_error(tdb) != TDB_ERR_NOEXIST)) {
-			DEBUG(0,("allowable_number_of_smbd_processes: can't fetch INFO/total_smbds. Error %s\n",
-				tdb_errorstr(tdb) ));
-			return False;
-		}
-		if (val > max_processes) {
-			DEBUG(0,("allowable_number_of_smbd_processes: number of processes (%d) is over allowed limit (%d)\n",
-				val, max_processes ));
-			return False;
-		}
-	}
-	return True;
+	return num_children < max_processes;
 }
 
 /****************************************************************************
@@ -255,7 +293,7 @@ static BOOL open_sockets_smbd(BOOL is_daemon, BOOL interactive, const char *smb_
 #endif
 
 	/* Stop zombies */
-	CatchChild();
+	CatchSignal(SIGCLD, sig_cld);
 				
 	FD_ZERO(&listen_set);
 
@@ -392,6 +430,15 @@ static BOOL open_sockets_smbd(BOOL is_daemon, BOOL interactive, const char *smb_
 		/* Ensure we respond to PING and DEBUG messages from the main smbd. */
 		message_dispatch();
 
+		if (got_sig_cld) {
+			pid_t pid;
+			got_sig_cld = False;
+
+			while ((pid = sys_waitpid(-1, NULL, WNOHANG)) > 0) {
+				remove_child_pid(pid);
+			}
+		}
+
 		memcpy((char *)&lfds, (char *)&listen_set, 
 		       sizeof(listen_set));
 		
@@ -421,6 +468,7 @@ static BOOL open_sockets_smbd(BOOL is_daemon, BOOL interactive, const char *smb_
 		for( ; num > 0; num--) {
 			struct sockaddr addr;
 			socklen_t in_addrlen = sizeof(addr);
+			pid_t child = 0;
 
 			s = -1;
 			for(i = 0; i < num_sockets; i++) {
@@ -451,7 +499,8 @@ static BOOL open_sockets_smbd(BOOL is_daemon, BOOL interactive, const char *smb_
 				return True;
 			
 			if (allowable_number_of_smbd_processes() &&
-			    smbd_server_fd() != -1 && sys_fork()==0) {
+			    smbd_server_fd() != -1 &&
+			    ((child = sys_fork())==0)) {
 				/* Child code ... */
 				
 				/* close the listening socket(s) */
@@ -499,6 +548,10 @@ static BOOL open_sockets_smbd(BOOL is_daemon, BOOL interactive, const char *smb_
 
 			smbd_set_server_fd(-1);
 
+			if (child != 0) {
+				add_child_pid(child);
+			}
+
 			/* Force parent to check log size after
 			 * spawning child.  Fix from
 			 * klausr@ITAP.Physik.Uni-Stuttgart.De.  The
@@ -640,7 +693,6 @@ static void exit_server_common(enum server_exit_reason how,
 	yield_connection(NULL,"");
 
 	respond_to_all_remaining_local_messages();
-	decrement_smbd_process_count();
 
 #ifdef WITH_DFS
 	if (dcelogin_atmost_once) {