[PATCH] Fix bug #2662 - revamped directory handling code is not 64bit clean

Jeremy Allison jra at samba.org
Mon Jan 13 12:21:56 MST 2014


I finally felt bad enough about the new
directory code on 64-bit servers breaking
old DOS clients that I coded up a fix
for it over the weekend :-).

It adds a memcache map into the smbd
internal directory struct - only initialized
when old DOS-style search requests come in,
so it should have zero effect on modern
clients. It creates a map from 64-bit
directory offset cookies and an incrementing
counter for the directory, which is used
as the 32-bit DOS cookie.

Andreas and David - I think Red Hat and
SuSE customers have suffered most from
this so I'm hoping you can review it
for me (or Ira, if you have time.. :-).

Re-enables the base.dir2 and raw.search
tests as we now pass them cleanly on
64-bit boxes.

Cheers,

	Jeremy.
-------------- next part --------------
From b6ab3baf4bca75a334325549f1d2f232c2f953d4 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Sat, 11 Jan 2014 14:36:17 -0800
Subject: [PATCH 1/8] s3:dir - In the old SMB1 search code, rename offset to
 wire_offset to distinguish between wire and native offsets.

Rename uint32 type to correct uint32_t.

https://bugzilla.samba.org/show_bug.cgi?id=2662

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/dir.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index 8fa320b..38da2ca 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -903,16 +903,16 @@ bool dptr_fill(struct smbd_server_connection *sconn,
 {
 	unsigned char *buf = (unsigned char *)buf1;
 	struct dptr_struct *dptr = dptr_get(sconn, key, false);
-	uint32 offset;
+	uint32_t wire_offset;
 	if (!dptr) {
 		DEBUG(1,("filling null dirptr %d\n",key));
 		return(False);
 	}
-	offset = (uint32)TellDir(dptr->dir_hnd);
+	wire_offset = (uint32_t)TellDir(dptr->dir_hnd);
 	DEBUG(6,("fill on key %u dirptr 0x%lx now at %d\n",key,
-		(long)dptr->dir_hnd,(int)offset));
+		(long)dptr->dir_hnd,(int)wire_offset));
 	buf[0] = key;
-	SIVAL(buf,1,offset);
+	SIVAL(buf,1,wire_offset);
 	return(True);
 }
 
