[PATCH] A bugfix (IMHO) for namemap_cache
Volker Lendecke
Volker.Lendecke at SerNet.DE
Mon Oct 15 13:54:38 UTC 2018
Hi!
Unfortunately this failed twice in
[781(4738)/849 at 2h7m25s] samba4.ldap.password_lockout.python(ad_dc_ntvfs)(ad_dc_ntvfs)
so we can't be sure that this is right yet. But nevertheless I would
appreciate feedback, because I am missing the overview how this patch
interacts with the AD DC code in a way to kill it.
Anybody with an idea?
Thanks,
Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 869d395307fea00a7cf065ccd0092ca192205492 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 13 Oct 2018 11:39:03 +0200
Subject: [PATCH 1/3] winbindd_cache: Fix timeout calculation for sid<->name
cache
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/winbindd_cache.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
index 2f3bac7587b..be3e5ffc2ad 100644
--- a/source3/winbindd/winbindd_cache.c
+++ b/source3/winbindd/winbindd_cache.c
@@ -1749,7 +1749,7 @@ static void wcache_name_to_sid_fn(const struct dom_sid *sid,
*state->sid = *sid;
*state->type = type;
- state->found = (state->offline || (timeout < time(NULL)));
+ state->found = (state->offline || (timeout > time(NULL)));
}
static NTSTATUS wcache_name_to_sid(struct winbindd_domain *domain,
@@ -1884,7 +1884,7 @@ static void wcache_sid_to_name_fn(const char *domain, const char *name,
return;
}
*state->type = type;
- state->found = (state->offline || (timeout < time(NULL)));
+ state->found = (state->offline || (timeout > time(NULL)));
}
static NTSTATUS wcache_sid_to_name(struct winbindd_domain *domain,
--
2.11.0
From 9c5ac362fac56e42417303dbe3e890ec561fc5cb Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 13 Oct 2018 12:01:41 +0200
Subject: [PATCH 2/3] namemap_cache: Absorb the expired calculation into
namemap_cache.c
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/namemap_cache.c | 31 ++++++++++++++++++++++---------
source3/lib/namemap_cache.h | 12 ++++++++----
source3/torture/test_namemap_cache.c | 27 ++++++++++++++++++---------
source3/winbindd/winbindd_cache.c | 13 ++++++++-----
4 files changed, 56 insertions(+), 27 deletions(-)
diff --git a/source3/lib/namemap_cache.c b/source3/lib/namemap_cache.c
index 38f7a2ddc88..f6e71cd6d16 100644
--- a/source3/lib/namemap_cache.c
+++ b/source3/lib/namemap_cache.c
@@ -85,8 +85,10 @@ fail:
}
struct namemap_cache_find_sid_state {
- void (*fn)(const char *domain, const char *name,
- enum lsa_SidType type, time_t timeout,
+ void (*fn)(const char *domain,
+ const char *name,
+ enum lsa_SidType type,
+ bool expired,
void *private_data);
void *private_data;
bool ok;
@@ -127,15 +129,20 @@ static void namemap_cache_find_sid_parser(time_t timeout, DATA_BLOB blob,
return;
}
- state->fn(domain, name, (enum lsa_SidType)type, timeout,
+ state->fn(domain,
+ name,
+ (enum lsa_SidType)type,
+ timeout <= time(NULL),
state->private_data);
state->ok = true;
}
bool namemap_cache_find_sid(const struct dom_sid *sid,
- void (*fn)(const char *domain, const char *name,
- enum lsa_SidType type, time_t timeout,
+ void (*fn)(const char *domain,
+ const char *name,
+ enum lsa_SidType type,
+ bool expired,
void *private_data),
void *private_data)
{
@@ -227,7 +234,8 @@ fail:
struct namemap_cache_find_name_state {
void (*fn)(const struct dom_sid *sid,
- enum lsa_SidType type, time_t timeout,
+ enum lsa_SidType type,
+ bool expired,
void *private_data);
void *private_data;
bool ok;
@@ -274,14 +282,19 @@ static void namemap_cache_find_name_parser(time_t timeout, DATA_BLOB blob,
return;
}
- state->fn(&sid, (enum lsa_SidType)type, timeout, state->private_data);
+ state->fn(&sid,
+ (enum lsa_SidType)type,
+ timeout <= time(NULL),
+ state->private_data);
state->ok = true;
}
-bool namemap_cache_find_name(const char *domain, const char *name,
+bool namemap_cache_find_name(const char *domain,
+ const char *name,
void (*fn)(const struct dom_sid *sid,
- enum lsa_SidType type, time_t timeout,
+ enum lsa_SidType type,
+ bool expired,
void *private_data),
void *private_data)
{
diff --git a/source3/lib/namemap_cache.h b/source3/lib/namemap_cache.h
index a70de34b885..5de8ce4c0f2 100644
--- a/source3/lib/namemap_cache.h
+++ b/source3/lib/namemap_cache.h
@@ -32,13 +32,17 @@ bool namemap_cache_set_name2sid(const char *domain, const char *name,
enum lsa_SidType type,
time_t timeout);
bool namemap_cache_find_sid(const struct dom_sid *sid,
- void (*fn)(const char *domain, const char *name,
- enum lsa_SidType type, time_t timeout,
+ void (*fn)(const char *domain,
+ const char *name,
+ enum lsa_SidType type,
+ bool expired,
void *private_data),
void *private_data);
-bool namemap_cache_find_name(const char *domain, const char *name,
+bool namemap_cache_find_name(const char *domain,
+ const char *name,
void (*fn)(const struct dom_sid *sid,
- enum lsa_SidType type, time_t timeout,
+ enum lsa_SidType type,
+ bool expired,
void *private_data),
void *private_data);
diff --git a/source3/torture/test_namemap_cache.c b/source3/torture/test_namemap_cache.c
index 07c6bf4744e..01dd1b7c8dc 100644
--- a/source3/torture/test_namemap_cache.c
+++ b/source3/torture/test_namemap_cache.c
@@ -26,8 +26,10 @@ static const struct dom_sid domsid = {
1, 4, {0,0,0,0,0,5}, {21, 123, 456, 789}
};
-static void namemap_cache1_fn1(const char *domain, const char *name,
- enum lsa_SidType type, time_t timeout,
+static void namemap_cache1_fn1(const char *domain,
+ const char *name,
+ enum lsa_SidType type,
+ bool expired,
void *private_data)
{
bool *p_ok = private_data;
@@ -41,7 +43,8 @@ static void namemap_cache1_fn1(const char *domain, const char *name,
}
static void namemap_cache1_fn2(const struct dom_sid *sid,
- enum lsa_SidType type, time_t timeout,
+ enum lsa_SidType type,
+ bool expired,
void *private_data)
{
bool *p_ok = private_data;
@@ -53,8 +56,10 @@ static void namemap_cache1_fn2(const struct dom_sid *sid,
*p_ok = ok;
}
-static void namemap_cache1_fn3(const char *domain, const char *name,
- enum lsa_SidType type, time_t timeout,
+static void namemap_cache1_fn3(const char *domain,
+ const char *name,
+ enum lsa_SidType type,
+ bool expired,
void *private_data)
{
bool *p_ok = private_data;
@@ -68,7 +73,8 @@ static void namemap_cache1_fn3(const char *domain, const char *name,
}
static void namemap_cache1_fn4(const struct dom_sid *sid,
- enum lsa_SidType type, time_t timeout,
+ enum lsa_SidType type,
+ bool expired,
void *private_data)
{
bool *p_ok = private_data;
@@ -80,8 +86,10 @@ static void namemap_cache1_fn4(const struct dom_sid *sid,
*p_ok = ok;
}
-static void namemap_cache1_fn5(const char *domain, const char *name,
- enum lsa_SidType type, time_t timeout,
+static void namemap_cache1_fn5(const char *domain,
+ const char *name,
+ enum lsa_SidType type,
+ bool expired,
void *private_data)
{
bool *p_ok = private_data;
@@ -95,7 +103,8 @@ static void namemap_cache1_fn5(const char *domain, const char *name,
}
static void namemap_cache1_fn6(const struct dom_sid *sid,
- enum lsa_SidType type, time_t timeout,
+ enum lsa_SidType type,
+ bool expired,
void *private_data)
{
bool *p_ok = private_data;
diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
index be3e5ffc2ad..7d98d613ac4 100644
--- a/source3/winbindd/winbindd_cache.c
+++ b/source3/winbindd/winbindd_cache.c
@@ -1742,14 +1742,15 @@ struct wcache_name_to_sid_state {
};
static void wcache_name_to_sid_fn(const struct dom_sid *sid,
- enum lsa_SidType type, time_t timeout,
+ enum lsa_SidType type,
+ bool expired,
void *private_data)
{
struct wcache_name_to_sid_state *state = private_data;
*state->sid = *sid;
*state->type = type;
- state->found = (state->offline || (timeout > time(NULL)));
+ state->found = (!expired || state->offline);
}
static NTSTATUS wcache_name_to_sid(struct winbindd_domain *domain,
@@ -1869,8 +1870,10 @@ struct wcache_sid_to_name_state {
bool found;
};
-static void wcache_sid_to_name_fn(const char *domain, const char *name,
- enum lsa_SidType type, time_t timeout,
+static void wcache_sid_to_name_fn(const char *domain,
+ const char *name,
+ enum lsa_SidType type,
+ bool expired,
void *private_data)
{
struct wcache_sid_to_name_state *state = private_data;
@@ -1884,7 +1887,7 @@ static void wcache_sid_to_name_fn(const char *domain, const char *name,
return;
}
*state->type = type;
- state->found = (state->offline || (timeout > time(NULL)));
+ state->found = (!expired || state->offline);
}
static NTSTATUS wcache_sid_to_name(struct winbindd_domain *domain,
--
2.11.0
From 6d551337a957503d5975a7cc08a678c3e8307f17 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 13 Oct 2018 13:41:59 +0200
Subject: [PATCH 3/3] lib: Move the "expired" for gencache_parse calculation
into gencache.c
Make it more robust
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/gencache.c | 38 ++++++++++++++++++++++++++------------
source3/lib/gencache.h | 11 ++++++++++-
source3/lib/idmap_cache.c | 5 +++--
source3/lib/namemap_cache.c | 16 ++++++++++------
source3/lib/util.c | 6 ++++--
source3/torture/torture.c | 4 +++-
source3/winbindd/wb_dsgetdcname.c | 6 ++++--
7 files changed, 60 insertions(+), 26 deletions(-)
diff --git a/source3/lib/gencache.c b/source3/lib/gencache.c
index b4d374bf837..787ef981ec0 100644
--- a/source3/lib/gencache.c
+++ b/source3/lib/gencache.c
@@ -42,6 +42,14 @@ static struct tdb_wrap *cache_notrans;
*
**/
+struct gencache_timeout {
+ time_t timeout;
+};
+
+bool gencache_timeout_expired(const struct gencache_timeout *t)
+{
+ return t->timeout <= time(NULL);
+}
/**
* Cache initialisation function. Opens cache tdb file or creates
@@ -131,7 +139,8 @@ struct gencache_have_val_state {
bool gotit;
};
-static void gencache_have_val_parser(time_t old_timeout, DATA_BLOB data,
+static void gencache_have_val_parser(const struct gencache_timeout *old_timeout,
+ DATA_BLOB data,
void *private_data)
{
struct gencache_have_val_state *state =
@@ -146,7 +155,7 @@ static void gencache_have_val_parser(time_t old_timeout, DATA_BLOB data,
* value, just extending the remaining timeout by less than 10%.
*/
- cache_time_left = old_timeout - now;
+ cache_time_left = old_timeout->timeout - now;
if (cache_time_left <= 0) {
/*
* timed out, write new value
@@ -338,10 +347,11 @@ done:
return ret == 0;
}
-static void gencache_del_parser(time_t timeout, DATA_BLOB blob,
+static void gencache_del_parser(const struct gencache_timeout *t,
+ DATA_BLOB blob,
void *private_data)
{
- if (timeout != 0) {
+ if (t->timeout != 0) {
bool *exists = private_data;
*exists = true;
}
@@ -419,7 +429,9 @@ static bool gencache_pull_timeout(uint8_t *val, time_t *pres, char **payload)
}
struct gencache_parse_state {
- void (*parser)(time_t timeout, DATA_BLOB blob, void *private_data);
+ void (*parser)(const struct gencache_timeout *timeout,
+ DATA_BLOB blob,
+ void *private_data);
void *private_data;
bool copy_to_notrans;
};
@@ -428,21 +440,21 @@ static int gencache_parse_fn(TDB_DATA key, TDB_DATA data, void *private_data)
{
struct gencache_parse_state *state;
DATA_BLOB blob;
- time_t t;
+ struct gencache_timeout t;
char *payload;
bool ret;
if (data.dptr == NULL) {
return -1;
}
- ret = gencache_pull_timeout(data.dptr, &t, &payload);
+ ret = gencache_pull_timeout(data.dptr, &t.timeout, &payload);
if (!ret) {
return -1;
}
state = (struct gencache_parse_state *)private_data;
blob = data_blob_const(
payload, data.dsize - PTR_DIFF(payload, data.dptr));
- state->parser(t, blob, state->private_data);
+ state->parser(&t, blob, state->private_data);
if (state->copy_to_notrans) {
tdb_store(cache_notrans->tdb, key, data, 0);
@@ -452,7 +464,8 @@ static int gencache_parse_fn(TDB_DATA key, TDB_DATA data, void *private_data)
}
bool gencache_parse(const char *keystr,
- void (*parser)(time_t timeout, DATA_BLOB blob,
+ void (*parser)(const struct gencache_timeout *timeout,
+ DATA_BLOB blob,
void *private_data),
void *private_data)
{
@@ -511,17 +524,18 @@ struct gencache_get_data_blob_state {
bool result;
};
-static void gencache_get_data_blob_parser(time_t timeout, DATA_BLOB blob,
+static void gencache_get_data_blob_parser(const struct gencache_timeout *t,
+ DATA_BLOB blob,
void *private_data)
{
struct gencache_get_data_blob_state *state =
(struct gencache_get_data_blob_state *)private_data;
- if (timeout == 0) {
+ if (t->timeout == 0) {
state->result = false;
return;
}
- state->timeout = timeout;
+ state->timeout = t->timeout;
if (state->blob == NULL) {
state->result = true;
diff --git a/source3/lib/gencache.h b/source3/lib/gencache.h
index fa72a4aa466..3d8ebe53872 100644
--- a/source3/lib/gencache.h
+++ b/source3/lib/gencache.h
@@ -32,8 +32,17 @@ bool gencache_set(const char *keystr, const char *value, time_t timeout);
bool gencache_del(const char *keystr);
bool gencache_get(const char *keystr, TALLOC_CTX *mem_ctx, char **value,
time_t *ptimeout);
+
+/*
+ * This might look like overkill, but namemap_cache.c shows it's
+ * necessary :-)
+ */
+struct gencache_timeout;
+bool gencache_timeout_expired(const struct gencache_timeout *t);
+
bool gencache_parse(const char *keystr,
- void (*parser)(time_t timeout, DATA_BLOB blob,
+ void (*parser)(const struct gencache_timeout *timeout,
+ DATA_BLOB blob,
void *private_data),
void *private_data);
bool gencache_get_data_blob(const char *keystr, TALLOC_CTX *mem_ctx,
diff --git a/source3/lib/idmap_cache.c b/source3/lib/idmap_cache.c
index 1e8a1ebc607..187723689ad 100644
--- a/source3/lib/idmap_cache.c
+++ b/source3/lib/idmap_cache.c
@@ -194,7 +194,8 @@ struct idmap_cache_xid2sid_state {
bool ret;
};
-static void idmap_cache_xid2sid_parser(time_t timeout, DATA_BLOB blob,
+static void idmap_cache_xid2sid_parser(const struct gencache_timeout *timeout,
+ DATA_BLOB blob,
void *private_data)
{
struct idmap_cache_xid2sid_state *state =
@@ -217,7 +218,7 @@ static void idmap_cache_xid2sid_parser(time_t timeout, DATA_BLOB blob,
state->ret = string_to_sid(state->sid, value);
}
if (state->ret) {
- *state->expired = (timeout <= time(NULL));
+ *state->expired = gencache_timeout_expired(timeout);
}
}
diff --git a/source3/lib/namemap_cache.c b/source3/lib/namemap_cache.c
index f6e71cd6d16..c153aa608e2 100644
--- a/source3/lib/namemap_cache.c
+++ b/source3/lib/namemap_cache.c
@@ -94,8 +94,10 @@ struct namemap_cache_find_sid_state {
bool ok;
};
-static void namemap_cache_find_sid_parser(time_t timeout, DATA_BLOB blob,
- void *private_data)
+static void namemap_cache_find_sid_parser(
+ const struct gencache_timeout *timeout,
+ DATA_BLOB blob,
+ void *private_data)
{
struct namemap_cache_find_sid_state *state = private_data;
const char *strv = (const char *)blob.data;
@@ -132,7 +134,7 @@ static void namemap_cache_find_sid_parser(time_t timeout, DATA_BLOB blob,
state->fn(domain,
name,
(enum lsa_SidType)type,
- timeout <= time(NULL),
+ gencache_timeout_expired(timeout),
state->private_data);
state->ok = true;
@@ -241,8 +243,10 @@ struct namemap_cache_find_name_state {
bool ok;
};
-static void namemap_cache_find_name_parser(time_t timeout, DATA_BLOB blob,
- void *private_data)
+static void namemap_cache_find_name_parser(
+ const struct gencache_timeout *timeout,
+ DATA_BLOB blob,
+ void *private_data)
{
struct namemap_cache_find_name_state *state = private_data;
const char *strv = (const char *)blob.data;
@@ -284,7 +288,7 @@ static void namemap_cache_find_name_parser(time_t timeout, DATA_BLOB blob,
state->fn(&sid,
(enum lsa_SidType)type,
- timeout <= time(NULL),
+ gencache_timeout_expired(timeout),
state->private_data);
state->ok = true;
diff --git a/source3/lib/util.c b/source3/lib/util.c
index 394fa5fd191..da8ec55fa61 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -1242,12 +1242,14 @@ struct ra_parser_state {
enum remote_arch_types ra;
};
-static void ra_parser(time_t timeout, DATA_BLOB blob, void *priv_data)
+static void ra_parser(const struct gencache_timeout *t,
+ DATA_BLOB blob,
+ void *priv_data)
{
struct ra_parser_state *state = (struct ra_parser_state *)priv_data;
const char *ra_str = NULL;
- if (timeout <= time(NULL)) {
+ if (gencache_timeout_expired(t)) {
return;
}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 098560968dd..3053596b439 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -10238,7 +10238,9 @@ static bool run_local_base64(int dummy)
return ret;
}
-static void parse_fn(time_t timeout, DATA_BLOB blob, void *private_data)
+static void parse_fn(const struct gencache_timeout *t,
+ DATA_BLOB blob,
+ void *private_data)
{
return;
}
diff --git a/source3/winbindd/wb_dsgetdcname.c b/source3/winbindd/wb_dsgetdcname.c
index 2f450c7a2b4..cbd2cf6fd78 100644
--- a/source3/winbindd/wb_dsgetdcname.c
+++ b/source3/winbindd/wb_dsgetdcname.c
@@ -171,12 +171,14 @@ struct dcinfo_parser_state {
struct netr_DsRGetDCNameInfo *dcinfo;
};
-static void dcinfo_parser(time_t timeout, DATA_BLOB blob, void *private_data)
+static void dcinfo_parser(const struct gencache_timeout *timeout,
+ DATA_BLOB blob,
+ void *private_data)
{
struct dcinfo_parser_state *state = private_data;
enum ndr_err_code ndr_err;
- if (timeout <= time(NULL)) {
+ if (gencache_timeout_expired(timeout)) {
return;
}
--
2.11.0
More information about the samba-technical
mailing list