[SCM] Samba Shared Repository - branch master updated
Jeremy Allison
jra at samba.org
Tue Nov 26 22:56:04 UTC 2019
The branch, master has been updated
via ff35160dc64 net: Delete share ACL from 'net conf delshare'
via 9dddb28a171 selftest: Add test for 'net conf delshare' also removing share ACL
via cb907fbee83 selftest: Make net command available in sharesec test
via 6bf6e5364a9 sharesec: Return NTSTATUS from delete_share_security
via 1f304fc16c6 sharesec: Return NTSTATUS from set_share_security
via 6dfa5bb64fc sharesec: Return NTSTATUS from share_info_db_init
via 39f21a58b0c g_lock: Speed up sync g_lock_lock()
via ba852a3d92f g_lock: Move a variable inside the block were it's used
via f849d8c0cf7 g_lock: Factor out g_lock_cleanup_shared()
from 03854960870 s3:winbind: Also set the cmd name for bool dispatch table
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit ff35160dc6415e1ef5d57717d7636f38ea25a8f0
Author: Christof Schmitt <cs at samba.org>
Date: Wed Nov 20 11:39:20 2019 -0700
net: Delete share ACL from 'net conf delshare'
Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
Autobuild-User(master): Jeremy Allison <jra at samba.org>
Autobuild-Date(master): Tue Nov 26 22:55:38 UTC 2019 on sn-devel-184
commit 9dddb28a1719c753e84e2631ef1e1f12460cc3df
Author: Christof Schmitt <cs at samba.org>
Date: Tue Nov 26 09:59:15 2019 -0700
selftest: Add test for 'net conf delshare' also removing share ACL
Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit cb907fbee8373666100984693f004e37cb20cacf
Author: Christof Schmitt <cs at samba.org>
Date: Tue Nov 26 09:58:32 2019 -0700
selftest: Make net command available in sharesec test
Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit 6bf6e5364a9ad3e5d8186f1226c83ecf16637a37
Author: Christof Schmitt <cs at samba.org>
Date: Wed Sep 18 13:37:32 2019 -0700
sharesec: Return NTSTATUS from delete_share_security
Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit 1f304fc16c675f402c61f95601481e7a3e27db04
Author: Christof Schmitt <cs at samba.org>
Date: Wed Sep 18 13:20:35 2019 -0700
sharesec: Return NTSTATUS from set_share_security
Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit 6dfa5bb64fce1ddd054966bd89b07e43034c6edb
Author: Christof Schmitt <cs at samba.org>
Date: Tue Sep 17 12:11:59 2019 -0700
sharesec: Return NTSTATUS from share_info_db_init
Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit 39f21a58b0c196ef32e66a27051837e4395bb958
Author: Volker Lendecke <vl at samba.org>
Date: Fri Nov 22 12:02:22 2019 +0100
g_lock: Speed up sync g_lock_lock()
The comment "this is used in very hot code paths" is not true right now, but
will get true soon....
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit ba852a3d92fd762e0a4d4e1b3a8177a30b5e34d5
Author: Volker Lendecke <vl at samba.org>
Date: Fri Nov 22 11:56:55 2019 +0100
g_lock: Move a variable inside the block were it's used
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit f849d8c0cf7b8f04dd53cad4f633628c41565d2c
Author: Volker Lendecke <vl at samba.org>
Date: Fri Nov 22 11:55:52 2019 +0100
g_lock: Factor out g_lock_cleanup_shared()
This function will find a second user soon
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
-----------------------------------------------------------------------
Summary of changes:
source3/include/proto.h | 7 +-
source3/lib/g_lock.c | 147 +++++++++++++++++++++++-------
source3/lib/sharesec.c | 64 +++++++------
source3/param/loadparm.c | 5 +-
source3/rpc_server/fss/srv_fss_agent.c | 6 +-
source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 9 +-
source3/script/tests/test_sharesec.sh | 21 ++++-
source3/selftest/tests.py | 3 +-
source3/smbd/server.c | 3 +-
source3/utils/net_conf.c | 9 ++
source3/utils/sharesec.c | 13 ++-
11 files changed, 210 insertions(+), 77 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/include/proto.h b/source3/include/proto.h
index 332a4132424..223f45b69ee 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -138,12 +138,13 @@ struct named_mutex *grab_named_mutex(TALLOC_CTX *mem_ctx, const char *name,
/* The following definitions come from lib/sharesec.c */
-bool share_info_db_init(void);
+NTSTATUS share_info_db_init(void);
struct security_descriptor *get_share_security_default( TALLOC_CTX *ctx, size_t *psize, uint32_t def_access);
struct security_descriptor *get_share_security( TALLOC_CTX *ctx, const char *servicename,
size_t *psize);
-bool set_share_security(const char *share_name, struct security_descriptor *psd);
-bool delete_share_security(const char *servicename);
+NTSTATUS set_share_security(const char *share_name,
+ struct security_descriptor *psd);
+NTSTATUS delete_share_security(const char *servicename);
bool share_access_check(const struct security_token *token,
const char *sharename,
uint32_t desired_access,
diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c
index a05bb443b41..1f4dcda2605 100644
--- a/source3/lib/g_lock.c
+++ b/source3/lib/g_lock.c
@@ -266,6 +266,34 @@ static ssize_t g_lock_find_shared(
return -1;
}
+static void g_lock_cleanup_shared(struct g_lock *lck)
+{
+ size_t i;
+ struct server_id check;
+ bool exists;
+
+ if (lck->num_shared == 0) {
+ return;
+ }
+
+ /*
+ * Read locks can stay around forever if the process dies. Do
+ * a heuristic check for process existence: Check one random
+ * process for existence. Hopefully this will keep runaway
+ * read locks under control.
+ */
+ i = generate_random() % lck->num_shared;
+ g_lock_get_shared(lck, i, &check);
+
+ exists = serverid_exists(&check);
+ if (!exists) {
+ struct server_id_buf tmp;
+ DBG_DEBUG("Shared locker %s died -- removing\n",
+ server_id_str_buf(check, &tmp));
+ g_lock_del_shared(lck, i);
+ }
+}
+
struct g_lock_lock_state {
struct tevent_context *ev;
struct g_lock_ctx *ctx;
@@ -295,11 +323,8 @@ static NTSTATUS g_lock_trylock(
enum g_lock_type type = req_state->type;
bool retry = req_state->retry;
struct g_lock lck = { .exclusive.pid = 0 };
- struct server_id check;
struct server_id_buf tmp;
NTSTATUS status;
- size_t i;
- bool exists;
bool ok;
ok = g_lock_parse(data.dptr, data.dsize, &lck);
@@ -319,7 +344,7 @@ static NTSTATUS g_lock_trylock(
bool self_exclusive = server_id_equal(&self, &lck.exclusive);
if (!self_exclusive) {
- exists = serverid_exists(&lck.exclusive);
+ bool exists = serverid_exists(&lck.exclusive);
if (!exists) {
lck.exclusive = (struct server_id) { .pid=0 };
goto noexclusive;
@@ -465,31 +490,7 @@ do_shared:
return status;
}
- /*
- * Read locks can stay around forever if the process dies. Do
- * a heuristic check for process existence: Check one random
- * process for existence. Hopefully this will keep runaway
- * read locks under control.
- */
-
- i = generate_random() % lck.num_shared;
-
- g_lock_get_shared(&lck, i, &check);
-
- if (!serverid_exists(&check)) {
-
- DBG_DEBUG("Shared locker %s died -- removing\n",
- server_id_str_buf(check, &tmp));
-
- g_lock_del_shared(&lck, i);
-
- status = g_lock_store(rec, &lck, NULL);
- if (!NT_STATUS_IS_OK(status)) {
- DBG_DEBUG("g_lock_store() failed: %s\n",
- nt_errstr(status));
- return status;
- }
- }
+ g_lock_cleanup_shared(&lck);
status = g_lock_store(rec, &lck, &self);
if (!NT_STATUS_IS_OK(status)) {
@@ -652,14 +653,98 @@ NTSTATUS g_lock_lock_recv(struct tevent_req *req)
return tevent_req_simple_recv_ntstatus(req);
}
+struct g_lock_lock_simple_state {
+ struct server_id me;
+ enum g_lock_type type;
+ NTSTATUS status;
+};
+
+static void g_lock_lock_simple_fn(
+ struct db_record *rec,
+ TDB_DATA value,
+ void *private_data)
+{
+ struct g_lock_lock_simple_state *state = private_data;
+ struct g_lock lck = { .exclusive.pid = 0 };
+ bool ok;
+
+ ok = g_lock_parse(value.dptr, value.dsize, &lck);
+ if (!ok) {
+ DBG_DEBUG("g_lock_parse failed\n");
+ state->status = NT_STATUS_INTERNAL_DB_CORRUPTION;
+ return;
+ }
+
+ if (lck.exclusive.pid != 0) {
+ goto not_granted;
+ }
+
+ if (state->type == G_LOCK_WRITE) {
+ if (lck.num_shared != 0) {
+ goto not_granted;
+ }
+ lck.exclusive = state->me;
+ state->status = g_lock_store(rec, &lck, NULL);
+ return;
+ }
+
+ if (state->type == G_LOCK_READ) {
+ g_lock_cleanup_shared(&lck);
+ state->status = g_lock_store(rec, &lck, &state->me);
+ return;
+ }
+
+not_granted:
+ state->status = NT_STATUS_LOCK_NOT_GRANTED;
+}
+
NTSTATUS g_lock_lock(struct g_lock_ctx *ctx, TDB_DATA key,
enum g_lock_type type, struct timeval timeout)
{
- TALLOC_CTX *frame = talloc_stackframe();
+ TALLOC_CTX *frame;
struct tevent_context *ev;
struct tevent_req *req;
struct timeval end;
- NTSTATUS status = NT_STATUS_NO_MEMORY;
+ NTSTATUS status;
+
+ if ((type == G_LOCK_READ) || (type == G_LOCK_WRITE)) {
+ /*
+ * This is an abstraction violation: Normally we do
+ * the sync wrappers around async functions with full
+ * nested event contexts. However, this is used in
+ * very hot code paths, so avoid the event context
+ * creation for the good path where there's no lock
+ * contention. My benchmark gave a factor of 2
+ * improvement for lock/unlock.
+ */
+ struct g_lock_lock_simple_state state = {
+ .me = messaging_server_id(ctx->msg),
+ .type = type,
+ };
+ status = dbwrap_do_locked(
+ ctx->db, key, g_lock_lock_simple_fn, &state);
+ if (!NT_STATUS_IS_OK(status)) {
+ DBG_DEBUG("dbwrap_do_locked() failed: %s\n",
+ nt_errstr(status));
+ return status;
+ }
+ if (NT_STATUS_IS_OK(state.status)) {
+ return NT_STATUS_OK;
+ }
+ if (!NT_STATUS_EQUAL(
+ state.status, NT_STATUS_LOCK_NOT_GRANTED)) {
+ return state.status;
+ }
+
+ /*
+ * Fall back to the full g_lock_trylock logic,
+ * g_lock_lock_simple_fn() called above only covers
+ * the uncontended path.
+ */
+ }
+
+ frame = talloc_stackframe();
+ status = NT_STATUS_NO_MEMORY;
ev = samba_tevent_context_init(frame);
if (ev == NULL) {
diff --git a/source3/lib/sharesec.c b/source3/lib/sharesec.c
index 58a2dee3bcf..acbdd8b5df9 100644
--- a/source3/lib/sharesec.c
+++ b/source3/lib/sharesec.c
@@ -24,6 +24,7 @@
#include "dbwrap/dbwrap.h"
#include "dbwrap/dbwrap_open.h"
#include "util_tdb.h"
+#include "libcli/util/ntstatus.h"
/*******************************************************************
Create the share security tdb.
@@ -136,7 +137,7 @@ static int upgrade_v2_to_v3(struct db_record *rec, void *priv)
return 0;
}
-bool share_info_db_init(void)
+NTSTATUS share_info_db_init(void)
{
const char *vstring = "INFO/version";
int32_t vers_id = 0;
@@ -145,12 +146,12 @@ bool share_info_db_init(void)
char *db_path;
if (share_db != NULL) {
- return True;
+ return NT_STATUS_OK;
}
db_path = state_path(talloc_tos(), "share_info.tdb");
if (db_path == NULL) {
- return false;
+ return NT_STATUS_NO_MEMORY;
}
share_db = db_open(NULL, db_path, 0,
@@ -160,7 +161,7 @@ bool share_info_db_init(void)
DEBUG(0,("Failed to open share info database %s (%s)\n",
db_path, strerror(errno)));
TALLOC_FREE(db_path);
- return False;
+ return map_nt_error_from_unix_common(errno);
}
TALLOC_FREE(db_path);
@@ -170,13 +171,13 @@ bool share_info_db_init(void)
}
if (vers_id == SHARE_DATABASE_VERSION_V3) {
- return true;
+ return NT_STATUS_OK;
}
if (dbwrap_transaction_start(share_db) != 0) {
DEBUG(0, ("transaction_start failed\n"));
TALLOC_FREE(share_db);
- return false;
+ return NT_STATUS_INTERNAL_DB_ERROR;
}
status = dbwrap_fetch_int32_bystring(share_db, vstring, &vers_id);
@@ -191,7 +192,7 @@ bool share_info_db_init(void)
if (dbwrap_transaction_cancel(share_db)) {
smb_panic("transaction_cancel failed");
}
- return true;
+ return NT_STATUS_OK;
}
/* Move to at least V2. */
@@ -228,10 +229,16 @@ bool share_info_db_init(void)
/* Finally upgrade to version 3, with canonicalized sharenames. */
status = dbwrap_traverse(share_db, upgrade_v2_to_v3, &upgrade_ok, NULL);
- if (!NT_STATUS_IS_OK(status) || upgrade_ok == false) {
+ if (!NT_STATUS_IS_OK(status)) {
DEBUG(0, ("traverse failed\n"));
goto cancel;
}
+ if (!upgrade_ok) {
+ DBG_ERR("upgrade failed.\n");
+ status = NT_STATUS_INTERNAL_ERROR;
+ goto cancel;
+ }
+
status = dbwrap_store_int32_bystring(
share_db, vstring, SHARE_DATABASE_VERSION_V3);
if (!NT_STATUS_IS_OK(status)) {
@@ -242,17 +249,17 @@ bool share_info_db_init(void)
if (dbwrap_transaction_commit(share_db) != 0) {
DEBUG(0, ("transaction_commit failed\n"));
- return false;
+ return NT_STATUS_INTERNAL_ERROR;
}
- return true;
+ return NT_STATUS_OK;
cancel:
if (dbwrap_transaction_cancel(share_db)) {
smb_panic("transaction_cancel failed");
}
- return false;
+ return status;
}
/*******************************************************************
@@ -304,7 +311,8 @@ struct security_descriptor *get_share_security( TALLOC_CTX *ctx, const char *ser
return NULL;
}
- if (!share_info_db_init()) {
+ status = share_info_db_init();
+ if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(c_servicename);
return NULL;
}
@@ -349,20 +357,22 @@ struct security_descriptor *get_share_security( TALLOC_CTX *ctx, const char *ser
Store a security descriptor in the share db.
********************************************************************/
-bool set_share_security(const char *share_name, struct security_descriptor *psd)
+NTSTATUS set_share_security(const char *share_name,
+ struct security_descriptor *psd)
{
TALLOC_CTX *frame = talloc_stackframe();
char *key;
- bool ret = False;
TDB_DATA blob;
NTSTATUS status;
char *c_share_name = canonicalize_servicename(frame, share_name);
- if (!c_share_name) {
+ if (c_share_name == NULL) {
+ status = NT_STATUS_INVALID_PARAMETER;
goto out;
}
- if (!share_info_db_init()) {
+ status = share_info_db_init();
+ if (!NT_STATUS_IS_OK(status)) {
goto out;
}
@@ -376,6 +386,7 @@ bool set_share_security(const char *share_name, struct security_descriptor *psd)
if (!(key = talloc_asprintf(frame, SHARE_SECURITY_DB_KEY_PREFIX_STR "%s", c_share_name))) {
DEBUG(0, ("talloc_asprintf failed\n"));
+ status = NT_STATUS_NO_MEMORY;
goto out;
}
@@ -388,37 +399,38 @@ bool set_share_security(const char *share_name, struct security_descriptor *psd)
}
DEBUG(5,("set_share_security: stored secdesc for %s\n", share_name ));
- ret = True;
+ status = NT_STATUS_OK;
out:
TALLOC_FREE(frame);
- return ret;
+ return status;
}
/*******************************************************************
Delete a security descriptor.
********************************************************************/
-bool delete_share_security(const char *servicename)
+NTSTATUS delete_share_security(const char *servicename)
{
TDB_DATA kbuf;
char *key;
NTSTATUS status;
char *c_servicename = canonicalize_servicename(talloc_tos(), servicename);
- if (!c_servicename) {
- return NULL;
+ if (c_servicename == NULL) {
+ return NT_STATUS_INVALID_PARAMETER;
}
- if (!share_info_db_init()) {
+ status = share_info_db_init();
+ if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(c_servicename);
- return False;
+ return status;
}
if (!(key = talloc_asprintf(talloc_tos(), SHARE_SECURITY_DB_KEY_PREFIX_STR "%s",
c_servicename))) {
TALLOC_FREE(c_servicename);
- return False;
+ return NT_STATUS_NO_MEMORY;
}
kbuf = string_term_tdb_data(key);
@@ -427,11 +439,11 @@ bool delete_share_security(const char *servicename)
DEBUG(0, ("delete_share_security: Failed to delete entry for "
"share %s: %s\n", c_servicename, nt_errstr(status)));
TALLOC_FREE(c_servicename);
- return False;
+ return status;
}
TALLOC_FREE(c_servicename);
- return True;
+ return NT_STATUS_OK;
}
/*******************************************************************
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 31fa229d5ff..ff11146fe98 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -3329,6 +3329,7 @@ static int process_usershare_file(const char *dir_name, const char *file_name, i
char *canon_name = NULL;
bool added_service = false;
int ret = -1;
+ NTSTATUS status;
/* Ensure share name doesn't contain invalid characters. */
if (!validate_net_name(file_name, INVALID_SHARENAME_CHARS, strlen(file_name))) {
@@ -3365,7 +3366,6 @@ static int process_usershare_file(const char *dir_name, const char *file_name, i
{
TDB_DATA data;
- NTSTATUS status;
status = dbwrap_fetch_bystring(ServiceHash, canon_name,
canon_name, &data);
@@ -3462,7 +3462,8 @@ static int process_usershare_file(const char *dir_name, const char *file_name, i
}
/* Write the ACL of the new/modified share. */
- if (!set_share_security(canon_name, psd)) {
+ status = set_share_security(canon_name, psd);
+ if (!NT_STATUS_IS_OK(status)) {
DEBUG(0, ("process_usershare_file: Failed to set share "
"security for user share %s\n",
canon_name ));
diff --git a/source3/rpc_server/fss/srv_fss_agent.c b/source3/rpc_server/fss/srv_fss_agent.c
index 13b4806e6a2..925b68e9fa2 100644
--- a/source3/rpc_server/fss/srv_fss_agent.c
+++ b/source3/rpc_server/fss/srv_fss_agent.c
@@ -1099,10 +1099,10 @@ static uint32_t fss_sc_expose(struct smbconf_ctx *fconf_ctx,
DEBUG(2, ("no share SD to clone for %s snapshot\n",
sc_smap->share_name));
} else {
- bool ok;
- ok = set_share_security(sc_smap->sc_share_name, sd);
+ NTSTATUS status;
+ status = set_share_security(sc_smap->sc_share_name, sd);
TALLOC_FREE(sd);
- if (!ok) {
+ if (!NT_STATUS_IS_OK(status)) {
DEBUG(0, ("failed to set %s share SD\n",
sc_smap->sc_share_name));
err = HRES_ERROR_V(HRES_E_FAIL);
diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
index 34a227c76e9..c0d74bb7af4 100644
--- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
+++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c
@@ -1936,13 +1936,16 @@ WERROR _srvsvc_NetShareSetInfo(struct pipes_struct *p,
if (psd) {
struct security_descriptor *old_sd;
size_t sd_size;
+ NTSTATUS status;
old_sd = get_share_security(p->mem_ctx, lp_servicename(talloc_tos(), snum), &sd_size);
if (old_sd && !security_descriptor_equal(old_sd, psd)) {
- if (!set_share_security(share_name, psd))
+ status = set_share_security(share_name, psd);
+ if (!NT_STATUS_IS_OK(status)) {
DEBUG(0,("_srvsvc_NetShareSetInfo: Failed to change security info in share %s.\n",
share_name ));
+ }
}
}
--
Samba Shared Repository
More information about the samba-cvs
mailing list