[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Fri Nov 30 14:06:03 UTC 2018


The branch, master has been updated
       via  5b627edc556 WHATSNEW: standard process limits
       via  f90cf49970a s4 smdb standard: Limit processes forked on accept.
       via  2381b4ff679 s4 smbd standard tests: limit forked processes
      from  b3490049574 replace: Correctly check for 'extern char **environ' in unistd.h

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 5b627edc55683417e7ac56d3aff7b7aa7cec5d92
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Wed Sep 19 15:52:15 2018 +1200

    WHATSNEW: standard process limits
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    
    Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date(master): Fri Nov 30 15:05:04 CET 2018 on sn-devel-144

commit f90cf49970a0edf4dce5d145726a04e798530a2e
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Fri Sep 7 07:04:48 2018 +1200

    s4 smdb standard: Limit processes forked on accept.
    
    Limit the number of processes started by the standard model on accept.
    For those services that support fork on accept, the standard model forks
    a new process for each new connection. This patch limits the number of
    processes to the value specified in 'max smbd processes', a value of
    zero indicates that there is no limit on the number of processes that
    can be forked.
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 2381b4ff6794ba514cca57d3995459bc2cef8352
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Tue Sep 18 11:21:40 2018 +1200

    s4 smbd standard tests: limit forked processes
    
    Tests to confirm the standard process model honours the smbd.conf
    variable "max smbd processes", when forking a new process on accept.
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 WHATSNEW.txt                                    |  9 +++
 docs-xml/smbdotconf/tuning/maxsmbdprocesses.xml |  7 +++
 python/samba/tests/process_limits.py            | 78 +++++++++++++++++++++++++
 selftest/target/README                          |  7 +++
 selftest/target/Samba.pm                        |  1 +
 selftest/target/Samba4.pm                       | 39 +++++++++++++
 source4/selftest/tests.py                       |  9 +++
 source4/smbd/process_standard.c                 | 33 ++++++++++-
 8 files changed, 181 insertions(+), 2 deletions(-)
 create mode 100644 python/samba/tests/process_limits.py


Changeset truncated at 500 lines:

diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index 23a88c295b5..50543637c86 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -75,6 +75,15 @@ delay between restart attempts up until it reaches "prefork maximum backoff".
 Using the default sequence the restart delays (in seconds) are:
   0, 10, 20, ..., 120, 120, ...
 
+standard process model
+----------------------
+
+When using the standard process model samba forks a new process to handle ldap
+and netlogon connections.  Samba now honours the 'max smbd processes' smb.conf
+parameter.  The default value of 0, indicates there is no limit.  The limit
+is applied individually to netlogon and ldap.  When the process limit is
+exceeded Samba drops new connections immediately.
+
 REMOVED FEATURES
 ================
 
diff --git a/docs-xml/smbdotconf/tuning/maxsmbdprocesses.xml b/docs-xml/smbdotconf/tuning/maxsmbdprocesses.xml
index a194a26a13c..f5b1e4230be 100644
--- a/docs-xml/smbdotconf/tuning/maxsmbdprocesses.xml
+++ b/docs-xml/smbdotconf/tuning/maxsmbdprocesses.xml
@@ -10,6 +10,13 @@
     conditions, each user will have an <citerefentry><refentrytitle>smbd</refentrytitle>
     <manvolnum>8</manvolnum></citerefentry> associated with him or her to handle connections to all
     shares from a given host.</para>
+
+    <para>For a Samba ADDC running the standard process model this option
+    limits the number of processes forked to handle requests.
+    Currently new processes are only forked for ldap and netlogon
+    requests.
+    </para>
+
 </description>
 
 <value type="default">0</value>
diff --git a/python/samba/tests/process_limits.py b/python/samba/tests/process_limits.py
new file mode 100644
index 00000000000..b62906e294f
--- /dev/null
+++ b/python/samba/tests/process_limits.py
@@ -0,0 +1,78 @@
+# Tests for limiting processes forked on accept by the standard process model
+#
+# Copyright (C) Andrew Bartlett <abartlet at samba.org> 2018
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+from __future__ import print_function
+"""Tests limits on processes forked by fork on accept in the standard process
+   model.
+   NOTE: This test runs in an environment with an artificially low setting for
+         smbd max processes
+"""
+
+
+import os
+from samba.tests import TestCase
+from samba.samdb import SamDB
+from ldb import LdbError, ERR_OPERATIONS_ERROR
+
+
+class StandardModelProcessLimitTests(TestCase):
+
+    def setUp(self):
+        super(StandardModelProcessLimitTests, self).setUp()
+
+    def tearDown(self):
+        super(StandardModelProcessLimitTests, self).tearDown()
+
+    def simple_bind(self):
+        creds = self.insta_creds(template=self.get_credentials())
+        creds.set_bind_dn("%s\\%s" % (creds.get_domain(),
+                                      creds.get_username()))
+
+        return SamDB(url="ldaps://%s" % os.environ["SERVER"],
+                     lp=self.get_loadparm(),
+                     credentials=creds)
+
+    def test_process_limits(self):
+        creds = self.insta_creds(template=self.get_credentials())
+        creds.set_bind_dn("%s\\%s" % (creds.get_domain(),
+                                      creds.get_username()))
+
+        connections = []
+        try:
+            # Open a series of LDAP connections, the maximum number of
+            # active connections should be 20, so the 21st should fail.
+            # But as it is possible that there may be other processes holding
+            # connections, need to allow for earlier connection failures.
+            for _ in range(21):
+                connections.append(self.simple_bind())
+            self.fail(
+                "Processes not limited, able to make more than 20 connections")
+        except LdbError as e:
+            (errno, estr) = e.args
+            if errno != ERR_OPERATIONS_ERROR:
+                raise
+            if not (estr.endswith("NT_STATUS_CONNECTION_DISCONNECTED") or
+                    estr.endswith("NT_STATUS_CONNECTION_RESET")):
+                raise
+            pass
+        #
+        # Clean up the connections we've just opened, by deleting the
+        # connection in python. This should invoke the talloc destructor to
+        # release any resources and close the actual connection to the server.
+        for c in connections:
+            del c
diff --git a/selftest/target/README b/selftest/target/README
index b25dbab97da..36b68d5dd24 100644
--- a/selftest/target/README
+++ b/selftest/target/README
@@ -108,3 +108,10 @@ preforkrestartdc testenv
 Used to test killing and restarting processes under the pre-fork model. Due to
 the destructive nature of the tests, it's not recommended to use this testenv
 for anything else.
+
+proclimitdc testenv
+-------------------
+Used to test process limits on the standard model. It sets the number of
+allowed processes artificially low, to test that new connections are refused
+correctly.  Due to the limited number of connections accepted, it's not
+recommended to use this testenv for anything else.
diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
index f5c490fb05b..4820f987fd7 100644
--- a/selftest/target/Samba.pm
+++ b/selftest/target/Samba.pm
@@ -425,6 +425,7 @@ sub get_interface($)
     $interfaces{"offlinebackupdc"} = 44;
     $interfaces{"customdc"} = 45;
     $interfaces{"prockilldc"} = 46;
+    $interfaces{"proclimitdc"} = 47;
 
     # update lib/socket_wrapper/socket_wrapper.c
     #  #define MAX_WRAPPED_INTERFACES 64
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 0a6c85d1ba7..c54942b61fd 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -2228,6 +2228,7 @@ sub check_env($$)
 	renamedc             => ["backupfromdc"],
 	offlinebackupdc      => ["backupfromdc"],
 	labdc                => ["backupfromdc"],
+	proclimitdc          => [],
 
 	none                 => [],
 );