@@ -925,7 +925,7 @@ struct dptr_struct *dptr_fetch(struct smbd_server_connection *sconn,
 {
 	unsigned int key = *(unsigned char *)buf;
 	struct dptr_struct *dptr = dptr_get(sconn, key, false);
-	uint32 offset;
+	uint32_t wire_offset;
 	long seekoff;
 
 	if (!dptr) {
@@ -933,11 +933,11 @@ struct dptr_struct *dptr_fetch(struct smbd_server_connection *sconn,
 		return(NULL);
 	}
 	*num = key;
-	offset = IVAL(buf,1);
-	if (offset == (uint32)-1) {
+	wire_offset = IVAL(buf,1);
+	if (wire_offset == (uint32_t)-1) {
 		seekoff = END_OF_DIRECTORY_OFFSET;
 	} else {
-		seekoff = (long)offset;
+		seekoff = (long)wire_offset;
 	}
 	SeekDir(dptr->dir_hnd,seekoff);
 	DEBUG(3,("fetching dirptr %d for path %s at offset %d\n",
-- 
1.8.5.1


From a3907343eeb7dc9f004d24185216395a25ac35f1 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Sat, 11 Jan 2014 14:48:00 -0800
Subject: [PATCH 2/8] s3:dir - Introduce a function to map a directory cookie
 to a 32-bit wire cookie.

Make this an identity for now.

https://bugzilla.samba.org/show_bug.cgi?id=2662

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/dir.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index 38da2ca..fb2ad88 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -895,6 +895,15 @@ void dptr_init_search_op(struct dptr_struct *dptr)
 }
 
 /****************************************************************************
+ Map a native directory offset to a 32-bit cookie.
+****************************************************************************/
+
+static uint32_t map_dir_offset_to_wire(struct dptr_struct *dptr, long offset)
+{
+	return (uint32_t)offset;
+}
+
+/****************************************************************************
  Fill the 5 byte server reserved dptr field.
 ****************************************************************************/
 
@@ -908,7 +917,7 @@ bool dptr_fill(struct smbd_server_connection *sconn,
 		DEBUG(1,("filling null dirptr %d\n",key));
 		return(False);
 	}
-	wire_offset = (uint32_t)TellDir(dptr->dir_hnd);
+	wire_offset = map_dir_offset_to_wire(dptr,TellDir(dptr->dir_hnd));
 	DEBUG(6,("fill on key %u dirptr 0x%lx now at %d\n",key,
 		(long)dptr->dir_hnd,(int)wire_offset));
 	buf[0] = key;
-- 
1.8.5.1


From fdc4d01f896fa92e9dcd5bbda8817e8a5706cf4b Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Sat, 11 Jan 2014 14:56:57 -0800
Subject: [PATCH 3/8] s3: dir - Introduce 32-bit wire versions of the 'special'
 values.

https://bugzilla.samba.org/show_bug.cgi?id=2662

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/dir.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index fb2ad88..ee9110e 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -34,6 +34,11 @@
 #define START_OF_DIRECTORY_OFFSET ((long)0)
 #define DOT_DOT_DIRECTORY_OFFSET ((long)0x80000000)
 
+/* "Special" directory offsets in 32-bit wire format. */
+#define WIRE_END_OF_DIRECTORY_OFFSET ((uint32_t)0xFFFFFFFF)
+#define WIRE_START_OF_DIRECTORY_OFFSET ((uint32_t)0)
+#define WIRE_DOT_DOT_DIRECTORY_OFFSET ((uint32_t)0x80000000)
+
 /* Make directory handle internals available. */
 
 struct name_cache_entry {
-- 
1.8.5.1


From b5ec24a00819ae93074f36277a1420bb46023e3d Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Sat, 11 Jan 2014 14:59:00 -0800
Subject: [PATCH 4/8] s3:dir - Cope with fixed mapping of 'special' values.

https://bugzilla.samba.org/show_bug.cgi?id=2662

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/dir.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index ee9110e..13e2090 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -905,6 +905,13 @@ void dptr_init_search_op(struct dptr_struct *dptr)
 
 static uint32_t map_dir_offset_to_wire(struct dptr_struct *dptr, long offset)
 {
+	if (offset == END_OF_DIRECTORY_OFFSET) {
+		return WIRE_END_OF_DIRECTORY_OFFSET;
+	} else if(offset == START_OF_DIRECTORY_OFFSET) {
+		return WIRE_START_OF_DIRECTORY_OFFSET;
+	} else if (offset == DOT_DOT_DIRECTORY_OFFSET) {
+		return WIRE_DOT_DOT_DIRECTORY_OFFSET;
+	}
 	return (uint32_t)offset;
 }
 
-- 
1.8.5.1


From 1aa0dec1061d9c5f4fb5ee9bbb6a4ca903428001 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Sat, 11 Jan 2014 15:04:38 -0800
Subject: [PATCH 5/8] s3:dir - Map wire offsets to native directory cookies.

Take care of the special offsets.

https://bugzilla.samba.org/show_bug.cgi?id=2662

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/dir.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index 13e2090..980ca58 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -938,6 +938,22 @@ bool dptr_fill(struct smbd_server_connection *sconn,
 }
 
 /****************************************************************************
+ Map a 32-bit wire cookie to a native directory offset.
+****************************************************************************/
+
+static long map_wire_to_dir_offset(struct dptr_struct *dptr, uint32_t wire_offset)
+{
+	if (wire_offset == WIRE_END_OF_DIRECTORY_OFFSET) {
+		return END_OF_DIRECTORY_OFFSET;
+	} else if(wire_offset == WIRE_START_OF_DIRECTORY_OFFSET) {
+		return START_OF_DIRECTORY_OFFSET;
+	} else if (wire_offset == WIRE_DOT_DOT_DIRECTORY_OFFSET) {
+		return DOT_DOT_DIRECTORY_OFFSET;
+	}
+	return (long)wire_offset;
+}
+
+/****************************************************************************
  Fetch the dir ptr and seek it given the 5 byte server field.
 ****************************************************************************/
 
@@ -955,11 +971,7 @@ struct dptr_struct *dptr_fetch(struct smbd_server_connection *sconn,
 	}
 	*num = key;
 	wire_offset = IVAL(buf,1);
-	if (wire_offset == (uint32_t)-1) {
-		seekoff = END_OF_DIRECTORY_OFFSET;
-	} else {
-		seekoff = (long)wire_offset;
-	}
+	seekoff = map_wire_to_dir_offset(dptr, wire_offset);
 	SeekDir(dptr->dir_hnd,seekoff);
 	DEBUG(3,("fetching dirptr %d for path %s at offset %d\n",
 		key, dptr->path, (int)seekoff));
-- 
1.8.5.1


From 9d929ac0e773af32e433a23d10844c473260dd17 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Sat, 11 Jan 2014 13:58:46 -0800
Subject: [PATCH 6/8] s3:dir - Add a new memcache type (non-talloc) -
 SMB1_SEARCH_OFFSET_MAP.

We will use this in mapping 64-bit directory offset
cookies to a 32-bit counter.

https://bugzilla.samba.org/show_bug.cgi?id=2662

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/include/memcache.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source3/include/memcache.h b/source3/include/memcache.h
index e0ac4af..9362483 100644
--- a/source3/include/memcache.h
+++ b/source3/include/memcache.h
@@ -40,7 +40,8 @@ enum memcache_number {
 	MANGLE_HASH2_CACHE,
 	PDB_GETPWSID_CACHE,	/* talloc */
 	SINGLETON_CACHE_TALLOC,	/* talloc */
-	SINGLETON_CACHE
+	SINGLETON_CACHE,
+	SMB1_SEARCH_OFFSET_MAP
 };
 
 /*
-- 
1.8.5.1


From 0385e54be799cff02299f0a7730d7fd27e02c02b Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Sat, 11 Jan 2014 15:45:48 -0800
Subject: [PATCH 7/8] s3:dir - Introduce a 64-bit directory offset <-> 32 bit
 wire offset map using memcache.

Should fix the DOS clients against 64-bit smbd's bug.

https://bugzilla.samba.org/show_bug.cgi?id=2662

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/dir.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 87 insertions(+), 2 deletions(-)

diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index 980ca58..1be5daa 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -24,6 +24,7 @@
 #include "smbd/globals.h"
 #include "libcli/security/security.h"
 #include "lib/util/bitmap.h"
+#include "memcache.h"
 
 /*
    This module implements directory related functions for Samba.
@@ -72,6 +73,8 @@ struct dptr_struct {
 	bool has_wild; /* Set to true if the wcard entry has MS wildcard characters in it. */
 	bool did_stat; /* Optimisation for non-wcard searches. */
 	bool priv;     /* Directory handle opened with privilege. */
+	uint32_t counter;
+	struct memcache *dptr_cache;
 };
 
 static struct smb_Dir *OpenDir_fsp(TALLOC_CTX *mem_ctx, connection_struct *conn,
@@ -158,6 +161,8 @@ static void dptr_idle(struct dptr_struct *dptr)
 	if (dptr->dir_hnd) {
 		DEBUG(4,("Idling dptr dnum %d\n",dptr->dnum));
 		TALLOC_FREE(dptr->dir_hnd);
+		TALLOC_FREE(dptr->dptr_cache);
+		dptr->counter = 0;
 	}
 }
 
@@ -905,6 +910,9 @@ void dptr_init_search_op(struct dptr_struct *dptr)
 
 static uint32_t map_dir_offset_to_wire(struct dptr_struct *dptr, long offset)
 {
+	DATA_BLOB key;
+	DATA_BLOB val;
+
 	if (offset == END_OF_DIRECTORY_OFFSET) {
 		return WIRE_END_OF_DIRECTORY_OFFSET;
 	} else if(offset == START_OF_DIRECTORY_OFFSET) {
@@ -912,7 +920,58 @@ static uint32_t map_dir_offset_to_wire(struct dptr_struct *dptr, long offset)
 	} else if (offset == DOT_DOT_DIRECTORY_OFFSET) {
 		return WIRE_DOT_DOT_DIRECTORY_OFFSET;
 	}
-	return (uint32_t)offset;
+	if (sizeof(long) == 4) {
+		/* 32-bit machine. We can cheat... */
+		return (uint32_t)offset;
+	}
+	if (dptr->dptr_cache == NULL) {
+		/* Lazy initialize cache. */
+		dptr->dptr_cache = memcache_init(dptr, 0);
+		if (dptr->dptr_cache == NULL) {
+			return WIRE_END_OF_DIRECTORY_OFFSET;
+		}
+	} else {
+		/* Have we seen this offset before ? */
+		key.data = (void *)&offset;
+		key.length = sizeof(offset);
+		if (memcache_lookup(dptr->dptr_cache,
+					SMB1_SEARCH_OFFSET_MAP,
+					key,
+					&val)) {
+			uint32_t wire_offset;
+			SMB_ASSERT(val.length == sizeof(wire_offset));
+			memcpy(&wire_offset, val.data, sizeof(wire_offset));
+			DEBUG(10,("found wire %u <-> offset %ld\n",
+				(unsigned int)wire_offset,
+				(long)offset));
+			return wire_offset;
+		}
+	}
+	/* Allocate a new wire cookie. */
+	do {
+		dptr->counter++;
+	} while (dptr->counter == WIRE_START_OF_DIRECTORY_OFFSET ||
+		 dptr->counter == WIRE_END_OF_DIRECTORY_OFFSET ||
+		 dptr->counter == WIRE_DOT_DOT_DIRECTORY_OFFSET);
+	/* Store it in the cache. */
+	key.data = (void *)&offset;
+	key.length = sizeof(offset);
+	val.data = (void *)&dptr->counter;
+	val.length = sizeof(dptr->counter); /* MUST BE uint32_t ! */
+	memcache_add(dptr->dptr_cache,
+			SMB1_SEARCH_OFFSET_MAP,
+			key,
+			val);
+	/* And the reverse mapping for lookup from
+	   map_wire_to_dir_offset(). */
+	memcache_add(dptr->dptr_cache,
+			SMB1_SEARCH_OFFSET_MAP,
+			val,
+			key);
+	DEBUG(10,("stored wire %u <-> offset %ld\n",
+		(unsigned int)dptr->counter,
+		(long)offset));
+	return dptr->counter;
 }
 
 /****************************************************************************
@@ -943,6 +1002,9 @@ bool dptr_fill(struct smbd_server_connection *sconn,
 
 static long map_wire_to_dir_offset(struct dptr_struct *dptr, uint32_t wire_offset)
 {
+	DATA_BLOB key;
+	DATA_BLOB val;
+
 	if (wire_offset == WIRE_END_OF_DIRECTORY_OFFSET) {
 		return END_OF_DIRECTORY_OFFSET;
 	} else if(wire_offset == WIRE_START_OF_DIRECTORY_OFFSET) {
@@ -950,7 +1012,30 @@ static long map_wire_to_dir_offset(struct dptr_struct *dptr, uint32_t wire_offse
 	} else if (wire_offset == WIRE_DOT_DOT_DIRECTORY_OFFSET) {
 		return DOT_DOT_DIRECTORY_OFFSET;
 	}
-	return (long)wire_offset;
+	if (sizeof(long) == 4) {
+		/* 32-bit machine. We can cheat... */
+		return (long)wire_offset;
+	}
+	if (dptr->dptr_cache == NULL) {
+		/* Logic error, cache should be initialized. */
+		return END_OF_DIRECTORY_OFFSET;
+	}
+	key.data = (void *)&wire_offset;
+	key.length = sizeof(wire_offset);
+	if (memcache_lookup(dptr->dptr_cache,
+				SMB1_SEARCH_OFFSET_MAP,
+				key,
+				&val)) {
+		/* Found mapping. */
+		long offset;
+		SMB_ASSERT(val.length == sizeof(offset));
+		memcpy(&offset, val.data, sizeof(offset));
+		DEBUG(10,("lookup wire %u <-> offset %ld\n",
+			(unsigned int)wire_offset,
+			(long)offset));
+		return offset;
+	}
+	return END_OF_DIRECTORY_OFFSET;
 }
 
 /****************************************************************************
-- 
1.8.5.1


From 697fb46d4c95cb5302795ff18a20b217c041e534 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 13 Jan 2014 10:20:25 -0800
Subject: [PATCH 8/8] s3:dir - We now pass the previously spinning directory
 tests on ext4.

https://bugzilla.samba.org/show_bug.cgi?id=2662

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 selftest/skip | 2 --
 1 file changed, 2 deletions(-)

diff --git a/selftest/skip b/selftest/skip
index de721f3..c16dcf5 100644
--- a/selftest/skip
+++ b/selftest/skip
@@ -107,5 +107,3 @@ bench # don't run benchmarks in our selftest
 ^samba4.blackbox.ktpass # this test isn't portable ...
 ^samba4.rpc.unixinfo # This contains a server-side getpwuid call which hangs the server when nss_winbindd is in use
 ^samba.tests.dcerpc.unix  # This contains a server-side getpwuid call which hangs the server when nss_winbindd is in use
-base.dir2 # This test spins on modern ext4, so we have to skip it
-raw.search # This test spins on modern ext4, so we have to skip it
-- 
1.8.5.1



More information about the samba-technical mailing list