The Samba-Bugzilla – Attachment 16774 Details for
Bug 14821
CPU over-use and flapping test patches
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
patch for 4.15, 4.14, 4.13
cpu-overuse.patch (text/plain), 33.59 KB, created by
Andrew Bartlett
on 2021-09-06 03:15:32 UTC
(
hide
)
Description:
patch for 4.15, 4.14, 4.13
Filename:
MIME Type:
Creator:
Andrew Bartlett
Created:
2021-09-06 03:15:32 UTC
Size:
33.59 KB
patch
obsolete
>From 21b7cec6993b81227e17e43ebb32c095e0bd20ed Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Sat, 4 Sep 2021 12:28:20 +1200 >Subject: [PATCH 1/6] selftest: Split up targets for samba_tool_drs from > samba_tool_drs_showrepl > >These now run in the disconnected sets schema_dc/schema_pair_dc and >ad_dc/vampire_dc/promoted_dc. By aiming at different sets ofservers >we can't cause cross-contamination in terms of which servers are >listed as outbound connections. > >Also, by running the tests only once we reduce the chaces of trouble >by half. > >RN: Address flapping samba_tool_drs_showrepl test >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14818 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit e8b4599e0935290c5e59df9fd4f695ad8d6f361c) >--- > source4/selftest/tests.py | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > >diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py >index 8938754d0fc..a011d50a768 100755 >--- a/source4/selftest/tests.py >+++ b/source4/selftest/tests.py >@@ -1295,6 +1295,18 @@ planoldpythontestsuite(env, "ridalloc_exop", > environ={'DC1': "$DC_SERVER", 'DC2': '$SERVER'}, > extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) > >+# This test can pollute the environment a little by creating and >+# deleting DCs which can get into the replication state for a while. >+# >+# The setting of DC1 to $DC_SERVER means that it will join towards and >+# operate on schema_dc. This matters most when running >+# test_samba_tool_replicate_local as this sets up a full temp DC and >+# does new replication to it, which can show up in the replication >+# topology. >+# >+# That is why this test is run on the isolated environment and not on >+# those connected with ad_dc (vampiredc/promoteddc) >+ > env = 'schema_pair_dc' > planoldpythontestsuite("%s:local" % env, "samba_tool_drs", > extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], >@@ -1309,11 +1321,6 @@ planoldpythontestsuite(env, "getnc_schema", > extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) > > for env in ['vampire_dc', 'promoted_dc']: >- planoldpythontestsuite("%s:local" % env, "samba_tool_drs", >- extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], >- name="samba4.drs.samba_tool_drs.python(%s)" % env, >- environ={'DC1': '$DC_SERVER', 'DC2': '$SERVER'}, >- extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) > planoldpythontestsuite("%s:local" % env, "samba_tool_drs_showrepl", > extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], > name="samba4.drs.samba_tool_drs_showrepl.python(%s)" % env, >-- >2.25.1 > > >From 46ad637584149328e60463aec65593c42bcdc602 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Sat, 4 Sep 2021 13:11:08 +1200 >Subject: [PATCH 2/6] selftest: Only run samba_tool_drs_showrepl test once > >This test is not slow, but there is no value running it twice. > >Running this test twice just increases the chances we might >loose a race as it shows and validates live replication data. > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit 75a5ed66731e947fa16af81aab7649d1fddec45f) >--- > source4/selftest/tests.py | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > >diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py >index a011d50a768..32059461831 100755 >--- a/source4/selftest/tests.py >+++ b/source4/selftest/tests.py >@@ -1320,12 +1320,18 @@ planoldpythontestsuite(env, "getnc_schema", > "PLEASE_BREAK_MY_WINDOWS": "1"}, > extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) > >+# This test can be sensitive to the DC joins and replications don in >+# "samba_tool_drs" so run this is run against scheam_pair_dc/schema_dc >+# not the set of environments connected with ad_dc. >+ >+# This will show the replication state of ad_dc >+planoldpythontestsuite("promoted_dc:local", "samba_tool_drs_showrepl", >+ extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], >+ name="samba4.drs.samba_tool_drs_showrepl.python(%s)" % env, >+ environ={'DC1': '$DC_SERVER', 'DC2': '$SERVER'}, >+ extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) >+ > for env in ['vampire_dc', 'promoted_dc']: >- planoldpythontestsuite("%s:local" % env, "samba_tool_drs_showrepl", >- extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], >- name="samba4.drs.samba_tool_drs_showrepl.python(%s)" % env, >- environ={'DC1': '$DC_SERVER', 'DC2': '$SERVER'}, >- extra_args=['-U$DOMAIN/$DC_USERNAME%$DC_PASSWORD']) > planoldpythontestsuite("%s:local" % env, "replica_sync", > extra_path=[os.path.join(samba4srcdir, 'torture/drs/python')], > name="samba4.drs.replica_sync.python(%s)" % env, >-- >2.25.1 > > >From e1cef55699cc912c932fc3198f0eec7ed78e8a3d Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Wed, 25 Aug 2021 09:41:11 +1200 >Subject: [PATCH 3/6] dsdb: Be careful to avoid use of the expensive > talloc_is_parent() > >The wrong talloc API was selected while addressing a memory leak. > >commit ee2fe56ba0ef6626b634376e8dc2185aa89f8c99 >Author: Aaron Haslett <aaronhaslett@catalyst.net.nz> >Date: Tue Nov 27 11:07:44 2018 +1300 > > drepl: memory leak fix > > Fixes a memory leak where schema reference attached to ldb > instance is lost before it can be freed. > > BUG: https://bugzilla.samba.org/show_bug.cgi?id=14042 > > Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz> > > Reviewed-by: Andrew Bartlett <abartlet@samba.org> > Reviewed-by: Garming Sam <garming@catalyst.net.nz> > > Autobuild-User(master): Garming Sam <garming@samba.org> > Autobuild-Date(master): Wed Jul 17 06:17:10 UTC 2019 on sn-devel-184 > >By using talloc_get_parent() walking the entire talloc tree is >avoided. > >RN: Address a signifcant performance regression in database access in the AD DC since Samba 4.12 > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14806 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit 8affe4a1e625104de4ca024fdc3e9cd96498aff3) >--- > source4/dsdb/schema/schema_set.c | 41 ++++++++++++++++++++++---------- > 1 file changed, 29 insertions(+), 12 deletions(-) > >diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c >index f4934917c7c..45faa0912ec 100644 >--- a/source4/dsdb/schema/schema_set.c >+++ b/source4/dsdb/schema/schema_set.c >@@ -698,6 +698,8 @@ int dsdb_reference_schema(struct ldb_context *ldb, struct dsdb_schema *schema, > { > int ret; > void *ptr; >+ void *schema_parent = NULL; >+ bool is_already_parent; > struct dsdb_schema *old_schema; > old_schema = ldb_get_opaque(ldb, "dsdb_schema"); > ret = ldb_set_opaque(ldb, "dsdb_schema", schema); >@@ -710,8 +712,9 @@ int dsdb_reference_schema(struct ldb_context *ldb, struct dsdb_schema *schema, > talloc_unlink(ldb, old_schema); > > /* Reference schema on ldb if it wasn't done already */ >- ret = talloc_is_parent(ldb, schema); >- if (ret == 0) { >+ schema_parent = talloc_parent(schema); >+ is_already_parent = (schema_parent == ldb); >+ if (!is_already_parent) { > ptr = talloc_reference(ldb, schema); > if (ptr == NULL) { > return ldb_oom(ldb); >@@ -775,10 +778,10 @@ int dsdb_set_global_schema(struct ldb_context *ldb) > /* Don't write indices and attributes, it's expensive */ > ret = dsdb_schema_set_indices_and_attributes(ldb, global_schema, SCHEMA_MEMORY_ONLY); > if (ret == LDB_SUCCESS) { >- /* If ldb doesn't have a reference to the schema, make one, >- * just in case the original copy is replaced */ >- ret = talloc_is_parent(ldb, global_schema); >- if (ret == 0) { >+ void *schema_parent = talloc_parent(global_schema); >+ bool is_already_parent = >+ (schema_parent == ldb); >+ if (!is_already_parent) { > ptr = talloc_reference(ldb, global_schema); > if (ptr == NULL) { > return ldb_oom(ldb); >@@ -809,7 +812,6 @@ struct dsdb_schema *dsdb_get_schema(struct ldb_context *ldb, TALLOC_CTX *referen > dsdb_schema_refresh_fn refresh_fn; > struct ldb_module *loaded_from_module; > bool use_global_schema; >- int ret; > TALLOC_CTX *tmp_ctx = talloc_new(reference_ctx); > if (tmp_ctx == NULL) { > return NULL; >@@ -860,13 +862,28 @@ struct dsdb_schema *dsdb_get_schema(struct ldb_context *ldb, TALLOC_CTX *referen > /* This removes the extra reference above */ > talloc_free(tmp_ctx); > >- /* If ref ctx exists and doesn't already reference schema, then add >- * a reference. Otherwise, just return schema.*/ >- ret = talloc_is_parent(reference_ctx, schema_out); >- if ((ret == 1) || (!reference_ctx)) { >+ /* >+ * If ref ctx exists and doesn't already reference schema, then add >+ * a reference. Otherwise, just return schema. >+ * >+ * We must use talloc_parent(), which is not quite free (there >+ * is no direct parent pointer in talloc, only one on the >+ * first child within a linked list), but is much cheaper than >+ * talloc_is_parent() which walks the whole tree up to the top >+ * looking for a potential grand-grand(etc)-parent. >+ */ >+ if (reference_ctx == NULL) { > return schema_out; > } else { >- return talloc_reference(reference_ctx, schema_out); >+ void *schema_parent = talloc_parent(schema_out); >+ bool is_already_parent = >+ schema_parent == reference_ctx; >+ if (is_already_parent) { >+ return schema_out; >+ } else { >+ return talloc_reference(reference_ctx, >+ schema_out); >+ } > } > } > >-- >2.25.1 > > >From fa6f8b6e68037dded768efa735cea8c47d0e9daa Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Wed, 25 Aug 2021 09:54:04 +0000 >Subject: [PATCH 4/6] selftest: Add a test for LookupSids3 and LookupNames4 in > python > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14807 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit b40761b42e889369599c5eb355028ba377c43b49) >--- > python/samba/tests/dcerpc/lsa.py | 333 +++++++++++++++++++++++++++++++ > source4/selftest/tests.py | 1 + > 2 files changed, 334 insertions(+) > create mode 100644 python/samba/tests/dcerpc/lsa.py > >diff --git a/python/samba/tests/dcerpc/lsa.py b/python/samba/tests/dcerpc/lsa.py >new file mode 100644 >index 00000000000..4377c42e9b8 >--- /dev/null >+++ b/python/samba/tests/dcerpc/lsa.py >@@ -0,0 +1,333 @@ >+# -*- coding: utf-8 -*- >+# >+# Unix SMB/CIFS implementation. >+# Copyright © Andrew Bartlett <abartlet@samba.org> 2021 >+# Copyright (C) Catalyst IT Ltd. 2017 >+# >+# 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/>. >+# >+ >+"""Tests for samba.dcerpc.sam.""" >+ >+from samba.dcerpc import samr, security, lsa >+from samba.credentials import Credentials >+from samba.tests import TestCase >+from samba.dcerpc.security import dom_sid >+from samba import NTSTATUSError >+from samba.ntstatus import NT_STATUS_ACCESS_DENIED >+import samba.tests >+ >+class LsaTests(TestCase): >+ >+ def setUp(self): >+ self.lp = self.get_loadparm() >+ self.server = samba.tests.env_get_var_value('SERVER') >+ >+ def test_lsa_LookupSids3_multiple(self): >+ machine_creds = Credentials() >+ machine_creds.guess(self.lp) >+ machine_creds.set_machine_account() >+ >+ c = lsa.lsarpc( >+ "ncacn_ip_tcp:%s[schannel,seal]" % self.server, >+ self.lp, >+ machine_creds) >+ >+ sids = lsa.SidArray() >+ sid = lsa.SidPtr() >+ # Need a set >+ x = dom_sid("S-1-5-7") >+ sid.sid = x >+ sids.sids = [sid] >+ sids.num_sids = 1 >+ names = lsa.TransNameArray2() >+ level = lsa.LSA_LOOKUP_NAMES_ALL >+ count = 0 >+ lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES >+ client_revision = lsa.LSA_CLIENT_REVISION_2 >+ >+ # We want to run LookupSids3 multiple times on the same >+ # connection as we have code to re-use the sam.ldb and we need >+ # to check things work for the second request. >+ (domains, names, count) = c.LookupSids3(sids, names, level, count, lookup_options, client_revision) >+ self.assertEqual(count, 1) >+ self.assertEqual(names.count, 1) >+ self.assertEqual(names.names[0].name.string, >+ "ANONYMOUS LOGON") >+ (domains2, names2, count2) = c.LookupSids3(sids, names, level, count, lookup_options, client_revision) >+ self.assertEqual(count2, 1) >+ self.assertEqual(names2.count, 1) >+ self.assertEqual(names2.names[0].name.string, >+ "ANONYMOUS LOGON") >+ >+ # Just looking for any exceptions in the last couple of loops >+ c.LookupSids3(sids, names, level, count, lookup_options, client_revision) >+ c.LookupSids3(sids, names, level, count, lookup_options, client_revision) >+ >+ def test_lsa_LookupSids3_multiple_conns(self): >+ machine_creds = Credentials() >+ machine_creds.guess(self.lp) >+ machine_creds.set_machine_account() >+ >+ c = lsa.lsarpc( >+ "ncacn_ip_tcp:%s[schannel,seal]" % self.server, >+ self.lp, >+ machine_creds) >+ >+ sids = lsa.SidArray() >+ sid = lsa.SidPtr() >+ # Need a set >+ x = dom_sid("S-1-5-7") >+ sid.sid = x >+ sids.sids = [sid] >+ sids.num_sids = 1 >+ names = lsa.TransNameArray2() >+ level = lsa.LSA_LOOKUP_NAMES_ALL >+ count = 0 >+ lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES >+ client_revision = lsa.LSA_CLIENT_REVISION_2 >+ >+ # We want to run LookupSids3, and then again on a new >+ # connection to show that we don't have an issue with the DB >+ # being tied to the wrong connection. >+ (domains, names, count) = c.LookupSids3(sids, >+ names, >+ level, >+ count, >+ lookup_options, >+ client_revision) >+ self.assertEqual(count, 1) >+ self.assertEqual(names.count, 1) >+ self.assertEqual(names.names[0].name.string, >+ "ANONYMOUS LOGON") >+ >+ c = lsa.lsarpc( >+ "ncacn_ip_tcp:%s[schannel,seal]" % self.server, >+ self.lp, >+ machine_creds) >+ >+ (domains, names, count) = c.LookupSids3(sids, >+ names, >+ level, >+ count, >+ lookup_options, >+ client_revision) >+ self.assertEqual(count, 1) >+ self.assertEqual(names.count, 1) >+ self.assertEqual(names.names[0].name.string, >+ "ANONYMOUS LOGON") >+ >+ >+ def test_lsa_LookupNames4_LookupSids3_multiple(self): >+ """ >+ Test by going back and forward between real DB lookups >+ name->sid->name to ensure the sam.ldb handle is fine once >+ shared >+ """ >+ >+ machine_creds = Credentials() >+ machine_creds.guess(self.lp) >+ machine_creds.set_machine_account() >+ >+ c_normal = lsa.lsarpc( >+ "ncacn_np:%s[seal]" % self.server, >+ self.lp, >+ machine_creds) >+ >+ username, domain = c_normal.GetUserName(None, None, None) >+ >+ c = lsa.lsarpc( >+ "ncacn_ip_tcp:%s[schannel,seal]" % self.server, >+ self.lp, >+ machine_creds) >+ >+ sids = lsa.TransSidArray3() >+ names = [username] >+ level = lsa.LSA_LOOKUP_NAMES_ALL >+ count = 0 >+ lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES >+ client_revision = lsa.LSA_CLIENT_REVISION_2 >+ (domains, sids, count) = c.LookupNames4(names, >+ sids, >+ level, >+ count, >+ lookup_options, >+ client_revision) >+ >+ # Another lookup on the same connection, will re-used the >+ # server-side implicit state handle on the connection >+ (domains, sids, count) = c.LookupNames4(names, >+ sids, >+ level, >+ count, >+ lookup_options, >+ client_revision) >+ >+ self.assertEqual(count, 1) >+ self.assertEqual(sids.count, 1) >+ >+ # Now look the SIDs back up >+ names = lsa.TransNameArray2() >+ sid = lsa.SidPtr() >+ sid.sid = sids.sids[0].sid >+ lookup_sids = lsa.SidArray() >+ lookup_sids.sids = [sid] >+ lookup_sids.num_sids = 1 >+ level = lsa.LSA_LOOKUP_NAMES_ALL >+ count = 1 >+ lookup_options = 0 >+ client_revision = lsa.LSA_CLIENT_REVISION_2 >+ >+ (domains, names, count) = c.LookupSids3(lookup_sids, >+ names, >+ level, >+ count, >+ lookup_options, >+ client_revision) >+ self.assertEqual(count, 1) >+ self.assertEqual(names.count, 1) >+ self.assertEqual(names.names[0].name.string, >+ username.string) >+ >+ # And once more just to be sure, just checking for a fault >+ sids = lsa.TransSidArray3() >+ names = [username] >+ level = lsa.LSA_LOOKUP_NAMES_ALL >+ count = 0 >+ lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES >+ client_revision = lsa.LSA_CLIENT_REVISION_2 >+ (domains, sids, count) = c.LookupNames4(names, >+ sids, >+ level, >+ count, >+ lookup_options, >+ client_revision) >+ >+ >+ def test_lsa_LookupNames4_multiple_conns(self): >+ """ >+ Test by going back and forward between real DB lookups >+ name->sid->name to ensure the sam.ldb handle is fine once >+ shared >+ """ >+ >+ machine_creds = Credentials() >+ machine_creds.guess(self.lp) >+ machine_creds.set_machine_account() >+ >+ c_normal = lsa.lsarpc( >+ "ncacn_np:%s[seal]" % self.server, >+ self.lp, >+ machine_creds) >+ >+ username, domain = c_normal.GetUserName(None, None, None) >+ >+ c = lsa.lsarpc( >+ "ncacn_ip_tcp:%s[schannel,seal]" % self.server, >+ self.lp, >+ machine_creds) >+ >+ sids = lsa.TransSidArray3() >+ names = [username] >+ level = lsa.LSA_LOOKUP_NAMES_ALL >+ count = 0 >+ lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES >+ client_revision = lsa.LSA_CLIENT_REVISION_2 >+ (domains, sids, count) = c.LookupNames4(names, >+ sids, >+ level, >+ count, >+ lookup_options, >+ client_revision) >+ >+ c = lsa.lsarpc( >+ "ncacn_ip_tcp:%s[schannel,seal]" % self.server, >+ self.lp, >+ machine_creds) >+ >+ sids = lsa.TransSidArray3() >+ names = [username] >+ level = lsa.LSA_LOOKUP_NAMES_ALL >+ count = 0 >+ lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES >+ client_revision = lsa.LSA_CLIENT_REVISION_2 >+ (domains, sids, count) = c.LookupNames4(names, >+ sids, >+ level, >+ count, >+ lookup_options, >+ client_revision) >+ >+ def test_lsa_LookupNames4_without_schannel(self): >+ >+ machine_creds = Credentials() >+ machine_creds.guess(self.lp) >+ machine_creds.set_machine_account() >+ >+ c_normal = lsa.lsarpc( >+ "ncacn_np:%s[seal]" % self.server, >+ self.lp, >+ machine_creds) >+ >+ username, domain = c_normal.GetUserName(None, None, None) >+ >+ sids = lsa.TransSidArray3() >+ names = [username] >+ level = lsa.LSA_LOOKUP_NAMES_ALL >+ count = 0 >+ lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES >+ client_revision = lsa.LSA_CLIENT_REVISION_2 >+ >+ with self.assertRaises(NTSTATUSError) as e: >+ c_normal.LookupNames4(names, >+ sids, >+ level, >+ count, >+ lookup_options, >+ client_revision) >+ if (e.exception.args[0] != NT_STATUS_ACCESS_DENIED): >+ raise AssertionError("LookupNames4 without schannel must fail with ACCESS_DENIED") >+ >+ def test_lsa_LookupSids3_without_schannel(self): >+ machine_creds = Credentials() >+ machine_creds.guess(self.lp) >+ machine_creds.set_machine_account() >+ >+ c = lsa.lsarpc( >+ "ncacn_ip_tcp:%s[seal]" % self.server, >+ self.lp, >+ machine_creds) >+ >+ sids = lsa.SidArray() >+ sid = lsa.SidPtr() >+ # Need a set >+ x = dom_sid("S-1-5-7") >+ sid.sid = x >+ sids.sids = [sid] >+ sids.num_sids = 1 >+ names = lsa.TransNameArray2() >+ level = lsa.LSA_LOOKUP_NAMES_ALL >+ count = 0 >+ lookup_options = lsa.LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES >+ client_revision = lsa.LSA_CLIENT_REVISION_2 >+ >+ with self.assertRaises(NTSTATUSError) as e: >+ c.LookupSids3(sids, >+ names, >+ level, >+ count, >+ lookup_options, >+ client_revision) >+ if (e.exception.args[0] != NT_STATUS_ACCESS_DENIED): >+ raise AssertionError("LookupSids3 without schannel must fail with ACCESS_DENIED") >diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py >index 32059461831..638bd57334e 100755 >--- a/source4/selftest/tests.py >+++ b/source4/selftest/tests.py >@@ -770,6 +770,7 @@ planpythontestsuite("ad_dc_default:local", "samba.tests.dcerpc.sam") > planpythontestsuite("ad_dc_default:local", "samba.tests.dsdb") > planpythontestsuite("none", "samba.tests.dsdb_lock") > planpythontestsuite("ad_dc_default:local", "samba.tests.dcerpc.bare") >+planpythontestsuite("ad_dc_default:local", "samba.tests.dcerpc.lsa") > planpythontestsuite("ad_dc_default:local", "samba.tests.dcerpc.unix") > planpythontestsuite("ad_dc_ntvfs:local", "samba.tests.dcerpc.srvsvc") > planpythontestsuite("ad_dc_default:local", "samba.tests.samba_tool.timecmd") >-- >2.25.1 > > >From 3d88c735ee87311db27871550fe58fa676fe3122 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Wed, 25 Aug 2021 12:03:08 +1200 >Subject: [PATCH 5/6] s4-lsa: Cache sam.ldb handle in > lsa_LookupSids3/LookupNames4 > >Since 5c0345ea9bb34695dcd7be6c913748323bebe937 this >would not have been implicitly cached via the ldb_wrap >cache, due to the recording of the remote IP address >(which is a good thing). > >This creates a more explicit and direct correct >cache on the connection. > >The common code, including the SCHANNEL check is >placed into a helper function. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14807 > >RN: Fix performance regression in lsa_LookupSids3/LookupNames4 since Samba 4.9 by using an explicit database handle cache > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> > >Autobuild-User(master): Jeremy Allison <jra@samba.org> >Autobuild-Date(master): Sun Sep 5 03:19:26 UTC 2021 on sn-devel-184 > >(cherry picked from commit ae57d22e45b33537e9fca5969e9b68abd1ad633f) >--- > source4/rpc_server/lsa/lsa_lookup.c | 131 ++++++++++++++++++++-------- > 1 file changed, 94 insertions(+), 37 deletions(-) > >diff --git a/source4/rpc_server/lsa/lsa_lookup.c b/source4/rpc_server/lsa/lsa_lookup.c >index d41997d4b3d..61cb8a10a23 100644 >--- a/source4/rpc_server/lsa/lsa_lookup.c >+++ b/source4/rpc_server/lsa/lsa_lookup.c >@@ -663,25 +663,24 @@ NTSTATUS dcesrv_lsa_LookupSids2(struct dcesrv_call_state *dce_call, > return status; > } > >+/* A random hexidecimal number (honest!) */ >+#define LSA_SERVER_IMPLICIT_POLICY_STATE_MAGIC 0xc0c99e00 > > /* >- lsa_LookupSids3 >- >- Identical to LookupSids2, but doesn't take a policy handle >- >+ Ensure we're operating on an schannel connection, >+ and use a lsa_policy_state cache on the connection. > */ >-NTSTATUS dcesrv_lsa_LookupSids3(struct dcesrv_call_state *dce_call, >- TALLOC_CTX *mem_ctx, >- struct lsa_LookupSids3 *r) >+static NTSTATUS schannel_call_setup(struct dcesrv_call_state *dce_call, >+ struct lsa_policy_state **_policy_state) > { >+ struct lsa_policy_state *policy_state = NULL; > enum dcerpc_transport_t transport = > dcerpc_binding_get_transport(dce_call->conn->endpoint->ep_description); > enum dcerpc_AuthType auth_type = DCERPC_AUTH_TYPE_NONE; >- struct dcesrv_lsa_LookupSids_base_state *state = NULL; >- NTSTATUS status; >- > if (transport != NCACN_IP_TCP) { >- DCESRV_FAULT(DCERPC_FAULT_ACCESS_DENIED); >+ /* We can't call DCESRV_FAULT() in the sub-function */ >+ dce_call->fault_code = DCERPC_FAULT_ACCESS_DENIED; >+ return NT_STATUS_ACCESS_DENIED; > } > > /* >@@ -693,8 +692,59 @@ NTSTATUS dcesrv_lsa_LookupSids3(struct dcesrv_call_state *dce_call, > */ > dcesrv_call_auth_info(dce_call, &auth_type, NULL); > if (auth_type != DCERPC_AUTH_TYPE_SCHANNEL) { >- DCESRV_FAULT(DCERPC_FAULT_ACCESS_DENIED); >+ /* We can't call DCESRV_FAULT() in the sub-function */ >+ dce_call->fault_code = DCERPC_FAULT_ACCESS_DENIED; >+ return NT_STATUS_ACCESS_DENIED; >+ } >+ >+ /* >+ * We don't have a policy handle on this call, so we want to >+ * make a policy state and cache it for the life of the >+ * connection, to avoid re-opening the DB each call. >+ */ >+ policy_state >+ = dcesrv_iface_state_find_conn(dce_call, >+ LSA_SERVER_IMPLICIT_POLICY_STATE_MAGIC, >+ struct lsa_policy_state); >+ >+ if (policy_state == NULL) { >+ NTSTATUS status >+ = dcesrv_lsa_get_policy_state(dce_call, >+ dce_call /* mem_ctx */, >+ 0, /* we skip access checks */ >+ &policy_state); >+ if (!NT_STATUS_IS_OK(status)) { >+ return status; >+ } >+ >+ /* >+ * This will talloc_steal() policy_state onto the >+ * connection, which has longer lifetime than the >+ * immidiate caller requires >+ */ >+ status = dcesrv_iface_state_store_conn(dce_call, >+ LSA_SERVER_IMPLICIT_POLICY_STATE_MAGIC, >+ policy_state); >+ if (!NT_STATUS_IS_OK(status)) { >+ return status; >+ } > } >+ *_policy_state = policy_state; >+ return NT_STATUS_OK; >+} >+ >+/* >+ lsa_LookupSids3 >+ >+ Identical to LookupSids2, but doesn't take a policy handle >+ >+*/ >+NTSTATUS dcesrv_lsa_LookupSids3(struct dcesrv_call_state *dce_call, >+ TALLOC_CTX *mem_ctx, >+ struct lsa_LookupSids3 *r) >+{ >+ struct dcesrv_lsa_LookupSids_base_state *state = NULL; >+ NTSTATUS status; > > *r->out.domains = NULL; > r->out.names->count = 0; >@@ -706,16 +756,27 @@ NTSTATUS dcesrv_lsa_LookupSids3(struct dcesrv_call_state *dce_call, > return NT_STATUS_NO_MEMORY; > } > >- state->dce_call = dce_call; >- state->mem_ctx = mem_ctx; >- >- status = dcesrv_lsa_get_policy_state(state->dce_call, mem_ctx, >- 0, /* we skip access checks */ >- &state->policy_state); >+ /* >+ * We don't have a policy handle on this call, so we want to >+ * make a policy state and cache it for the life of the >+ * connection, to avoid re-opening the DB each call. >+ * >+ * This also enforces that this is only available over >+ * ncacn_ip_tcp and with SCHANNEL. >+ * >+ * schannel_call_setup may also set the fault state. >+ * >+ * state->policy_state is shared between all calls on this >+ * connection and is moved with talloc_steal() under the >+ * connection, not dce_call or state. >+ */ >+ status = schannel_call_setup(dce_call, &state->policy_state); > if (!NT_STATUS_IS_OK(status)) { > return status; > } > >+ state->dce_call = dce_call; >+ state->mem_ctx = mem_ctx; > state->r.in.sids = r->in.sids; > state->r.in.level = r->in.level; > state->r.in.lookup_options = r->in.lookup_options; >@@ -1296,25 +1357,9 @@ NTSTATUS dcesrv_lsa_LookupNames3(struct dcesrv_call_state *dce_call, > NTSTATUS dcesrv_lsa_LookupNames4(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx, > struct lsa_LookupNames4 *r) > { >- enum dcerpc_transport_t transport = >- dcerpc_binding_get_transport(dce_call->conn->endpoint->ep_description); >- enum dcerpc_AuthType auth_type = DCERPC_AUTH_TYPE_NONE; > struct dcesrv_lsa_LookupNames_base_state *state = NULL; > NTSTATUS status; > >- if (transport != NCACN_IP_TCP) { >- DCESRV_FAULT(DCERPC_FAULT_ACCESS_DENIED); >- } >- >- /* >- * We don't have policy handles on this call. So this must be restricted >- * to crypto connections only. >- */ >- dcesrv_call_auth_info(dce_call, &auth_type, NULL); >- if (auth_type != DCERPC_AUTH_TYPE_SCHANNEL) { >- DCESRV_FAULT(DCERPC_FAULT_ACCESS_DENIED); >- } >- > *r->out.domains = NULL; > r->out.sids->count = 0; > r->out.sids->sids = NULL; >@@ -1328,9 +1373,21 @@ NTSTATUS dcesrv_lsa_LookupNames4(struct dcesrv_call_state *dce_call, TALLOC_CTX > state->dce_call = dce_call; > state->mem_ctx = mem_ctx; > >- status = dcesrv_lsa_get_policy_state(state->dce_call, state, >- 0, /* we skip access checks */ >- &state->policy_state); >+ /* >+ * We don't have a policy handle on this call, so we want to >+ * make a policy state and cache it for the life of the >+ * connection, to avoid re-opening the DB each call. >+ * >+ * This also enforces that this is only available over >+ * ncacn_ip_tcp and with SCHANNEL. >+ * >+ * schannel_call_setup may also set the fault state. >+ * >+ * state->policy_state is shared between all calls on this >+ * connection and is moved with talloc_steal() under the >+ * connection, not dce_call or state. >+ */ >+ status = schannel_call_setup(dce_call, &state->policy_state); > if (!NT_STATUS_IS_OK(status)) { > return status; > } >-- >2.25.1 > > >From 0037c931bdd40ccddde2428e5be7edcb0febaebd Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Mon, 6 Sep 2021 08:52:21 +1200 >Subject: [PATCH 6/6] selftest: Add prefix to new schema attributes to avoid > flapping dsdb_schema_attributes > >If two of these unit tests run in the same second they could >select the same name, as the name was only based on the time >and a common prefix. > >As observed by Jeremy Allison. Thanks for the report! > >RN: Address flapping dsdb_schema_attributes test > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14819 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> > >Autobuild-User(master): Jeremy Allison <jra@samba.org> >Autobuild-Date(master): Mon Sep 6 02:32:51 UTC 2021 on sn-devel-184 > >(cherry picked from commit 6590bb0b77c641f0d4686b39c713c1405ffb64f5) >--- > python/samba/tests/dsdb_schema_attributes.py | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > >diff --git a/python/samba/tests/dsdb_schema_attributes.py b/python/samba/tests/dsdb_schema_attributes.py >index bb4603d4703..d9b538d4af4 100644 >--- a/python/samba/tests/dsdb_schema_attributes.py >+++ b/python/samba/tests/dsdb_schema_attributes.py >@@ -87,7 +87,7 @@ systemOnly: FALSE > > def test_AddIndexedAttribute(self): > # create names for an attribute to add >- (attr_name, attr_ldap_name, attr_dn) = self._make_obj_names("schemaAttributes-Attr-") >+ (attr_name, attr_ldap_name, attr_dn) = self._make_obj_names("schemaAttributes-IdxAttr-") > ldif = self._make_attr_ldif(attr_name, attr_dn, 1, > "searchFlags: %d" % samba.dsdb.SEARCH_FLAG_ATTINDEX) > >@@ -111,7 +111,7 @@ systemOnly: FALSE > > def test_AddUnIndexedAttribute(self): > # create names for an attribute to add >- (attr_name, attr_ldap_name, attr_dn) = self._make_obj_names("schemaAttributes-Attr-") >+ (attr_name, attr_ldap_name, attr_dn) = self._make_obj_names("schemaAttributes-UnIdxAttr-") > ldif = self._make_attr_ldif(attr_name, attr_dn, 2) > > # add the new attribute >@@ -134,7 +134,7 @@ systemOnly: FALSE > > def test_AddTwoIndexedAttributes(self): > # create names for an attribute to add >- (attr_name, attr_ldap_name, attr_dn) = self._make_obj_names("schemaAttributes-Attr-") >+ (attr_name, attr_ldap_name, attr_dn) = self._make_obj_names("schemaAttributes-2IdxAttr-") > ldif = self._make_attr_ldif(attr_name, attr_dn, 3, > "searchFlags: %d" % samba.dsdb.SEARCH_FLAG_ATTINDEX) > >-- >2.25.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
jra
:
review+
abartlet
:
ci-passed+
Actions:
View
Attachments on
bug 14821
: 16774 |
17535