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

diff --git a/source/lib/dummysmbd.c b/source/lib/dummysmbd.c
index 087de2fe253f20540ad82c896aee88f6cc003a62..5bb71e120e13e2c8810160221859c5016aecf777 100644 (file)
--- a/source/lib/dummysmbd.c
+++ b/source/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/source/lib/util.c b/source/lib/util.c
index f985c57ed955dfcf515a9baa2085d9da5b0d5ef0..ef954015d66e7e463943949064cae51f7622cc88 100644 (file)
--- a/source/lib/util.c
+++ b/source/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/source/smbd/process.c b/source/smbd/process.c
index f8c66d93eaebdd261d9f8f5dcea055a56d73bb3d..a202c1fa87a131c5bd2821722cf519a7ebee7bfd 100644 (file)
--- a/source/smbd/process.c
+++ b/source/smbd/process.c
@@ -1031,60 +1031,6 @@ static int construct_reply(char *inbuf,char *outbuf,int size,int bufsize)
        return(outsize);
 }
 
-/****************************************************************************
- 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/source/smbd/server.c b/source/smbd/server.c
index edde12465f0c35e174d061205798b32e7e0d3bc1..460e2cc44bda9b4e543d6a21455f86a480425847 100644 (file)
--- a/source/smbd/server.c
+++ b/source/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;
@@ -92,6 +93,15 @@ static void sig_hup(int sig)
        sys_select_signal(SIGHUP);
 }
 
+/****************************************************************************
+ 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) {