[PATCH] Add remote arch caching
Ralph Boehme
slow at samba.org
Tue Apr 26 08:18:07 UTC 2016
Hi!
On Mon, Apr 25, 2016 at 09:38:49PM +0200, Volker Lendecke wrote:
> > const char *get_remote_arch_str(void)
> > {
> > - if (!remote_arch_str) {
> > - return "UNKNOWN";
> > - }
> > - return remote_arch_str;
> > + return remote_arch_strings[ra_type];
> > }
>
> Question -- do we want range checking here? I don't think compilers
> enforce the enum ranges.
the range checking is already done at set time, so I had skipped it
here. It doesn't hurt and adds a safety net, so I added it.
>
> > +enum remote_arch_types get_remote_arch_from_str(const char *remote_arch_string)
> > +{
> > + for (int i = 0; i < sizeof(remote_arch_strings); i++) {
>
> Shouldn't that be ARRAY_SIZE(remote_arch_strings)?
yes. Emberrassing, hiding away in shame.
> > + guid_string = GUID_string(mem_ctx, client_guid);
> > + if (guid_string == NULL) {
> > + return NULL;
> > + }
>
> Probably it does not matter significantly performance-wise, but we
> have GUID_buf_string(). Can we use that here?
yes, good idea.
> > + ra = get_remote_arch_from_str(ra_str);
> > + if (ra != RA_UNKNOWN) {
> > + DBG_INFO("Settting remote arch [%s] from cache\n", ra_str);
> > + set_remote_arch(ra);
> > + }
> > +
> > + return true;
> > +}
>
> Doesn't this leak "ra_str"? gencache_parse() would avoid that....
Using gencache_parse now as well as fstrings which got this comletely
rid of talloc.
Thanks!
-Ralph
-------------- next part --------------
From e2960c57211081b308766f11dc16a54e91e69707 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 13 Apr 2016 17:39:26 +0200
Subject: [PATCH 1/4] s3/lib: rework get_remote_arch_str() to use an array
By using C99 designated array initializers we can simplify the code and
remove the dependency on initializers appearing in a particular order.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/include/smb.h | 2 +-
source3/lib/util.c | 80 ++++++++++++++++++++++-----------------------------
2 files changed, 35 insertions(+), 47 deletions(-)
diff --git a/source3/include/smb.h b/source3/include/smb.h
index 7eeef88..dec3189 100644
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -529,7 +529,7 @@ http://msdn.microsoft.com/en-us/library/cc246334(PROT.13).aspx
#define NO_SUBSTREAMS 0x2
#define NO_REPARSETAG 0x4
-/* Remote architectures we know about. */
+/* Remote architectures we know about, keep in sync with remote_arch_strings */
enum remote_arch_types {RA_UNKNOWN, RA_WFWG, RA_OS2, RA_WIN95, RA_WINNT,
RA_WIN2K, RA_WINXP, RA_WIN2K3, RA_VISTA,
RA_SAMBA, RA_CIFSFS, RA_WINXP64, RA_OSX};
diff --git a/source3/lib/util.c b/source3/lib/util.c
index 2895c14..a02e0cf 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -1230,14 +1230,34 @@ void ra_lanman_string( const char *native_lanman )
set_remote_arch( RA_WIN2K3 );
}
-static const char *remote_arch_str;
+static const char *remote_arch_strings[] = {
+ [RA_UNKNOWN] = "UNKNOWN",
+ [RA_WFWG] = "WfWg",
+ [RA_OS2] = "OS2",
+ [RA_WIN95] = "Win95",
+ [RA_WINNT] = "WinNT",
+ [RA_WIN2K] = "Win2K",
+ [RA_WINXP] = "WinXP",
+ [RA_WIN2K3] = "Win2K3",
+ [RA_VISTA] = "Vista",
+ [RA_SAMBA] = "Samba",
+ [RA_CIFSFS] = "CIFSFS",
+ [RA_WINXP64] = "WinXP64",
+ [RA_OSX] = "OSX",
+};
const char *get_remote_arch_str(void)
{
- if (!remote_arch_str) {
- return "UNKNOWN";
+ if (ra_type >= ARRAY_SIZE(remote_arch_strings)) {
+ /*
+ * set_remote_arch() already checks this so ra_type
+ * should be in the allowed range, but anyway, let's
+ * do another bound check here.
+ */
+ DBG_ERR("Remote arch info out of sync [%d] missing\n", ra_type);
+ ra_type = RA_UNKNOWN;
}
- return remote_arch_str;
+ return remote_arch_strings[ra_type];
}
/*******************************************************************
@@ -1246,52 +1266,20 @@ const char *get_remote_arch_str(void)
void set_remote_arch(enum remote_arch_types type)
{
- ra_type = type;
- switch( type ) {
- case RA_WFWG:
- remote_arch_str = "WfWg";
- break;
- case RA_OS2:
- remote_arch_str = "OS2";
- break;
- case RA_WIN95:
- remote_arch_str = "Win95";
- break;
- case RA_WINNT:
- remote_arch_str = "WinNT";
- break;
- case RA_WIN2K:
- remote_arch_str = "Win2K";
- break;
- case RA_WINXP:
- remote_arch_str = "WinXP";
- break;
- case RA_WINXP64:
- remote_arch_str = "WinXP64";
- break;
- case RA_WIN2K3:
- remote_arch_str = "Win2K3";
- break;
- case RA_VISTA:
- remote_arch_str = "Vista";
- break;
- case RA_SAMBA:
- remote_arch_str = "Samba";
- break;
- case RA_CIFSFS:
- remote_arch_str = "CIFSFS";
- break;
- case RA_OSX:
- remote_arch_str = "OSX";
- break;
- default:
+ if (ra_type >= ARRAY_SIZE(remote_arch_strings)) {
+ /*
+ * This protects against someone adding values to enum
+ * remote_arch_types without updating
+ * remote_arch_strings array.
+ */
+ DBG_ERR("Remote arch info out of sync [%d] missing\n", ra_type);
ra_type = RA_UNKNOWN;
- remote_arch_str = "UNKNOWN";
- break;
+ return;
}
+ ra_type = type;
DEBUG(10,("set_remote_arch: Client arch is \'%s\'\n",
- remote_arch_str));
+ get_remote_arch_str()));
}
/*******************************************************************
--
2.5.0
From fbf33b8dcf8863aee3c10cde84dd38c79bbe3f6d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 13 Apr 2016 17:55:11 +0200
Subject: [PATCH 2/4] s3/lib: add get_remote_arch_from_str()
This will be used when fetching remote arch from gencache.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/include/proto.h | 1 +
source3/lib/util.c | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/source3/include/proto.h b/source3/include/proto.h
index afbbff0..8baf9d9 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -389,6 +389,7 @@ bool fcntl_getlock(int fd, off_t *poffset, off_t *pcount, int *ptype, pid_t *ppi
bool is_myname(const char *s);
void ra_lanman_string( const char *native_lanman );
const char *get_remote_arch_str(void);
+enum remote_arch_types get_remote_arch_from_str(const char *remote_arch_string);
void set_remote_arch(enum remote_arch_types type);
enum remote_arch_types get_remote_arch(void);
const char *tab_depth(int level, int depth);
diff --git a/source3/lib/util.c b/source3/lib/util.c
index a02e0cf..290224f 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -1260,6 +1260,16 @@ const char *get_remote_arch_str(void)
return remote_arch_strings[ra_type];
}
+enum remote_arch_types get_remote_arch_from_str(const char *remote_arch_string)
+{
+ for (int i = 0; i < ARRAY_SIZE(remote_arch_strings); i++) {
+ if (strcmp(remote_arch_string, remote_arch_strings[i]) == 0) {
+ return i;
+ }
+ }
+ return RA_UNKNOWN;
+}
+
/*******************************************************************
Set the horrid remote_arch string based on an enum.
********************************************************************/
--
2.5.0
From b26289a15bb6c648e4f1a004feb8a12efaf04cd5 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 13 Apr 2016 17:42:55 +0200
Subject: [PATCH 3/4] s3/lib: add remote arch caching
This allows caching the remote arch string in gencache. A subsequent
commit will use this in SMB2 negprot.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/include/proto.h | 2 +
source3/lib/util.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 136 insertions(+)
diff --git a/source3/include/proto.h b/source3/include/proto.h
index 8baf9d9..c032b9e 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -392,6 +392,8 @@ const char *get_remote_arch_str(void);
enum remote_arch_types get_remote_arch_from_str(const char *remote_arch_string);
void set_remote_arch(enum remote_arch_types type);
enum remote_arch_types get_remote_arch(void);
+bool remote_arch_cache_update(const struct GUID *client_guid);
+bool remote_arch_cache_delete(const struct GUID *client_guid);
const char *tab_depth(int level, int depth);
int str_checksum(const char *s);
void zero_free(void *p, size_t size);
diff --git a/source3/lib/util.c b/source3/lib/util.c
index 290224f..1b7fc37 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -1301,6 +1301,140 @@ enum remote_arch_types get_remote_arch(void)
return ra_type;
}
+#define RA_CACHE_TTL 7*24*3600
+
+static bool remote_arch_cache_key(const struct GUID *client_guid,
+ fstring key)
+{
+ struct GUID_txt_buf guid_buf;
+ char *guid_string = NULL;
+
+ guid_string = GUID_buf_string(client_guid, &guid_buf);
+ if (guid_string == NULL) {
+ return false;
+ }
+
+ fstr_sprintf(key, "RA/%s", guid_string);
+ return true;
+}
+
+struct ra_parser_state {
+ bool ok;
+ bool expired;
+ enum remote_arch_types ra;
+};
+
+static void ra_parser(time_t timeout, DATA_BLOB blob, void *priv_data)
+{
+ struct ra_parser_state *state = (struct ra_parser_state *)priv_data;
+
+ if ((blob.data[0] == '\0') || (blob.data[blob.length-1] != '\0')) {
+ DBG_ERR("Remote arch cache key not a string\n");
+ return;
+ }
+
+ if (time(NULL) > timeout) {
+ state->expired = true;
+ state->ok = true;
+ return;
+ }
+
+ DBG_INFO("Got remote arch [%s] from cache\n", (char *)blob.data);
+
+ state->ra = get_remote_arch_from_str((char *)blob.data);
+ state->ok = true;
+ return;
+}
+
+static bool remote_arch_cache_get(const struct GUID *client_guid)
+{
+ bool ok;
+ fstring ra_key;
+ struct ra_parser_state state;
+
+ state = (struct ra_parser_state) {
+ .ok = false,
+ .expired = false,
+ .ra = RA_UNKNOWN,
+ };
+
+ ok = remote_arch_cache_key(client_guid, ra_key);
+ if (!ok) {
+ return false;
+ }
+
+ ok = gencache_parse(ra_key, ra_parser, &state);
+ if (!ok || !state.ok) {
+ return false;
+ }
+
+ if (state.expired) {
+ return gencache_del(ra_key);
+ }
+
+ if (state.ra == RA_UNKNOWN) {
+ return true;
+ }
+
+ set_remote_arch(state.ra);
+ return true;
+}
+
+static bool remote_arch_cache_set(const struct GUID *client_guid)
+{
+ bool ok;
+ fstring ra_key;
+ const char *ra_str = NULL;
+
+ if (get_remote_arch() == RA_UNKNOWN) {
+ return true;
+ }
+
+ ok = remote_arch_cache_key(client_guid, ra_key);
+ if (!ok) {
+ return false;
+ }
+
+ ra_str = get_remote_arch_str();
+ if (ra_str == NULL) {
+ return false;
+ }
+
+ ok = gencache_set(ra_key, ra_str, time(NULL) + RA_CACHE_TTL);
+ if (!ok) {
+ return false;
+ }
+
+ return true;
+}
+
+bool remote_arch_cache_update(const struct GUID *client_guid)
+{
+ if (get_remote_arch() == RA_UNKNOWN) {
+ return remote_arch_cache_get(client_guid);
+ }
+
+ return remote_arch_cache_set(client_guid);
+}
+
+bool remote_arch_cache_delete(const struct GUID *client_guid)
+{
+ bool ok;
+ fstring ra_key;
+
+ ok = remote_arch_cache_key(client_guid, ra_key);
+ if (!ok) {
+ return false;
+ }
+
+ ok = gencache_del(ra_key);
+ if (!ok) {
+ return false;
+ }
+
+ return true;
+}
+
const char *tab_depth(int level, int depth)
{
if( CHECK_DEBUGLVL(level) ) {
--
2.5.0
From 3ebe1186300be56df73fbd746fd91efbceff30ab Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 13 Apr 2016 17:44:26 +0200
Subject: [PATCH 4/4] smbd: use remote arch caching
We're using the client guid as gencache db key, so this can only be used
with SMB 2_10 or higher.
The idea is that whenever we get a direct SMB2 negprot, we can then try
to see if a value is cached for the client's guid.
When a user logs off the cache entry is deleted.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smb2_negprot.c | 13 ++++++++++++-
source3/smbd/smb2_sesssetup.c | 11 +++++++++++
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c
index 9c03b2c..6cfa64f 100644
--- a/source3/smbd/smb2_negprot.c
+++ b/source3/smbd/smb2_negprot.c
@@ -162,6 +162,7 @@ NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
uint32_t max_write = lp_smb2_max_write();
NTTIME now = timeval_to_nttime(&req->request_time);
bool signing_required = true;
+ bool ok;
status = smbd_smb2_request_verify_sizes(req, 0x24);
if (!NT_STATUS_IS_OK(status)) {
@@ -260,6 +261,17 @@ NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
}
}
+ if ((dialect != SMB2_DIALECT_REVISION_2FF) &&
+ (protocol >= PROTOCOL_SMB2_10) &&
+ !GUID_all_zero(&in_guid))
+ {
+ ok = remote_arch_cache_update(&in_guid);
+ if (!ok) {
+ return smbd_smb2_request_error(
+ req, NT_STATUS_UNSUCCESSFUL);
+ }
+ }
+
switch (get_remote_arch()) {
case RA_VISTA:
case RA_SAMBA:
@@ -532,7 +544,6 @@ NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
static const uint8_t zeros[8];
size_t pad = 0;
size_t ofs;
- bool ok;
outdyn = data_blob_dup_talloc(req, security_buffer);
if (outdyn.length != security_buffer.length) {
diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c
index 78bda7b..873caed 100644
--- a/source3/smbd/smb2_sesssetup.c
+++ b/source3/smbd/smb2_sesssetup.c
@@ -1305,6 +1305,9 @@ static void smbd_smb2_logoff_shutdown_done(struct tevent_req *subreq)
struct smbd_smb2_logoff_state *state = tevent_req_data(
req, struct smbd_smb2_logoff_state);
NTSTATUS status;
+ bool ok;
+ const struct GUID *client_guid =
+ &state->smb2req->session->client->connections->smb2.client.guid;
status = smb2srv_session_shutdown_recv(subreq);
if (tevent_req_nterror(req, status)) {
@@ -1312,6 +1315,14 @@ static void smbd_smb2_logoff_shutdown_done(struct tevent_req *subreq)
}
TALLOC_FREE(subreq);
+ if (!GUID_all_zero(client_guid)) {
+ ok = remote_arch_cache_delete(client_guid);
+ if (!ok) {
+ /* Most likely not an error, but not in cache */
+ DBG_DEBUG("Deletion from remote arch cache failed\n");
+ }
+ }
+
/*
* As we've been awoken, we may have changed
* uid in the meantime. Ensure we're still
--
2.5.0
More information about the samba-technical
mailing list