@@ -2688,6 +2689,44 @@ sub setup_preforkrestartdc
 	return $env;
 }
 
+#
+# ad_dc test environment used solely to test standard process model connection
+# process limits. As the limit is set artificially low it should not be used
+# for other tests.
+sub setup_proclimitdc
+{
+	my ($self, $path) = @_;
+
+	# If we didn't build with ADS, pretend this env was never available
+	if (not $self->{target3}->have_ads()) {
+	       return "UNKNOWN";
+	}
+
+	my $env = $self->provision_ad_dc(
+		$path,
+		"proclimitdc",
+		"PROCLIMITDOM",
+		"proclimit.samba.example.com",
+		"max smbd processes = 20");
+	unless ($env) {
+		return undef;
+	}
+
+	$env->{NSS_WRAPPER_MODULE_SO_PATH} = undef;
+	$env->{NSS_WRAPPER_MODULE_FN_PREFIX} = undef;
+
+	if (not defined($self->check_or_start($env, "standard"))) {
+	    return undef;
+	}
+
+	my $upn_array = ["$env->{REALM}.upn"];
+	my $spn_array = ["$env->{REALM}.spn"];
+
+	$self->setup_namespaces($env, $upn_array, $spn_array);
+
+	return $env;
+}
+
 # Sets up a DC that's solely used to do a domain backup from. We then use the
 # backupfrom-DC to create the restore-DC - this proves that the backup/restore
 # process will create a Samba DC that will actually start up.
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 972f65307d8..575c14b8be2 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -1249,3 +1249,12 @@ planoldpythontestsuite("preforkrestartdc:local",
                        extra_args=['-U"$USERNAME%$PASSWORD"'],
                        name="samba.tests.prefork_restart",
                        py3_compatible=True)
+planoldpythontestsuite("proclimitdc:local",
+                       "samba.tests.process_limits",
+                       extra_path=[
+                           os.path.join(srcdir(), 'python/samba/tests')],
+                       extra_args=['-U"$USERNAME%$PASSWORD"'],
+                       environ={'CLIENT_IP': '127.0.0.11',
+                                'SOCKET_WRAPPER_DEFAULT_IFACE': 11},
+                       name="samba.tests.process_limits",
+                       py3_compatible=True)
diff --git a/source4/smbd/process_standard.c b/source4/smbd/process_standard.c
index b1cc7de155d..139339c92ec 100644
--- a/source4/smbd/process_standard.c
+++ b/source4/smbd/process_standard.c
@@ -32,6 +32,9 @@
 #include "lib/util/debug.h"
 #include "source3/lib/messages_dgm.h"
 
+static unsigned connections_active = 0;
+static unsigned smbd_max_processes = 0;
+
 struct standard_child_state {
 	const char *name;
 	pid_t pid;
@@ -143,8 +146,7 @@ static void standard_child_pipe_handler(struct tevent_context *ev,
 		if (errno == 0) {
 			errno = ECHILD;
 		}
-		TALLOC_FREE(state);
-		return;
+		goto done;
 	}
 	if (WIFEXITED(status)) {
 		status = WEXITSTATUS(status);
@@ -157,7 +159,17 @@ static void standard_child_pipe_handler(struct tevent_context *ev,
 		DBG_ERR("Child %d (%s) terminated with signal %d\n",
 			(int)state->pid, state->name, status);
 	}
+done:
 	TALLOC_FREE(state);
+	if (smbd_max_processes > 0) {
+		if (connections_active < 1) {
+			DBG_ERR("Number of active connections "
+				"less than 1 (%d)\n",
+				connections_active);
+			connections_active = 1;
+		}
+		connections_active--;
+	}
 	return;
 }
 
@@ -282,6 +294,21 @@ static void standard_accept_connection(
 		return;
 	}
 
+	if (smbd_max_processes > 0) {
+		if (connections_active >= smbd_max_processes) {
+			DBG_ERR("(%d) connections already active, "
+				"maximum is (%d). Dropping request\n",
+				connections_active,
+				smbd_max_processes);
+			/*
+			 * Drop the connection as we're overloaded at the moment
+			 */
+			talloc_free(sock2);
+			return;
+		}
+		connections_active++;
+	}
+
 	state = setup_standard_child_pipe(ev, NULL);
 	if (state == NULL) {
 		return;
@@ -486,6 +513,8 @@ static void standard_new_task(struct tevent_context *ev,
 		service_details->inhibit_fork_on_accept;
 	proc_ctx->forked_on_accept = false;
 
+	smbd_max_processes = lpcfg_max_smbd_processes(lp_ctx);
+
 	/* setup this new task.  Cluster ID is PID based for this process model */
 	task = new_task(ev, lp_ctx, cluster_id(pid, 0), private_data, proc_ctx);
 	/*


-- 
Samba Shared Repository



More information about the samba-cvs mailing list